Detailed debug info for reduce/reduce shift/reduce errors? about grmtools HOT 27 CLOSED

@ltratt I really like that this library DOESN'T comply with the arbitrary yacc rules - it is forcing me to understand the real underlying issues. It is painful, but the only reason that I am writing a language is to learn to write languages.

from grmtools.

have a look at the conflicts function of lrpar,
You may need to error_on_conflicts(false), and you should be able to get more detailed info from there.

From my lsp here it gets the prod idx then converts that to spans & lsp errors. It is a bit messy though.
https://github.com/ratmice/nimbleparse_lsp/blob/main/server/src/parse_thread.rs#L505-L571

from grmtools.

Thank you very much.

from grmtools.

It seems worth noting that code uses the RTParserBuilder, but I think CTParserBuilder should behave basically the same in this regard. Also worth mentioning I don't think that code has made it to a published version of the lsp yet, but should basically work.

from grmtools.

I'm having trouble seeing how to get access to an lrpar, and where to use conflicts. I don't know much about these libraries (because they work well enough that I haven't needed to), but I get that this needs to be in the build phase.

I have copied the boilerplate build.rs code. It looks like the parser is being built by the lexer builder via lrpar_config(). That is where I get lost.

from grmtools.

I see what you mean, I think there are 2 ways you can go about it.
In the old style, you build the CTLexerBuilder, CTParserBuilder separately, and combine them with a
token map, much like is described in https://softdevteam.github.io/grmtools/master/book/manuallexer.html
So you can build the CTParser yourself, outside of CTLexerBuilder, the example I can find from the repository that behaves this way is

There is some conflict testing code here also (build calls on lines 104-140 respectively)
https://github.com/softdevteam/grmtools/blob/4ed68d6feeff01738c073eced73a3192daad51c7/lrpar/src/lib/ctbuilder.rs#L1363C5-L1370

In theory, you could also construct a CTParserBuilder::new() set the output path out of the way, and .build().conflicts(), produce your errors, and panic. That really seems a bit hacky especially with the output paths part. I'd recommend the way above first.

from grmtools.

There is still one issue to be resolved, which is errors_on_conflicts(false) is going to allow the CTParserBuilder to generate output.

So, maybe the right way, is to do it in two steps:

• Contruct a LRNonStreamingLexer::new and RTParserBuilder checking for conflicts and erroring,
• if that succeeds build a CTLexerBuilder as normal?

I think that might be the most robust way to achieve this currently.

from grmtools.

Focusing on ways that we might improve this in the future:

The first thing that comes to mind is:

• CTLexerBuilder could gain an fn on_lex_build_error(f: fn(LexBuildError) -> Result<(), Box<dyn Error>>)
• CTParserBuilder could gain an fn on_grammar_error() -> (f: fn (cfgrammar::YaccGrammarError) -> Result<(), Box<dyn Error>>)
• Also a fn on_conflicts(f: fn (...) -> Result<(), Box<dyn Error>>)

These return values would then escape through the Result<..., Box<dyn Error>> result from CTLexer::build().
Curious what @ltratt thinks about the above sketch

from grmtools.

@ratmice @ltratt As a user I would be tickled pink if in the build.rs it was as simple as...

cfgrammar::yacc::generate("y", "l");

And in main.rs

cfgrammar::yacc::parse_str("");

And detailed messages on conflicts (and other things) were output when/if they occur.

And yes, I'm sure this is rather naive.

from grmtools.

I must admit that my memory of these aspects is a bit fuzzy, but I do remember some of the challenges:

1. Yacc's default is to warn about conflicts, but to resolve them based on some (fairly arbitrary) rules. This is used in some real grammars (e.g. Lua's), so we need to support it. errors_on_conflicts(false) was intended for these grammars.
2. In general, I tend to think that relying on Yacc's conflict resolution is a bad idea, so grmtools defaults to erroring out by default. You can also get it to print out information about the state table etc. (somehow? do you need to use nimbleparse for that? I don't remember!). However, personally I am too stupid to be able to understand the statetable of any grammar longer than 3 or 4 lines.

I don't necessarily object to finer-grained information, but I can't immediately see how people might end up using it. In a better, parallel univerise, Yacc would never have gifted us default conflict resolution....

from grmtools.

@ratmice

I'm getting closer. I would paste my code in here but I am having trouble formatting it well (I don't collaborate on github much).

My issue now is that when I build the CTParser up front the CTParserBuilder.build() returns an error (CTConflictsError{0 Reduce/Reduce, 1 Shift/Reduce}), but I need the CTParser to get to the detailed conflicts() info.

What am I missing?

I can parr things down and publish it in a repo if that helps.

from grmtools.

@DavidHVernon that is where error_on_conflicts(false) comes into play suppressing the CTConflictsError.
After that you should be able to get a CTParser and if there are conflicts rm generates_parser_output.rs presumably.

from grmtools.

@ltratt Yep! You said that earlier and it didn't sink in. Thanks again. I can now see the problem in my grammar.

from grmtools.

@ratmice @ltratt And FWIW, I really would love a simple YACC Genearte() / Parse() style API. I'd be interested in contributing on that if you guys were interested in steering me in the right direction.

from grmtools.

I think I'm going to give a go at the on_* PR, no harm in trying. I will try and dust off the pretty_errors example from my failed analysis branch. The output of which can be seen here:
#351

The implementation was in here (but didn't report conflicts):
440754e

from grmtools.

@ratmice I did try to give it a go.

I have your repo pulled locally and i'm on your error_callbacks branch and it builds.
I've pointed my project to use the local cfgrammar, lrlex, lrpar deps and it builds.
I've added
.on_grammar_error(&on_error)
.on_unexpected_conflicts(&on_unexpected_conflicts)
to the CTParserBuilder config chain and that works.

Here's where I get stuck.

fn on_error(yacc_grammar_error_vec: Vec) -> Box {
Box::new(yacc_grammar_error_vec[0])
}

fn on_unexpected_conflicts(
_: &YaccGrammarLexerTypesT::StorageT,
_: &StateGraphLexerTypesT::StorageT,
_: &StateTableLexerTypesT::StorageT,
_: &ConflictsLexerTypesT::StorageT,
) -> Box {
}

• I tried to define the functions as closure inside the CTParserBuilder chain. I couldn't figure that out.
• YaccGrammar, StateGraph, StateTable, & Conflicts are undefined and VSCode isn't giving me the "Quick Fix" that I need.
• I cheated with on_error() and just returned a thing that I had that was of the expected type, but I can't do that in on_unexpected_conflicts().

Could you provide an example of how this should be used? I'm close, but my recreational coding time is drawing to a close for the time being.

from grmtools.

I will certainly try, although I might not get it done in time. But should have something to look at in that regard soonish.

from grmtools.

I pushed a quick test, this is incomplete but it at least gets some of the spans out of some conflicts, consider it a work in progress.
https://github.com/ratmice/error_callbacks_test

from grmtools.

Wonderful. Thank you again. I will certainly take a peek.

from grmtools.

Thanks @ratmice!

from grmtools.

I did get a minute to kick the tires this morning.

In my particular application (with errors introduced) here is what I get as output from on_grammar_error and on_unexpected_conflicts:

Error: "Shift/Reduce: Some(Span { start: 3363, end: 3364 }) Shift: [ Reduce: Value\nShift/Reduce: Some(Span { start: 3363, end: 3364 }) Shift: [ Reduce: Value\nShift/Reduce: Some(Span { start: 3363, end: 3364 }) Shift: [ Reduce: Value\nShift/Reduce: Some(Span { start: 3363, end: 3364 }) Shift: [ Reduce: Value\n"

One comment on usability. I did have to pull in an additional dependency: lrtable. I had to pull it to get access to the parameters to on_unexpected_conflicts: StateGraph, StateTable, Conflicts.

from grmtools.

Yeah, I did a few updates to that error_callbacks_test repository, so the errors should be a bit better, and have a bit more information. (And I stopped printing out spans), this may require updating the branch as I had to add a YaccGrammarAST parameter.

Just one thing to note about the additional lrtable dependency, while it is a new dependency it's already depended upon by lrpar. It is a bit unfortunate to require it, but at least it shouldn't add any new compilation units to your project.

from grmtools.

Cool! I'll pull that tomorrow. I have the next two weeks off from work so I can refocus on more interesting things.

from grmtools.

Sorry for the plethora of branches/repos etc, I've been mostly experimenting with an alternate version based upon a trait in PR #428 There are links to two branches in the error_callbacks_test repo each with a implementation of the trait, The nicest output being from the ariadne_trait_impl branch, but the other is much simpler implementation-wise.

Sorry I haven't set these up as a library yet, (for now it's all in one for the sake of making build.rs fail), but it should be pretty similar.

The output still isn't super detailed though. Anyhow, I'll probably be heading out in the next few days, enjoy your holiday!

from grmtools.

I don't know what your process is for closing issues, but this has certainly been resolved from my perspective. And thank you very much for the explanation & enhancement.

from grmtools.

Thanks to @ratmice for all his work on this and other parts of grmtools!

from grmtools.

Yeah probably okay to close, I'll try to summarize the current status since a lot of my changes here have been difficult to land:

It is currently possible to do this sort of thing from within RTParserBuilder, likely modifying a tool like nimbleparse.
My various branches here should give a head start on getting more detailed debug output, and they should be usable from RTParserBuilder in some form without patching.

I haven't actually managed to land any fixes that make this easily accessible to CT*Builder, where we run into abstraction boundaries that make it difficult to extract all the information into one place.

But that is at least something.

from grmtools.