Closed DavidHVernon closed 6 months ago
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
Thank you very much.
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.
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.
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
https://github.com/softdevteam/grmtools/blob/4ed68d6feeff01738c073eced73a3192daad51c7/lrpar/cttests/src/cgen_helper.rs#L91-L94 https://github.com/softdevteam/grmtools/blob/4ed68d6feeff01738c073eced73a3192daad51c7/lrpar/cttests/src/cgen_helper.rs#L109-L112
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.
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:
LRNonStreamingLexer::new
and RTParserBuilder
checking for conflicts and erroring,CTLexerBuilder
as normal?I think that might be the most robust way to achieve this currently.
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>>)
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
@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.
I must admit that my memory of these aspects is a bit fuzzy, but I do remember some of the challenges:
errors_on_conflicts(false)
was intended for these grammars.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....
@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.
@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.
@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.
@ltratt Yep! You said that earlier and it didn't sink in. Thanks again. I can now see the problem in my grammar.
@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.
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:
https://github.com/softdevteam/grmtools/pull/351
The implementation was in here (but didn't report conflicts): https://github.com/softdevteam/grmtools/pull/353/commits/440754e50622276e3f63183375c503464c6b0057
@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
fn on_unexpectedconflicts(
: &YaccGrammar
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.
I will certainly try, although I might not get it done in time. But should have something to look at in that regard soonish.
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
Wonderful. Thank you again. I will certainly take a peek.
Thanks @ratmice!
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
.
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.
Cool! I'll pull that tomorrow. I have the next two weeks off from work so I can refocus on more interesting things.
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!
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.
Thanks to @ratmice for all his work on this and other parts of 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.
I was wondering if there is a way to get more detailed info on CTConflictsError errors (reduce/reduce and shift/reduce). In particular it would be handy to know which production rules are clashing.