swi-prolog/packages-cpp

The SWI-Prolog C++ interface

packages-cpp's Issues

PlFrame should be non-copyable

Rationale: If a PlFrame object is passed by value, the implicity generated copy constructor creates another PlFrame that holds the same fid value. We now have two references to the same prolog frame with different lifetimes, and when one is destroyed, the fid attribute of the other points to a frame that has already been closed. Correct me if I'm wrong, but that does not seem be intended.

Add support for "blob" atoms

Currently, the only way to store an arbitrary vaule is using the PL_put_pointer/PL_get_pointer interface. This leads to weird stuff:

?- make_my_object(X, 666), my_object_i(X, Y), free_my_object(X).
X = 95261619877024,
Y = 666.

?- my_object_i(95261619877024, Y).
Y = -753257696.

which also shows that this is dangerous because the deleted object can still be accessed (but returning an incorrect value).

The implementation of PlBlob should be an abstract class that requires defining the "release" and "compare" methods, with a default implementation of "write".

@jan - please assign this to me.

examples/README.md typo

In fourth line of README.md it reads "...example emedding SWI-Prolog." should read embedding.

Think too small subject to ask for PL, but can be stacked for new releases.

HTH

Cesar Rabak

Inconsistent PlTerm assignment semantics

The overloaded PlTerm assignment (unification) operator:
int PlTerm::operator =(const PlTerm &)
turns out to be a little treacherous in practice. Consider a statement like:

PlTerm t1;
PlTerm t2 = t1

Contrary to intuition, the second line does not use the overloaded assignment operator, but the copy constructor, and hence no unification is performed. We simply end up with a t2 that holds the same term_t reference as t1. That's why modern compilers usually output a warning when the copy assignment operator is overridden, but not the copy constructor.

So, my suggestion would be to make the copy assignment semantics consistent with copy construction. Either both should perform unification or none. I'm leaning towards performing unification in the copy constructor also, since that should be less likely to break existing code. Opinions?

Add PlPredicate, PlModule (and other?) wrappers

Occasional crash in cpp unit tests

I ran the following command:

set -e -x
cd ~/src/swipl-devel/build.debug
for i in {1..2500}
do
  env -i PATH=/usr/bin LANG=en_US.UTF-8 /usr/bin/ctest -j4 -V -R 'cpp|pcre|semweb'
  echo === $i ===
  sleep 2
done

in the build.debug directory, and on the 219th run, it got the attached error from cpp:cpp (file ~/src/swipl-devel/build.debug/Testing/Temporary/LastTest.log). I'll run again with just test cpp:cpp to see if I can reproduce it.
I've also attached the terminal output in file crash.log.
LastTest.log
crash.txt

SWI-Prolog [thread 1 (main) at Sat Mar  9 13:30:12 2024]: received fatal signal 11 (segv)
C-stack trace labeled "crash":
  [0] save_backtrace() at /home/peter/src/swipl-devel/src/os/pl-cstack.c:335 [0x7b1195c1c0e5]
  [1] print_c_backtrace() at /home/peter/src/swipl-devel/src/os/pl-cstack.c:909 [0x7b1195c1c871]
  [2] sigCrashHandler() at /home/peter/src/swipl-devel/src/os/pl-cstack.c:949 [0x7b1195c1c9ae]
  [3] alt_segv_handler() at /home/peter/src/swipl-devel/src/pl-setup.c:778 [0x7b1195b1b614]
  [4] __restore_rt() at libc_sigaction.c:? [0x7b1195845050]
  [5] fetchop() at /home/peter/src/swipl-devel/src/pl-comp.h:91 [0x7b1195a7b00e]
  [6] forAtomsInClause() at /home/peter/src/swipl-devel/src/pl-comp.c:4097 [0x7b1195a8baa8]
  [7] freeClause() at /home/peter/src/swipl-devel/src/pl-proc.c:1768 [0x7b1195aeeef5]
  [8] release_clause() at /home/peter/src/swipl-devel/src/pl-proc.c:1207 [0x7b1195aec276]
  [9] freeClauseRef() at /home/peter/src/swipl-devel/src/pl-proc.c:1238 [0x7b1195aec6e4]
  [10] removeClausesPredicate() at /home/peter/src/swipl-devel/src/pl-proc.c:1616 (discriminator 3) [0x7b1195aee5fa]
  [11] destroyDefinition() at /home/peter/src/swipl-devel/src/pl-proc.c:214 [0x7b1195ae88b6]
  [12] unallocProcedure() at /home/peter/src/swipl-devel/src/pl-proc.c:291 [0x7b1195ae9871]
  [13] unallocProcedureSymbol() at /home/peter/src/swipl-devel/src/pl-modul.c:75 [0x7b1195ac798a]
  [14] clearHTable() at /home/peter/src/swipl-devel/src/os/pl-table.c:632 [0x7b1195c0a60a]
  [15] destroyHTable() at /home/peter/src/swipl-devel/src/os/pl-table.c:507 [0x7b1195c09853]
  [16] unallocModule() at /home/peter/src/swipl-devel/src/pl-modul.c:346 [0x7b1195ac8773]
  [17] empty_module() at /home/peter/src/swipl-devel/src/pl-modul.c:439 [0x7b1195ac8ba8]
  [18] clearHTable() at /home/peter/src/swipl-devel/src/os/pl-table.c:632 [0x7b1195c0a60a]
  [19] destroyHTable() at /home/peter/src/swipl-devel/src/os/pl-table.c:507 [0x7b1195c09853]
  [20] cleanupModules() at /home/peter/src/swipl-devel/src/pl-modul.c:451 [0x7b1195ac8bf9]
  [21] PL_cleanup() at /home/peter/src/swipl-devel/src/pl-init.c:1656 [0x7b1195b6f211]
  [22] haltProlog() at /home/peter/src/swipl-devel/src/pl-fli.c:4764 [0x7b1195be15e5]
  [23] PL_halt() at /home/peter/src/swipl-devel/src/pl-fli.c:4776 [0x7b1195be162c]
  [24] pl_halt() at /home/peter/src/swipl-devel/src/pl-prims.c:5235 [0x7b1195ae17d9]
  [25] PL_next_solution___LD() at /home/peter/src/swipl-devel/src/pl-vmi.c:4669 [0x7b1195a5024f]
  [26] query_loop() at /home/peter/src/swipl-devel/src/pl-pro.c:147 [0x7b1195ae4f95]
  [27] prologToplevel() at /home/peter/src/swipl-devel/src/pl-pro.c:573 [0x7b1195ae5e64]
  [28] PL_toplevel() at /home/peter/src/swipl-devel/src/pl-fli.c:4731 [0x7b1195be1598]
  [29] /home/peter/src/swipl-devel/build.debug/src/swipl(+0x11b9) [0x586ff62891b9]
  [30] __libc_start_call_main() at ./csu/../sysdeps/x86/libc-start.c:74 [0x7b119583024a]
  [31] call_init() at ./csu/../csu/libc-start.c:128 [0x7b1195830305]
  [32] /home/peter/src/swipl-devel/build.debug/src/swipl(+0x10a1) [0x586ff62890a1]
Prolog stack:
Running on_halt hooks with status 139
Killing 21239 with default signal handlers
<end of output>

example average/3 does not clear exceptions

Running the tests with

$HOME//src/swipl-devel/build/src/swipl "-p" "foreign=" "-f" "none" "--no-packs" "--on-error=status" "-s" "$HOME/src/swipl-devel/packages/cpp/test_cpp.pl" 

?- average(X, foo(X), Average).
ERROR: Unknown procedure: foo/1
ERROR: In:
ERROR:   [13] foo(_6142)
ERROR:   [12] call(user:foo(_6176)) at /home/peter/src/swipl-devel/build/home/boot/init.pl:500
ERROR:   [11] <meta call>
ERROR:   [10] average(_6224,foo(_6232),_6228) <foreign>
ERROR:    [9] toplevel_call(user:user: ...) at /home/peter/src/swipl-devel/build/home/boot/toplevel.pl:1173
   Exception: (13) foo(_5138) ? l
Thread 1 (main): foreign predicate average/3 did not clear exception:
	error(existence_error(procedure,foo/1),context(prolog_stack([frame(13,call(user:foo/1),foo(_140)),frame(12,clause(<clause>(0x5c7a323bc120),3),call(user:foo(_174))),frame(11,meta_call,0),frame(10,foreign(user:average/3),average(_222,foo(_230),_226)),frame(9,clause(<clause>(0x5c7a325e8fc0),3),toplevel_call(user:user: ...))]),_102))
Average = 1.5NaN.

SWI-cpp2-plx.h new_hash_table invalid conversion from ‘void (*)(void*, void*)’ to ‘void (*)(table_key_t, table_value_t) #1266

SWI-Prolog/swipl-devel@1a577ef#diff-57543c056ac716618e2d0ebeff618004563d275ddc90a038c7f9fa2787c716d7L1355

The commit above changed the parameter type of function new_hash_table from void (*)(void*, void*) to void (*)(table_key_t, table_value_t).

packages-cpp/SWI-cpp2-plx.h

Line 537 in 27b016b

PLX_ASIS(hash_table_t , new_hash_table , (int size, void (*free_symbol)(table_key_t n, table_value_t v)), (size, free_symbol))

But in SWI-cpp2-plx.h line 537, there is a function call to new_hash_table whose arguement type is left unchanged.
So when I include SWI-cpp2.h, it gives me an error: invalid conversion from ‘void (*)(void*, void*)’ to ‘void (*)(table_key_t, table_value_t)
Please change the arguement type in SWI-cpp2-plx.h as well. Thanks.

typeinfo for PlException gets undefined_symbol error when loading with load_foreign_library

Since the PlExceptionBase class was added (circa v9.1.10), I get an undefined_symbol error for the typeinfo for PlException when I try to load my shared library with load_foreign_library. Before that, I could load the libraries and use the foreign predicates in Prolog (there was another issue with the string allocation functions in used with the PL_Blob_t write callback, which is why I upgraded to the latest version).

The undefined_symbol thing is usually something to do with the vtable and abstract base classes and the like; or how the objects are being linked (or both haha).

Previously, I was just using the "SWI-cpp2.h" file from my /usr/local includes as an include, but I didn't link with anything (would there even be anything to link to?) I just compiled my libraries using cmake --build and I didn't use swipl-ld. Should I be using swipl-ld? I saw that it's used in the examples. This is my cmake file: https://git.sr.ht/~sam_russell/swipl-sfml2/tree/main/item/CMakeLists.txt

I tried adding the "-rdynamic" link option to my CMakeLists.txt but that did not fix anything.

I looked PlException in the source and I saw that there were a couple TODOs associated with refactoring this and some other classes, so maybe that has something to do with it. I tried making a few haphazard code changes to see what would happen if I implemented some virtual members (also tried changing some to pure virtual) but most of these attempts wouldn't compile had other runtime errors.

I'm going to try to identify the problem in a more minimal example since my project has a lot of stuff in it, including Blob_t structs and callbacks that might not be right.

PlException should not use PlRecordExternalCopy (or PlRecord)

There should be no need to use the "record" API for the error term.

However, there appear to be some corner cases with PlQuery that can create an error term that becomes invalid. It's possible that these are incorrect uses of PlQuery - if so, they should be caught in the PlQuery constructor or PlQuery::next_solution(). Another possibility is creating multiple kinds of PlQuery for the 4 (or 6) combinations of PL_Q_EXT_STATUS and PL_CATCH_EXCEPTION / PL_PASS_EXCEPTION / PL_Q_NORMAL - and we need test cases for all these combinations.

In the meantime, using PlRecordExternalCopy for the exception term is "safe" in the sense that it doesn't crash, but it also can consume large amounts of memory by copying the contents of the error term.

SWI-cpp2.h should be in a namespace

The reason is given here: https://google.github.io/styleguide/cppguide.html#Namespaces

This would require users of SWI-cpp2.h to add a "using namespace swi_cpp2;" or add "swi_cpp2::" to all references to items in SWI-cpp2.h.

SWI-cpp2.h should include SWI-cpp2.cpp by default

See issue #60 -- users are confused by the need to #include <SWI-cpp2.cpp> as well as SWI-cpp2.h, so make the default to include both of them. Also should have a way of specifying that SWI-cpp2.cpp isn't included, to allow separate compilation (e.g., into a .a file).

PlRecord needs to define move constructor (possibly other classes as well)

All classes that contain a pointer or similar need to define a move constructor as well as a copy constructor because the default move constructor doesn't do the right thing. E.g., for PlRecord, both the copy and move constructors need to call Plx_duplicate_record(), and then the destructor can call Plx_erase().
(Need to do some experimentation to determine if the move constructor should also zero the record_t field, which is checked in the destructor before calling Plx_erase().)
(It might be necessary to keep a separate reference count in PlRecord.

It's not clear what PlQuery needs, but probably it also doesn't work properly with the default move constructor.

These changes are not in #39 - for the time being, I suggest documenting the problem because the correct fixes are not obvious.

Improve PL_scan_options in C++ api

Quoting @JanWielemaker in a code review (#31):

Overall, PL_scan_options() is an ok interface from C, but I think it needs a serious redesign for C++. Variadic argument functions and this way to use pointers to the inside of the C++ objects is not so nice 😢

How to write a predicate with the PREDICATE macro that supports multiple bindings for one variable?

I have a beginner question about the SWI-Prolog C++ interface. If this is the wrong place to post such a question, I apologize and ask: Where should I ask such a question?

I want to create a predicate foo_bar/1 that binds two possible solutions to its variable. Meaning, I want the following behavior of the resulting call in prolog:

?- foo_bar(X).
X = foo ;
X = bar.

In Prolog, I'd implement it like this:

foo_bar(X) :-
  X = foo;
  X = bar.

Now, I'd like to implement the same behavior using the C++ interface. I tried this:

PREDICATE(foo_bar, 1) {
    PL_A1 = std::string("foo").c_str();
    PL_A1 = std::string("bar").c_str();
    return true;
}

But it only returns the first binding:

?- foo(X).
X = foo.

Does the C++ interface support implementing this type of predicate? If yes, how can I do it?

Thank you very much in advance for your support!