guitrace edcg grammar #24
Comments
|
This appears to be a caused by The problem also showed when I replaced the definition of I tried wrapping with |
|
I've found out recently that the difference that allows the gui debugger to trace My very ugly workaround for now is to add a Is there some specific workaround for dcg grammar in the gui debugger ? |
|
I tried the example. Works fine for me (using current development version). Please be more precise about the versions and what you did exactly to produce this. |
|
Indeed, it works for me with the current development version, and crashes with 8.5.4-23-g92b15f1de-DIRTY (the "DIRTY" is my changes to PCRE, which shouldn't be relevant). |
|
And it also throws a "not sufficiently instantiated" error with 8.5.4-35-g1bba33785-DIRTY. |
|
Sorry, should have done this from the start. |
|
It fails to reproduce for me. Saved the file as f.pl, run swipl f.pl Tracing works fine. Only len/4 is shown in decompiled version rather than as source code. That is to be expected (and requires the edgc library to use the expansion predicates that also allow for rewriting the layout information or hook into the debugger to tell the debugger how the source term relates to the compiled term). @kamahen, the error probably happens in the debugger thread, so you can't catch it in the main thread. Errors inside the debugger are a bit nasty to debug. My typical solution is to run under gdb and set a breakpoint on PL_error(). Then, if the correct error is trapped, using (gdb) |
Just to be sure, are you able to step through inside In my case, I am able to reproduce the problem even after compiling swipl-devel master branch from source. |
|
I've been able to reproduce the problem on two systems (Ubuntu 20.0.4 and a Chromebook). I might have time later today (Pacific Time) to run this with gdb and try to get a traceback using gdb. I vaguely remember a similar problem showing up a while ago, but can't remember the details. |
|
Here's the backtrace: |
|
I don't know if this is relevant, but when I also got another call to PL_error (which was suppressed?). This happened when I initially ran |
|
Not all exceptions are wrong 😄 This comes from |
Unfortunately we have thee missing frames, so know little more than that we are dealing with trying to show the source code 😢 You can try first (but after starting the gui tools such that xpce is running): That will hopefully give a complete stack trace. |
|
I've followed the code for pce_prolog_tracer:show_source and I believe the missing frames are the following:
Here is the code of clause_end(ClauseRef, File, CharA, CharZ) :-
pce_clause_info(ClauseRef, File, TPos, _),
nonvar(TPos),
arg(2, TPos, CharA),
CharZ is CharA + 1.The problem is that term_position(_129142,_129144,_129146,_129148,[_129154,_129160])So I tried to query the ?- debug(clause_info), nth_clause(increment(1, 2), 1, Ref), clause_info(Ref, File, TermPos, Vars).
% clause_info(<clause>(0xedef30)) (1-st clause of increment/2)...
% from /home/kwon-young/prog/DMOS-prolog/test_edcg.pl:10 ...
% read ...
% unified ...
% got names
Ref = <clause>(0xedef30),
File = '/home/kwon-young/prog/DMOS-prolog/test_edcg.pl',
TermPos = term_position(_, _, _, _, [_, _]),
Vars = varnames('_', '_').I'll try to debug |
|
Thanks. That explains fine the error. What I don't get is why it works for me: Just to be sure, attached is the file I used. For edcg I use: |
|
Ah, I'm using a more recent version of edcg: ?- pack_info(edcg).
Package: edcg
Title: Extended DCG
Installed version: 0.9.1.7
Installed in directory: /home/kwon-young/.local/share/swi-prolog/pack/edcg
Author: Peter Van Roy <peter.[email protected]>
Maintainer: Peter Ludemann <peter.[email protected]>
Packager: Michael Hendricks <michael@ndrix.org>
Home page: https://github.com/kamahen/edcg
Download URL: https://github.com/kamahen/edcg/archive/*.zip
Provided libraries: edcg
true. |
|
So I found this in edcg source code: % term_expansion/4 is used to work around SWI-Prolog's attempts to
% match variable names when doing a listing (or interactive trace) and
% getting confused; this sometimes results in a strange error message
% for an unknown extended_pos(Pos,N).
% Returning a variable for _Layout2 means "I don't know".
% See https://swi-prolog.discourse.group/t/strange-warning-message-from-compile-or-listing/3774
% TODO: support ((H,PB-->>B) [same as regular DCG]
user:term_expansion((H-->>B), _Layout1, Expansion, _Layout2) :-
user:term_expansion((H-->>B), Expansion).
user:term_expansion((H,PB==>>B), _Layout1, Expansion, _Layout2) :-
user:term_expansion((H,PB==>>B), Expansion).
user:term_expansion((H==>>B), _Layout1, Expansion, _Layout2) :-
user:term_expansion((H==>>B), Expansion).By unifying
This reminded me that when I was working in my PhD with a custom grammar and obscure prolog implementation, we had a debugger that was able to switch views between the source level and decompiled level, so that we could see the variables used in the grammar at the source level, or see all the accumulators values when switching to the decompiled view. |
|
I'm a bit confused ... should I change the edcg code to have _Layout1=_Layout2, or does someone need to figure out why clause_info isn't getting the term position, or both? (Changing edcg to have proper term_expansion/4 is rather low on my "todo" list, I'm afraid ... partly because I don't use trace or gtrace much for EDCGs) |
|
Ideally term_expansion/4 should translate the layout. I've pushed a fix that should stop the instantiation error. Without layout info you'll typically (always?) end up with the decompiled code in the debugger. Translating the layout can be a nasty job. You need to produce a layout term that is consistent with the output of the term expansion (defined with Eventually we must make something better. I've tried to automate this, doing it heuristically by finding where sub-terms move during the translation. This worked to some extend, but finishing the project stalled due to too many other duties. An alternative could be a more high level description of what moves where or maybe a restricted form of term expansion where the tooling could inspect the expansion rules to understand what happened. |
|
I agree that computing the layout of the produce term is a very difficult job. For now, the last fix from Jan does remove the instantiation error but the gui debugger still does not creep into the I'll try to find a solution to this and make a pr as I've started to get familiar with debugging the debugger :) |
|
Here is one weird thing that I have found:
?- debug(clause_info), nth_clause(increment(0, 1), 1, Ref), clause_info(Ref, File, TermPos, Vars).
% clause_info(<clause>(0x2142f30)) (1-st clause of increment/2)...
% from /home/kwon-young/prog/DMOS-prolog/test_edcg.pl:10 user...
% read ...
% unified ...
% got names
Ref = <clause>(0x2142f30),
File = '/home/kwon-young/prog/DMOS-prolog/test_edcg.pl',
TermPos = term_position(_, _, _, _, [_, _]),
Vars = varnames('_', '_').
?- debug(clause_info), nth_clause(len(0, X, [a], []), 1, Ref), clause_info(Ref, File, TermPos, Vars).
% clause_info(<clause>(0x21518c0)) (1-st clause of len/4)...
% from /home/kwon-young/prog/DMOS-prolog/test_edcg.pl:17 user...
% read ...
% Could not unify clause
false.After some debugging, I think that the reason that This means that the gui debugger is somehow able to :
|
|
clause_info failing on source clauses is fine. That normally should make it revert to listing the clause and then compute the clause info not for the actual source code but for the decompiled version. That should always succeed. At least, that is how I have the story in my head ... |
|
@kwon-young wrote "the gui debugger still does not creep into the increment term" Do you get the same behavior with the non-gui debugger? Perhaps the call to increment has been optimized, so would only show if you run with "debug" mode? (Or maybe it's only backtrace that needs "debug" mode to show everything ...) |
|
@kwon-young If you improve edcg's term_expansion/4, I'll happily take a PR. And any other improvements while you're at it -- I'm not terribly happy with the need to declare all edcg expansions before any use; it would be nice to be able to declare the EDCG expansion info with the predicate, e.g.: Perhaps this could be done by 2-pass expansion, by adding a term_expansion on end_of_input ... there was some discussion here: https://swi-prolog.discourse.group/t/multi-pass-compilation-for-term-expansion/2196 |
|
Indeed, increment/2 is not traced. I think the problem is that the debugger reads the source term, calls term_expansion/4 to expect that to return a term layout. As the term is nicely expanded and a layout is returned, it is happy. Next the debugger tries to show the source given the layout, but fails. The best solution is surely to return a proper layout term. Alternatively we could consider to reject the layout if it is critically unbound. I've pushed a patch to library(clause_info) of the main repo to address this. At least you can now trace through all the code in decompiled format. |
Thank you very much for this !
Well, I'll try since I would really benefit from this.
This seems like a really hard problem to solve with my little knowledge of prolog ... i believe this issue can be closed now since the main problem was resolved. |
|
@kwon-young The 2-pass processing can be mostly done independently of improving the term_expansion/4, I think. I'm going to make a small change to the code, which should simplify integrating your changes and my 2-pass changes. (Hopefully, I'll do that today or tomorrow.) |
|
It looks pretty straightforward although tedious to put the proper position information into It also appears that I'd need to create a @kwon-young - Please wait a few days before starting to work on this -- I think I can make the changes quickly if higher priority things don't get in the way. (PS: @JanWielemaker - clicking on "make_varnames_hook/5" in https://www.swi-prolog.org/pldoc/doc_for?object=prolog_clause%3Amake_varnames/5 gives a 403 error: "You don't have permission to access /home/jan/src/plweb/pack/mirror/GIT/xlibrary/prolog/mapnlist.pl on this server") |
@kamahen I can do tedious work without a problem :) |
|
@kwon-young - I'm about half-way through a set of changes; but I won't be able to do more work on it until Tuesday (Pacific Time). I'd need to finish this set of changes before giving it to you for the "tedious" work. ;) In the meantime, perhaps you could look at the tests, which are run by
Here's some debugging code that recursively processes a term with its position information, and might be useful: https://gist.github.com/kamahen/a74f67d1fc2dcd6325d0a326a842fb3f |
Well, you need to be careful to pass on the clause location information if you do two pass compilation. If you temporary store a term you need to call |
|
@JanWielemaker Why would the expansion need |
|
That is not the way the gui tracer works. The compiler just stores the file and line of the clause. The gui tracer reads the clause to get the raw term and its layout info. It then needs to do the magic to relate this to the actual (transformed) clause and produce layout information for that. One of its steps is that it tries calling term_expansion/4 to see whether the source term+layout is transformed into the target clause+layout. If that fails it has a number of rules and heuristics as well as a hook that allows packages to do the matching. See library(prolog_clause). So, all the compiler needs to do is to ensure the source file:line is correct. If it clause is immediately asserted this is provided by source_location/2, which records this info from the last read term. If you create the clause later you must remember this location and pass it on. |
|
The guitracer's design makes 2-pass term_expansion rather tricky, if not impossible. I'll have to rethink things - and there are other issues, such as making edcg play nicely with modules. Anyway, it seems that besides term_expansion/4, I also need define prolog_clause:make_varnames_hook/5 and prolog_clause:unify_clause_hook/5. Anything else? |
This is probably true that term_expansion/4 cannot solve this problem when you have two passes. You need to use the prolog_clause:unify_clause_hook/5 hook for this case. Should you have had a working term_expansion/4 it should be quite eacy to define prolog_clause:unify_clause_hook/5 on top of that (I think). The |
|
Instead of making term_expansion/4 produce the correct Layout (which is turning to be both trickier and more tedious than I had expected), is a better solution to add sufficient information to prolog_clause:unify_clause_hook/5? Does the builtin DCG do this, or does it also have the equivalent of term_expansion/4 that produces a correct Layout? |
|
I'm now thinking about doing a much more minimal jobs of creating the Layout for the expanded clauses. For a "minimal" layout, appears that I need to do the following:
|
|
AFAIK, all it needs is a matching layout term up to the clause subgoals. The details are ignored. Maybe we need them at some point to show variable bindings on hovering, but probably it is simpler to just get the clause hovered and the name of the variable hovered, neither of which needs the clause layout. For the overall structure the subterm positions must be valid. For the final subgoal the first two arguments must be valid. The common structure of all layout terms is that the first two arguments give the character range for the whole thing and code that is not interested in the details thus (should) use arg/3 to get the range. Note that {G} uses And yes, Thinking of a more robust solution is a nice challenge. I once started on that, following the ideas behind dif: find (sub)term correspondences between the input and output clause and reconstruct the layout from there. It started to work, but the project stalled due to other priorities. I still think this is the way to go, although it probably also requires some hooks to help it resolving ambiguities. |
|
The changes have mostly been done, but I'm not sure how best to test them: |
|
@JanWielemaker - the |
|
Yes, such a predicate could be useful. I'd name it
Not really. The mechanisms in prolog_clause predate term_position/4. The latter was modeled after SICStus. I'm not happy with it. It is simply too complicated 😢 |
|
@kwon-young I won't be able to work on edcg.pl for a little while (not sure how long) ... I've pushed the latest code to https://github.com/kamahen/edcg/tree/te4 (that's the "te4" branch). You can see failing test cases by: I don't want to promote this code to "master" until it's complete; but it might be useful to you in its incomplete form. If there are changes to SWI-Prolog/swipl-devel#925 , I'll incorporate them; and when that PR becomes part of a new SWI-Prolog devel release, I'll make the appropriate changes to edcg.pl. |
Hello,
First of all, merry christmas !
I'm having some trouble to debug a grammar written using the edcg library.
Here is a simple example from the library:
Launching the gui debugger using
gtrace, len([a],1)., I can correctly creep intolenbut not intoincrement.I also get this error:
Using the command line tracer I obtain:
I'm available if you need any help in order to resolve this issue, although I've never used xpce before.
The text was updated successfully, but these errors were encountered: