diff --git a/tool/resources/org/antlr/v4/tool/templates/LeftRecursiveRules.stg b/tool/resources/org/antlr/v4/tool/templates/LeftRecursiveRules.stg index be4047d0d..68612d330 100644 --- a/tool/resources/org/antlr/v4/tool/templates/LeftRecursiveRules.stg +++ b/tool/resources/org/antlr/v4/tool/templates/LeftRecursiveRules.stg @@ -41,7 +41,8 @@ recRule(ruleName, precArgDef, argName, primaryAlts, opAlts, setResultAction, [] returns [] : ( {} }; separator="\n | "> ) - ( + ( options{preventepsilon=true;}: + )* ; >> diff --git a/tool/src/org/antlr/v4/analysis/LeftRecursiveRuleTransformer.java b/tool/src/org/antlr/v4/analysis/LeftRecursiveRuleTransformer.java index 802a05cec..e227abdb8 100644 --- a/tool/src/org/antlr/v4/analysis/LeftRecursiveRuleTransformer.java +++ b/tool/src/org/antlr/v4/analysis/LeftRecursiveRuleTransformer.java @@ -132,6 +132,9 @@ public class LeftRecursiveRuleTransformer { // System.out.println("created: "+newRuleText); RuleAST t = parseArtificialRule(g, newRuleText); + // reuse the name token from the original AST since it refers to the proper source location in the original grammar + ((GrammarAST)t.getChild(0)).token = ((GrammarAST)prevRuleAST.getChild(0)).getToken(); + // update grammar AST and set rule's AST. RULES.setChild(prevRuleAST.getChildIndex(), t); r.ast = t; @@ -146,7 +149,7 @@ public class LeftRecursiveRuleTransformer { r.recPrimaryAlts.addAll(leftRecursiveRuleWalker.prefixAlts); r.recPrimaryAlts.addAll(leftRecursiveRuleWalker.otherAlts); if (r.recPrimaryAlts.isEmpty()) { - tool.errMgr.grammarError(ErrorType.NO_NON_LR_ALTS, g.fileName, ((GrammarAST)prevRuleAST.getChild(0)).getToken(), r.name); + tool.errMgr.grammarError(ErrorType.NO_NON_LR_ALTS, g.fileName, ((GrammarAST)r.ast.getChild(0)).getToken(), r.name); } r.recOpAlts = new OrderedHashMap(); @@ -205,18 +208,20 @@ public class LeftRecursiveRuleTransformer { } /** - (RULE e int _p (returns int v) - (BLOCK - (ALT - (BLOCK - (ALT INT {$v = $INT.int;}) - (ALT '(' (= x e) ')' {$v = $x.v;}) - (ALT ID)) - (* (BLOCK - (ALT {7 >= $_p}? '*' (= b e) {$v = $a.v * $b.v;}) - (ALT {6 >= $_p}? '+' (= b e) {$v = $a.v + $b.v;}) - (ALT {3 >= $_p}? '++') (ALT {2 >= $_p}? '--')))))) - + *
+	 * (RULE e int _p (returns int v)
+	 * 	(BLOCK
+	 * 	  (ALT
+	 * 		(BLOCK
+	 * 			(ALT INT {$v = $INT.int;})
+	 * 			(ALT '(' (= x e) ')' {$v = $x.v;})
+	 * 			(ALT ID))
+	 * 		(* (BLOCK
+	 *			(OPTIONS ...)
+	 * 			(ALT {7 >= $_p}? '*' (= b e) {$v = $a.v * $b.v;})
+	 * 			(ALT {6 >= $_p}? '+' (= b e) {$v = $a.v + $b.v;})
+	 * 			(ALT {3 >= $_p}? '++') (ALT {2 >= $_p}? '--'))))))
+	 * 
*/ public void setAltASTPointers(LeftRecursiveRule r, RuleAST t) { // System.out.println("RULE: "+t.toStringTree()); @@ -234,7 +239,7 @@ public class LeftRecursiveRuleTransformer { } for (int i = 0; i < r.recOpAlts.size(); i++) { LeftRecursiveRuleAltInfo altInfo = r.recOpAlts.getElement(i); - altInfo.altAST = (AltAST)opsBlk.getChild(i); + altInfo.altAST = (AltAST)opsBlk.getChild(i + 1); altInfo.altAST.leftRecursiveAltInfo = altInfo; altInfo.originalAltAST.leftRecursiveAltInfo = altInfo; // altInfo.originalAltAST.parent = altInfo.altAST.parent; diff --git a/tool/src/org/antlr/v4/automata/ParserATNFactory.java b/tool/src/org/antlr/v4/automata/ParserATNFactory.java index accbc2803..bd7420c24 100644 --- a/tool/src/org/antlr/v4/automata/ParserATNFactory.java +++ b/tool/src/org/antlr/v4/automata/ParserATNFactory.java @@ -47,7 +47,9 @@ import org.antlr.v4.runtime.atn.BasicBlockStartState; import org.antlr.v4.runtime.atn.BasicState; import org.antlr.v4.runtime.atn.BlockEndState; import org.antlr.v4.runtime.atn.BlockStartState; +import org.antlr.v4.runtime.atn.DecisionState; import org.antlr.v4.runtime.atn.EpsilonTransition; +import org.antlr.v4.runtime.atn.LL1Analyzer; import org.antlr.v4.runtime.atn.LoopEndState; import org.antlr.v4.runtime.atn.NotSetTransition; import org.antlr.v4.runtime.atn.PlusBlockStartState; @@ -65,10 +67,12 @@ import org.antlr.v4.runtime.atn.WildcardTransition; import org.antlr.v4.runtime.misc.IntervalSet; import org.antlr.v4.runtime.misc.NotNull; import org.antlr.v4.runtime.misc.Nullable; +import org.antlr.v4.runtime.misc.Pair; import org.antlr.v4.semantics.UseDefAnalyzer; import org.antlr.v4.tool.ErrorManager; import org.antlr.v4.tool.ErrorType; import org.antlr.v4.tool.Grammar; +import org.antlr.v4.tool.LeftRecursiveRule; import org.antlr.v4.tool.Rule; import org.antlr.v4.tool.ast.ActionAST; import org.antlr.v4.tool.ast.AltAST; @@ -80,6 +84,7 @@ import org.antlr.v4.tool.ast.TerminalAST; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; +import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -98,6 +103,9 @@ public class ParserATNFactory implements ATNFactory { public int currentOuterAlt; + protected final List> preventEpsilonDecisions = + new ArrayList>(); + public ParserATNFactory(@NotNull Grammar g) { if (g == null) { throw new NullPointerException("g"); @@ -114,10 +122,19 @@ public class ParserATNFactory implements ATNFactory { addRuleFollowLinks(); addEOFTransitionToStartRules(); ATNOptimizer.optimize(g, atn); + + for (Pair pair : preventEpsilonDecisions) { + LL1Analyzer analyzer = new LL1Analyzer(atn); + if (analyzer.LOOK(pair.b, null).contains(org.antlr.v4.runtime.Token.EPSILON)) { + LeftRecursiveRule r; + g.tool.errMgr.grammarError(ErrorType.EPSILON_LR_FOLLOW, g.fileName, ((GrammarAST)pair.a.ast.getChild(0)).getToken(), pair.a.name); + } + } + return atn; } - public void _createATN(Collection rules) { + protected void _createATN(Collection rules) { createRuleStartAndStopATNStates(); GrammarASTAdaptor adaptor = new GrammarASTAdaptor(); @@ -367,7 +384,7 @@ public class ParserATNFactory implements ATNFactory { return null; } - protected Handle makeBlock(BlockStartState start, GrammarAST blkAST, List alts) { + protected Handle makeBlock(BlockStartState start, BlockAST blkAST, List alts) { BlockEndState end = newState(BlockEndState.class, blkAST); start.endState = end; for (Handle alt : alts) { @@ -383,6 +400,11 @@ public class ParserATNFactory implements ATNFactory { // FASerializer ser = new FASerializer(g, h.left); // System.out.println(blkAST.toStringTree()+":\n"+ser); blkAST.atnState = start; + + if (Boolean.valueOf(blkAST.getOptionString("preventepsilon"))) { + preventEpsilonDecisions.add(new Pair(currentRule, start)); + } + return h; } diff --git a/tool/src/org/antlr/v4/tool/ErrorType.java b/tool/src/org/antlr/v4/tool/ErrorType.java index 30397ec77..3fcb9075d 100644 --- a/tool/src/org/antlr/v4/tool/ErrorType.java +++ b/tool/src/org/antlr/v4/tool/ErrorType.java @@ -130,6 +130,7 @@ public enum ErrorType { MODE_WITHOUT_RULES(145, "lexer mode '' must contain at least one non-fragment rule", ErrorSeverity.ERROR), EPSILON_TOKEN(146, "non-fragment lexer rule '' can match the empty string", ErrorSeverity.ERROR), NO_NON_LR_ALTS(147, "left recursive rule '' must contain an alternative which is not left recursive", ErrorSeverity.ERROR), + EPSILON_LR_FOLLOW(148, "left recursive rule '' contains a left recursive alternative which can be followed by the empty string", ErrorSeverity.ERROR), // Backward incompatibility errors V3_TREE_GRAMMAR(200, "tree grammars are not supported in ANTLR 4", ErrorSeverity.ERROR), diff --git a/tool/src/org/antlr/v4/tool/Grammar.java b/tool/src/org/antlr/v4/tool/Grammar.java index 94859f6e6..679609ad0 100644 --- a/tool/src/org/antlr/v4/tool/Grammar.java +++ b/tool/src/org/antlr/v4/tool/Grammar.java @@ -87,6 +87,10 @@ public class Grammar implements AttributeResolver { public static final Set ruleOptions = new HashSet(); public static final Set ParserBlockOptions = new HashSet(); + static { + // LR rule transformation sets this to help with reporting EPSILON_LR_FOLLOW + ParserBlockOptions.add("preventepsilon"); + } public static final Set LexerBlockOptions = new HashSet(); diff --git a/tool/test/org/antlr/v4/test/TestLeftRecursion.java b/tool/test/org/antlr/v4/test/TestLeftRecursion.java index 8d8e41115..7290a088c 100644 --- a/tool/test/org/antlr/v4/test/TestLeftRecursion.java +++ b/tool/test/org/antlr/v4/test/TestLeftRecursion.java @@ -387,6 +387,20 @@ public class TestLeftRecursion extends BaseTest { testErrors(new String[] { grammar, expected }, false); } + @Test public void testCheckForLeftRecursiveEmptyFollow() throws Exception { + String grammar = + "grammar T;\n" + + "s @after {System.out.println($ctx.toStringTree(this));} : a ;\n" + + "a : a ID?\n" + + " | ID\n" + + " ;\n" + + "ID : 'a'..'z'+ ;\n" + + "WS : (' '|'\\n') -> skip ;\n"; + String expected = + "error(" + ErrorType.EPSILON_LR_FOLLOW.code + "): T.g4:3:0: left recursive rule 'a' contains a left recursive alternative which can be followed by the empty string\n"; + testErrors(new String[] { grammar, expected }, false); + } + public void runTests(String grammar, String[] tests, String startRule) { rawGenerateAndBuildRecognizer("T.g4", grammar, "TParser", "TLexer"); writeRecognizerAndCompile("TParser",