-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENHANCED: C++-compatible exception handling + more wrapper functions and classes #39
Conversation
…and classes SWI-cpp2-plx.h contains wrapper functions (imported from SWI-cpp2.h) - throw PlException for Prolog errors - most SWI-Prolog.h functions have a wrapper. verify() methods removed from PlAtom, PlTerm, etc. - the wrapper functions do the checking. Some executable code has been moved to SWI-cpp2.cpp - can be inlined, if desired. PlException is now a subclass of std::exception, not a subclass of PlTerm. PlTypeError, PlDomainError, etc. are no longer subclasses of PlTerm, but are functions for creating suitable PlException objects. The string comparison operators are deprecated; use as_string() and std::string comparison instead, which allows specifying the encoding. Added PlRecord, PlRecordExternal, PlControl (used by PREDICATE_NONDET), PlStream. Fixed numerous bugs and misfeatures; added tests.
This pull request has been mentioned on SWI-Prolog. There might be relevant details there: https://swi-prolog.discourse.group/t/cpp2-exceptions/6040/77 |
…and classes SWI-cpp2-plx.h contains wrapper functions (imported from SWI-cpp2.h) - throw PlException for Prolog errors - most SWI-Prolog.h functions have a wrapper. verify() methods removed from PlAtom, PlTerm, etc. - the wrapper functions do the checking. Some executable code has been moved to SWI-cpp2.cpp - can be inlined, if desired. PlException is now a subclass of std::exception, not a subclass of PlTerm. PlTypeError, PlDomainError, etc. are no longer subclasses of PlTerm, but are functions for creating suitable PlException objects. The string comparison operators are deprecated; use as_string() and std::string comparison instead, which allows specifying the encoding. Added PlRecord, PlRecordExternal, PlControl (used by PREDICATE_NONDET), PlStream. Fixed numerous bugs and misfeatures; added tests.
…into exceptions-cleaned
My apologies about the multiple commits -- something went wrong with either my emacs session or git (or both) and a section of If you wish, I can re-submit a new PR with a single commit. |
dfdc6f9
to
3c6aaad
Compare
I've opened bugs #40 and #41 for some problems with |
Using PL_record_external() is broken as it doesn't respect blobs. I propose to fix PlRecord first and use that for exceptions. That is still a far from elegant solution copying exceptions 4 times rather than once, but disregarding time and space requirements, it is at least correct. |
I originally used PL_record() in PlException but it also crashed, so I switched to PL_record_external(). I'll try PL_record again, but I probably won't be able to work on it for a little while, possibly more than a month. In the meantime, could the review of other things continue, please? It would be nice to also fix PlRecord to properly do PL_duplicate_record() and PL_erase() in its constructors (the move constructor needs to be added) and destructor [or do its own reference counting], but I don't think that would be necessary for it to work with PlException. (If I can't fix PlRecord, I'll document its current limitations and how I plan to fix them in a backwards compatible way) |
I tried PlRecord instead of PlRecordExternalCopy in PlExceptions -- it passed all my tests, but failed an ASAN test with rocksdb. I'll look into that, and if I can fix it, I'll switch to PlRecord. (Eventually, hope to get rid of PlRecord entirely and just use the exception term.) The updated PlRecord is still "manual" -- that is, it doesn't use PL_duplicate_record() in the copy/move constructors ... that's a future enhancement. |
The latest commit changes from PlRecordExternalCopy to PlRecord and it passes all my tests. In future, I hope to change this to PlTerm, but not in this PR. Also in the latest commit were some changes to the constructors (including both copy and move constructors) that seemed to be needed by some strange error messages from g++, and listing all the constructors and operator= with either |
Thanks! Merged after rebase and fixing one typo for a Windows API function prototype. |
SWI-cpp2-plx.h contains wrapper functions (imported from SWI-cpp2.h)
but are functions for creating suitable PlException objects.
The string comparison operators are deprecated; use as_string()
and std::string comparison instead, which allows specifying the encoding.
Added PlRecord, PlRecordExternal, PlControl (used by PREDICATE_NONDET), PlStream. Fixed numerous bugs and misfeatures; added tests.
======
This is a pretty big set of changes, but it would have been difficult to break it up into smaller pieces.
The documentation is still somewhat rough, but a lot of things can be easily understood from reading the code or the test cases.