pegex-parser / pegex-pm

Pegex Parser for Perl
62 stars 22 forks source link

Identifying unreachable rules #78

Open bpj opened 4 years ago

bpj commented 4 years ago

Is there any debugging aid for identifying "unreachable" rules?

I guess I should take questions like this to IRC but I'm uncomfortable with IRC due to my cerebral palsy. I keep hitting the wrong keys, including Return all the time.

bpj commented 4 years ago

To clarify I wonder whether some hook for printing a debug message when a rule is thrown out by the compiler as @ingydotnet mentioned here is or could be made available. I've looked at the code but couldn't find out where the throwing out takes place. If someone explains that I might try adding some code for printing such messages if there isn't some mechanism already.

ingydotnet commented 4 years ago

Good idea

On Mon, Apr 20, 2020, 11:59 Benct Philip Jonsson notifications@github.com wrote:

To clarify I wonder whether some hook for printing a debug message when a rule is thrown out by the compiler as @ingydotnet https://github.com/ingydotnet mentioned here https://github.com/pegex-parser/pegex-pm/issues/77#issuecomment-608213808 is or could be made available. I've looked at the code but couldn't find out where the throwing out takes place. If someone explains that I might try adding some code for printing such messages if there isn't some mechanism already.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pegex-parser/pegex-pm/issues/78#issuecomment-616746861, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAFKQROZP5CXQSHN7VVGBLRNSLRDANCNFSM4MLJCZ2A .

perlpunk commented 4 years ago

Related: additionally it could be useful to have a coverage output for the rules, so you can make sure you have enough test cases covering all rules.

ingydotnet commented 4 years ago

@bpj Sorry was on my phone when I first saw this and replied. The "throwing out" takes place here: https://github.com/pegex-parser/pegex-pm/blob/master/lib/Pegex/Compiler.pm#L58 The compiler walks the grammar tree to combine some regexes. Only the rules found by traversing the grammar from the start rule are kept. So the throwing out is passive not active.

I'm not sure how useful the a message would be to throw things out. It's usually what you want to happen. It would be interesting to hear the situation where this is tripping you up.

What I meant by "good idea" was that I misread, and thought you were suggesting that an option be added to not throw out anything from the grammar. But now I'm not even sure that's a good idea.

You can easily do yourself by dumping the output of Pegex::Compile:

perl -MPegex::Compiler -MXXX -e 'XXX + Pegex::Compiler->new->compile("a: / b /; b: /xyz/; c: /foo/")'
--- !perl/hash:Pegex::Compiler
tree:
  +toprule: a
  a:
    .rgx: !perl/regexp \Gxyz
...
  at -e line 1

Here rule b is not needed because it was folded into a. c was never referenced by a.

We can keep whatever rules we want by naming them in the call to ->compile:

perl -MPegex::Compiler -MXXX -e 'XXX + Pegex::Compiler->new->compile("a: / b /; b: /xyz/; c: /foo/", "a", "c")'
--- !perl/hash:Pegex::Compiler
tree:
  +toprule: a
  a:
    .rgx: !perl/regexp \Gxyz
  c:
    .rgx: !perl/regexp \Gfoo
...
  at -e line 1

Hope this helps. Let me know if you need more help.

bpj commented 4 years ago

@ingydotnet, sorry for not getting back before. I didn't see your last message before I had fetched my mail to my laptop and thus got things sorted into folders.

What I want to do is to get a list of those rules which get removed so that I can clear out cruft from my grammar.pgx file. I'm still very much at the trial and error stage when it comes to what I need to put into separate rules and how to best express my syntax. I seem to have succeeded by doing some keyhole surgery on Pegex::Compiler::compile():

    $self->parse($grammar);
    my @keys = keys %{$self->{tree}};
    $self->combinate(@rules);
    if ( $Pegex::Parser::DebugPruned ) {
        print STDERR "Pruned rules:\n";
        print STDERR qq{    $_\n} for
        sort grep { 
            !exists($self->{tree}{$_})
            ||
            !defined($self->{tree}{$_})
        } @keys;
    }
    $self->native;

Kind of ugly as it stands but it shows what I want to do and am now doing well enough for my current purposes. I think it may have some merit as a quick way to see which rules are not actually used. The list thus produced is much shorter than I expected and contains no surprises, so I must be doing something right! ;-)

mohawk2 commented 3 years ago

@bpj Interesting approach! Certainly a bit more introspectability would be a useful feature.

I note that your test could be simplified to just an exists check: if it doesn't exist, it certainly won't be defined!

bpj commented 3 years ago

I want to make sure both that it isn't autovivified and that it is defined if it exists. That double check achieves that.

mohawk2 commented 3 years ago

I believe you'll find that's not how autovivification works - it only creates hashes/arrays if you look inside them, not creating an undef value within an existing hash. Secondly, combinate doesn't create undef values. A simple boolean check would have been fine. Unless you have a test-case otherwise?