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

Grammar::Tracer changes outcome of parse #13

Closed
masak opened this issue Dec 17, 2014 · 6 comments
Closed

Grammar::Tracer changes outcome of parse #13

masak opened this issue Dec 17, 2014 · 6 comments

Comments

@masak
Copy link

masak commented Dec 17, 2014

See https://rt.perl.org/Ticket/Display.html?id=123452 for a concrete example. That ticket also establishes that Rakudo is right (when it fails the grammar), and Grammar::Tracer is wrong (when it makes it parse).

@meisl
Copy link
Contributor

meisl commented Jan 2, 2015

@masak,

I need to digest those comments

, too. I mean I still have to dig my way through the ticket you linked to.

However, even if there may be (ie apart from any) bugs in the example grammars given:
from your description's overall impression, and from

I know it's wrong that Grammar::Tracer heisenbugs the parse. :)

... I do get the very strong feeling, that this could be yet another symptom of a rather general flaw regarding how state (of Tracer/Debugger as well as the current parse) is handled.

The baseline is that state should be dynamic, rather than static. Maybe this rings a bell for you, as you're into the specific example...?
I'll have to have a closer look, though; when I find some time.

Cheers :)

@hoelzro
Copy link

hoelzro commented Jul 13, 2015

I've found this in a grammar I wrote as well. Golfed version:

use Grammar::Tracer;

grammar CppGrammar {
    proto token type { * }
    token type:builtin {
        [
            || 'const char *'
            || 'void'
        ]
    }

    rule type:identifier {
        <?>
        'const'?
        \w+
        $<pointy>=[<[&*]>* % <.ws>]

    }
}

class MyActions {
    method type:builtin ($/) {
        make 'builtin'
    }

    method type:identifier ($/) {
        make 'identifier'
    }
}

say CppGrammar.parse('const char *', :rule<type>, :actions(MyActions)).made;

Prints identifier without Grammar::Tracer, builtin with.

@cognominal
Copy link

Tracing breaks the logic of NFAs where, conceptually, the rules are entered in parallel to
find the longest match.
A dedicated logic would be needed to trace that.
Sad because G::T fails where it is most needed.
Simple example below. Works without Grammar::Tracer.

use Grammar::Tracer;
grammar A {
token TOP { }
proto token a { <...> }
token a:sym { 'a' }
token a:sym { 'aa' }
}
say 'got ' ~ A.parse('aa');

@nobodyinperson
Copy link

Tracing breaks the logic of NFAs where, conceptually, the rules are entered in parallel to
find the longest match.

Whatever NFA is, is it possible to somehow reproduce the matching results that Grammar::Tracer yields by setting some magical settings? I pretty often find that my grammar should match when it does with Grammar::Tracer but not without...
It's really frustrating debugging a complex grammar, tweaking it until it matches your string, removing the debugger just to see that everything has gone haywire again...

@masak
Copy link
Author

masak commented Feb 2, 2017

Certainly possible; it's "just" a matter of someone diving in and fixing it.

For what it's worth, you're not the only one who's frustrated. Here's my latest conversation about this with @jnthn, four days ago:

<masak> grmbl; another case of Grammar::Tracer changing the outcome of the parse... :/
<masak> jnthn: if it's worthwhile for me to document these, I could spend some time writing it up. if not, I'll just skip it. :)
<jnthn> masak: Or just write a patch that makes the returned closures have a !NFA method (or some such), perhaps by mixing in a role or smilar, which passes on the NFA of the wrapped-up rule.
<jnthn> I'm pretty sure that's what's to blame for the discrepancy
<masak> jnthn: I see we both wish I were more familiar with that codebase :P

jnthn added a commit that referenced this issue Feb 2, 2017
@jnthn
Copy link
Owner

jnthn commented Feb 2, 2017

Fixed, previously failing test case now passes. Bumped version. Also a few other small tweaks/cleanups along the way.

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

No branches or pull requests

6 participants