cenotelie / hime

Apache License 2.0
27 stars 4 forks source link

Error printing does not account for choice elements having zero length #79

Closed MechaFinch closed 12 months ago

MechaFinch commented 1 year ago

I do not know what specific conditions in a grammar cause this, but when reporting shift/reduce and reduce/reduce conflicts, there is a case where choice.elements.len() is zero and thus choice.elements[choice.elements.len() - 1] on lines 131 and 566 of sdk-rust/src/errors/print.rs subtracts with overflow and crashes the program.

woutersl commented 1 year ago

Thnak you for taking the time to report this. Would you mind sharing the grammar that raises this issue?

MechaFinch commented 1 year ago

Yes. Here are two versions. One is an older version of the grammar which is subject to the issue. I've made changes since that for whatever reason mean the current version is not subject to the issue, but I don't know what changes caused this because compiling with the rnglalr1 method, which was intended regardless, bypasses the issue.

nstl old.gram.txt nstl new.gram.txt

Additionally, having later found that specifying the parse method in the command line bypasses the issue, I've found that compiling for the Java runtime, using the rnglr1 method rather than rnglalr1 causes the AST result to be null on a successful parse, at least as indicated by syntax errors being detected when introduced to the test file. Further, the options set in the grammar file (other than Axiom and Separator) seem to be ignored, as the method and target had to be set in the command, and the package line of the generated source files was empty - being package ; regardless of the namespace set in the file or command.

woutersl commented 1 year ago

Note : the problem arises when a LR conflict is reported with a 0-length reduction, which may happen with generated rules for the * operator, as is the case in the provided grammar.

woutersl commented 1 year ago
woutersl commented 1 year ago

Further, the options set in the grammar file (other than Axiom and Separator) seem to be ignored, as the method and target had to be set in the command, and the package line of the generated source files was empty - being package ; regardless of the namespace set in the file or command.

The general principle should be that options can be specified within the grammar, but those can be overridden by their respective counterparts on the command line. Any behavior outside this principle should be considered a bug. There certainly is a bug as you mentioned, related to how java namespaces are handled. That should be fixed by c63d232.