-
Notifications
You must be signed in to change notification settings - Fork 204
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
Better error message for empty ambigious alternatives. #642
Comments
Is an error message like this an example of the issue?:
I'm struggling to figure out what this means. It appears a though there is no common prefix path between these...which begs the question "why is that a problem"? Or is this indicative of some other issue? |
@mtiller Well the common prefix is the empty alternative, meaning that both alternatives are able to be parsed without any input. See this example: $.RULE("A", () => {
$.OPTION(() => {
$.CONSUME(X);
});
}
$.RULE("B", () => {
$.OPTION(() => {
$.CONSUME(Y);
});
}
$.RULE("C", () => {
$.OR([{
ALT: () => $.SUBRULE($.A)
}, {
ALT: () => $.SUBRULE($.B)
}]);
} Although
Yes, it definitely is. |
I understood your case would generate this kind of message. But when I looked at mine, it didn't feature two potentially empty alternatives. Instead, my first rule has several optional patterns before finally (and certainly) consuming a token while the second rule had an immediate token consume rule. Because the possible first tokens for the first rule are mutually exclusive from the mandatory first token in the second rule it appeared unambiguous to me, i.e., that you take the next token and if it matches the token of the second rule, use the second rule and it doesn't match, it must be the first rule. However, that clearly isn't how the parser works. Fortunately, the solution was simple...I just reversed the two alternatives. With the first rule starting unconditionally with a specific token, the ambiguity goes away. I know that in the case of common prefixes order matters (longer potential match first). But I didn't think order would matter in this case. It might be useful to put a note in the error message about this. |
Hi @mtiller I am not sure why you are getting this misleading error message in this case. |
When the ambiguity is due to multiple empty alternatives
The error message is still built as though there is an actual "real" path of terminals.
This could be slightly confusing...
See:
https://github.com/SAP/chevrotain/blob/1f82f69d5be0d27d1ddddf724257b8ea97537afd/src/parse/grammar/checks.ts#L790-L797
The text was updated successfully, but these errors were encountered: