ichiban / prolog

The only reasonable scripting engine for Go.
MIT License
564 stars 27 forks source link

Enable Consult to be operated from external package, and test that pr… #293

Closed vernonr3 closed 1 year ago

vernonr3 commented 1 year ago

…edicate/rule can be consulted

Without operators the Consult function can't be called and correctly completed from an external package. The existing tests add the operators manually before calling consult. But the tests in text_test.go rely on the fact they are in the "engine" package. The SetPrologOperators is a - potentially expandable - set of default operators needed to Consult prolog files.

The rawtext map added to the VM structure is a placeholder for the next pull request (about saving files).

ichiban commented 1 year ago

Hi! Thank you for the PR. Could you elaborate the problem you're solving here, please?

Without operators the Consult function can't be called and correctly completed from an external package.

I'm afraid you may have mistaken this part as a part of func TestVM_Consult(t *testing.T) which is actually a part of func TestVM_Compile(t *testing.T):

https://github.com/ichiban/prolog/blob/main/engine/text_test.go#L426-L430

Regarding rawtext, which can potentially be a separate PR, you could do the same thing within the ISO standard:

:- initialization(main).
:- dynamic(foo/1).

main :- assertz(foo(a)),
    assertz(foo(b)),
    H = foo(_), bagof((H:-B), clause(H, B), Clauses),
    write_clauses(Clauses),
    halt.

write_clauses([]).
write_clauses([(H:-true)|Cs]) :- write(H), write('.'), nl, write_clauses(Cs).
write_clauses([C|Cs]) :- write(C), write('.'), nl, write_clauses(Cs).

Of course, you can also query from Go and write them down to a file. Anyways, both can be achieved without changing this library.

vernonr3 commented 1 year ago

Hi, Thanks for your quick response. To answer your questions:

A) The broad concept:

The raw text was conceived in order that the combined source from existing prolog files and "new dynamically created prolog" can be output to a new prolog file. This could then be used with another prolog interpreter (e.g. swipl) either for comparative purposes or to leverage modules and facilities not currently available in ichiban/prolog system.

I suppose the external program could try and keep track of all the files it consults, and all the dynamic predicates etc. I felt this was hard work - and potentially fragile? And the cost of keeping/collating the Prolog rawtext inside the engine itself wouldn't be high?

B) Defining Operators Apologies if the message was misleading.

As I read the code the Consult function in text.go calls vm.ensureLoaded which then calls vm.Compile. I found that if operators weren't defined calling the overall Consult failed - actually within the Compile function.

I found the tests within TestVM_Consult did not include a file that has the operator :- for example. I added the break_term_expansion.pl (which already existed in the textdata folder) at line 470 in text_test.go as part of my pull request.

If one debugs TestVM_Consult this new test fails with the unexpectedTokenError. I chased this down and came to the conclusion that it was the lack of defined operators when the consult lead to compilation that was the problem. I believe tests to Consult files like this ought to succeed? I would expect external prolog files to include such operators..

Hope this is clearer...

In terms of the solution proposed. It was unclear (at least for me) whether the definition of operators ought to be hidden inside the Consult function - or left as a separate function call - to allow different operators to be selected. The version I put in the pull request simply added the operators defined in TestVM_Compile. I realise you may feel this is the wrong answer in the medium/long term. How should we fix the problem?

I hope this is clearer

Thanks

ichiban commented 1 year ago

I still think you're reinventing clause/2 or listing/1. Is it a common practice in other implementations?

I felt this was hard work - and potentially fragile? And the cost of keeping/collating the Prolog rawtext inside the engine itself wouldn't be high?

I think the hard work is already done by the ISO standard authors and it's not fragile. Even if the cost is low, is it legitimized for us, the maintainers and the users, to pay it every time we asserta/1 or assertz/1 for this particular use case?

If one debugs TestVM_Consult this new test fails with the unexpectedTokenError. I chased this down and came to the conclusion that it was the lack of defined operators when the consult lead to compilation that was the problem. I believe tests to Consult files like this ought to succeed?

I see but this test must fail. We made a design decision that a zero value for VM is a sandboxed one without any built-in predicates and operators. In func TestVM_Consult(t *testing.T), we explicitly use an almost zero value VM so it doesn't recognize any operators.

In practice, you'll instantiate an interpreter with prolog.New() which has all the standard operators. So, there's no problem. https://github.com/ichiban/prolog/blob/main/bootstrap.pl#L5-L23

vernonr3 commented 1 year ago

Thanks... I'll consider your points in more detail..and come back to you.. BTW I certainly wasn't intending to imply that anything in the ISO standard was fragile. I meant that seeking to manage all sorts of context outside the engine (i.e. in client code) was potentially fragile.

ichiban commented 1 year ago

Any updates?

vernonr3 commented 1 year ago

Sorry for the delay in responding. I realise that I need to create a real scenario and populate it in order to demonstrate the "why". This will take time for me to do. If you want to close this PR in the meanwhile, I can open an issue in due course and proceed to a PR when/if we agree..

ichiban commented 1 year ago

I understand. Let's close this issue for now. Please feel free to reopen it or open another!