Fix multiple problems with optional block bypass at end of rule #1546
Conversation
This change updates the default sync() strategy to match the strategy used for selecting an alternative when prediction leaves the decision rule prior to reaching a syntax error. Closes antlr#1545
| @@ -226,7 +226,11 @@ public class DefaultErrorStrategy: ANTLRErrorStrategy { | |||
| // try cheaper subset first; might get lucky. seems to shave a wee bit off | |||
| //let set : IntervalSet = recognizer.getATN().nextTokens(s) | |||
|
|
|||
| if try recognizer.getATN().nextTokens(s).contains(la) || la == CommonToken.EOF { | |||
| if try recognizer.getATN().nextTokens(s).contains(CommonToken.EPSILON) { | |||
There was a problem hiding this comment.
nextTokens in a local variable like I've done for each of the other targets.
|
@sharwell looks like some tests are failing. I'll try to look soon. |
| @@ -414,7 +414,7 @@ | |||
|
|
|||
| /** | |||
| grammar T; | |||
| a : 'a' 'b'* ; | |||
| a : 'a' 'b'* EOF ; | |||
There was a problem hiding this comment.
📝 The original behavior of the test was retained by ensuring the 'b'* loop didn't look all the way to the end of a.
| @@ -436,7 +436,7 @@ | |||
|
|
|||
| /** | |||
| grammar T; | |||
| a : 'a' ('b'|'z'{<Pass()>})*; | |||
| a : 'a' ('b'|'z'{<Pass()>})* EOF ; | |||
There was a problem hiding this comment.
📝 The original behavior of the test was retained by ensuring the closure loop didn't look all the way to the end of a.
| @@ -135,7 +135,7 @@ | |||
| public static class InvalidEmptyInput extends BaseParserTestDescriptor { | |||
| public String input = ""; | |||
| public String output = null; | |||
| public String errors = "line 1:0 missing ID at '<EOF>'\n"; | |||
| public String errors = "line 1:0 mismatched input '<EOF>' expecting ID\n"; | |||
There was a problem hiding this comment.
@parrt This behavior change was caused by removing the la == TokenConstants.EOF check in DefaultErrorStrategy.sync. Do you have a preference regarding the handling?
We know by the time this check was reached that neither EPSILON nor the LL(1) symbol are in the lookahead set from the current state. Since EPSILON is not included, the state cannot see to the end of the rule and thus nextTokens contains the complete set of valid LL(1) symbols from the current state. It is therefore impossible for the LL(1) symbol to be "expected" when considering lookahead with full context.
|
Thanks @sharwell ! |
DefaultErrorStrategy.syncnot handling falling off decision rule the same way asadaptivePredictLL1OptionalBlockFixes #1545