-
Notifications
You must be signed in to change notification settings - Fork 128
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
Remove DCGs that have thrown an exception during term expansion #2681
Remove DCGs that have thrown an exception during term expansion #2681
Conversation
Thank you a lot for taking a look at this issue! Regarding the approach: In which module is |
src/lib/dcgs.pl
Outdated
@@ -216,7 +219,7 @@ | |||
|
|||
user:term_expansion(Term0, Term) :- | |||
nonvar(Term0), | |||
dcg_rule(Term0, Term). | |||
catch(dcg_rule(Term0, Term), E, (Term = null, format(" ~q~n", [E]))). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A blind catchall just leads to further problems. So restrict it to those terms that make sense here. Further, note that many implementations permit lists of clauses including []
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I would also like to use an empty list, but currently Scryer just compiles a clause []/0
, which isn't what I want to achieve here. Maybe that is a more deep problem to be solved instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One can always define the fact []
with the rule [] :- true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... and, it is still a catchall.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh... I forgot to push a commit
|
Additionally I do think that this has to be fixed in Prolog, because Rust code that generates this warning is applied universally to every compiled clause, so I will need to add an ugly special treatment for |
6706d38
to
ab91121
Compare
I think this small PR is ready for review.
|
looks good to me. further comments? |
Some DCG constructs aren't supported and can't be expanded, here we remove offending DCG rule and don't compile it at all – in a similar fashion to what we do when incorrect goal was found – whole predicate isn't getting compiled. Fixes mthom#2675
ab91121
to
a599a11
Compare
|
This just became infinitely more elegant! |
Some DCG constructs aren't supported and can't be expanded, here we remove offending DCG rule and don't compile it at all – in a similar fashion to what we do when incorrect goal was found – whole predicate isn't getting compiled.
Fixes #2675
I know that request has low priority, but it is so much simpler than loder.pl refactoring, so I just couldn't help myself and fixed it right away.