Skip to content
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

Changes to SWI-cpp.h API #2

Merged
merged 1 commit into from Nov 12, 2022
Merged

Changes to SWI-cpp.h API #2

merged 1 commit into from Nov 12, 2022

Conversation

kamahen
Copy link
Contributor

@kamahen kamahen commented Aug 8, 2022

This PR requires SWI-Prolog/packages-cpp#16 and should be committed as soon as possible after a new release contains the SWI-cpp.h changes (probably 8.5.16).

@mgondan
Copy link
Owner

mgondan commented Aug 9, 2022

Thank you. I'll check it when 8.5.16 is out.

@kamahen
Copy link
Contributor Author

kamahen commented Aug 9, 2022

I presume that the "checks were not successful" was because it uses the current swipl, which doesn't have the updated SWI-cpp.h; and that the checks will succeed whenever a new version of swipl is released.

@mgondan
Copy link
Owner

mgondan commented Aug 9, 2022

Exactly. Let's wait until the new header files are available.

@kamahen
Copy link
Contributor Author

kamahen commented Nov 12, 2022

The code in SWI-cpp2.h appears to have a bug, which I haven't had time to fix. This doesn't seem to affect any of the systems that I know of that use SWI-cpp2.h (rolog, rocksdb, swipl-win). When I fix the bug (hopefully within a week), I will send you a message, so that you can do whatever testing you need with the updated code.

My apologies -- I was sick (not COVID) and the work on the new C++ API isn't quite finished.

@mgondan
Copy link
Owner

mgondan commented Nov 12, 2022 via email

@kamahen
Copy link
Contributor Author

kamahen commented Nov 19, 2022

The PlCall() bug seems to be an incompatibility with ASAN; I've run my tests with Debug turned on, and they all pass, so far. I have a few small changes that I'm trying to get finished in time for the v9 release.
If you've made changes to the rolog code, please push them to github, or send me a patch/diff - so that I can run your tests with my latest version of SWI-cpp2.h

@mgondan
Copy link
Owner

mgondan commented Nov 21, 2022

Thanks, Peter. The github version is the most recent one.

@kamahen
Copy link
Contributor Author

kamahen commented Nov 22, 2022

I have built rolog with the latest version from github but haven't run any tests.
The latest (and I hope final) version of SWI-cpp2.h is in SWI-Prolog/packages-cpp#25
These didn't cause any code changes to rolog, so probably they won't break any of your tests (the changes were mainly for rocksdb, which uses a lot more of the C++ API).

@kamahen
Copy link
Contributor Author

kamahen commented Nov 24, 2022

The latest release of SWI-cpp2.h (just released with SWI-Prolog 9.x) has some additional [[nodiscard]] declarations, which cause warning messages in rolog if compiled with -Wunused-result.

You can fix these by simply wrapping the call with PlCheck. For example, line 270:

    tail.next(v) ;

becomes

    PlCheck(tail.next(v));

etc.

These are the warnings that I saw with my copy of rolog:

rolog.cpp:270:14: warning: ignoring return value of ‘bool PlTerm_tail::next(PlTerm&)’, declared with attribute ‘nodiscard’ [-Wunused-result]
  270 |     tail.next(v) ;
      |     ~~~~~~~~~^~~
rolog.cpp: In function ‘PlTerm r2pl_null()’:
rolog.cpp:432:24: warning: ignoring return value of ‘bool PlTerm_tail::close()’, declared with attribute ‘nodiscard’ [-Wunused-result]
  432 |   PlTerm_tail(pl).close() ;
      |   ~~~~~~~~~~~~~~~~~~~~~^~
rolog.cpp: In function ‘PlTerm r2pl_var(Rcpp::ExpressionVector, Rcpp::CharacterVector&, PlTerm&, Rcpp::List)’:
rolog.cpp:564:14: warning: ignoring return value of ‘bool PlTerm_tail::next(PlTerm&)’, declared with attribute ‘nodiscard’ [-Wunused-result]
  564 |     tail.next(v) ;
      |     ~~~~~~~~~^~~
rolog.cpp:572:14: warning: ignoring return value of ‘bool PlTerm_tail::append(const PlTerm&)’, declared with attribute ‘nodiscard’ [-Wunused-result]
  572 |   tail.append(pl) ;
      |   ~~~~~~~~~~~^~~~
rolog.cpp: In function ‘PlTerm r2pl_list(Rcpp::List, Rcpp::CharacterVector&, PlTerm&, Rcpp::List)’:
rolog.cpp:672:18: warning: ignoring return value of ‘bool PlTerm_tail::append(const PlTerm&)’, declared with attribute ‘nodiscard’ [-Wunused-result]
  672 |       tail.append(PlCompound("-", PlTermv(PlTerm_atom(n(i)), arg))) ;
      |       ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
rolog.cpp:674:18: warning: ignoring return value of ‘bool PlTerm_tail::append(const PlTerm&)’, declared with attribute ‘nodiscard’ [-Wunused-result]
  674 |       tail.append(arg) ; // no name
      |       ~~~~~~~~~~~^~~~~
rolog.cpp:677:13: warning: ignoring return value of ‘bool PlTerm_tail::close()’, declared with attribute ‘nodiscard’ [-Wunused-result]
  677 |   tail.close() ;
      |   ~~~~~~~~~~^~
rolog.cpp: In member function ‘Rcpp::List RlQuery::bindings()’:
rolog.cpp:808:14: warning: ignoring return value of ‘bool PlTerm_tail::next(PlTerm&)’, declared with attribute ‘nodiscard’ [-Wunused-result]
  808 |     tail.next(v) ;
      |     ~~~~~~~~~^~~
rolog.cpp: In function ‘Rcpp::RObject portray_(Rcpp::RObject, Rcpp::List)’:
rolog.cpp:931:14: warning: ignoring return value of ‘bool PlTerm_tail::append(const PlTerm&)’, declared with attribute ‘nodiscard’ [-Wunused-result]
  931 |   tail.append(PlCompound("quoted", PlTermv(PlTerm_atom("false")))) ;
      |   ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
rolog.cpp:932:14: warning: ignoring return value of ‘bool PlTerm_tail::append(const PlTerm&)’, declared with attribute ‘nodiscard’ [-Wunused-result]
  932 |   tail.append(PlCompound("spacing", PlTermv(PlTerm_atom("next_argument")))) ;
      |   ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
rolog.cpp:933:13: warning: ignoring return value of ‘bool PlTerm_tail::close()’, declared with attribute ‘nodiscard’ [-Wunused-result]
  933 |   tail.close() ;
      |   ~~~~~~~~~~^~

@kamahen
Copy link
Contributor Author

kamahen commented Nov 25, 2022

These warnings are removed in PR #3

@kamahen kamahen deleted the cpp branch November 25, 2022 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants