Skip to content

Add PlPredicate, PlModule classes#32

Closed
kamahen wants to merge 4 commits intoSWI-Prolog:masterfrom
kamahen:termv2
Closed

Add PlPredicate, PlModule classes#32
kamahen wants to merge 4 commits intoSWI-Prolog:masterfrom
kamahen:termv2

Conversation

@kamahen
Copy link
Copy Markdown
Member

@kamahen kamahen commented Dec 2, 2022

This is a follow-on to PR #31

- also removed some unused parameter defaults
Comment thread SWI-cpp2.h Outdated
{
public:
explicit PlPredicate()
: WrappedC<predicate_t>(PlPredicate::null) { }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is any scenario I'm aware of where it is useful to have a NULL predicate pointer.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed (it was in rocksdb, but I've now made it explicit)

Comment thread SWI-cpp2.h

class PlModule : public WrappedC<module_t>
{
public:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we may have a null constructor. Practically all API functions accept a NULL module as a shorthand for the current calling context, defaulting to user if such a context does not exist (when we make calls to Prolog without being called by Prolog).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a constructor that takes module_t, so PlModule(PlModule::null) is now possible.

@JanWielemaker
Copy link
Copy Markdown
Member

Great. I've lost track a little with all the PRs. When done, can you tell me by mail which to merge?

@JanWielemaker
Copy link
Copy Markdown
Member

Thanks. Squashed with #31. Added an additional patch to reduce the time needed for the overflow test by temporary lowering the stack limit.

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.

2 participants