diff --git a/tool-testsuite/test/org/antlr/v4/test/tool/TestSymbolIssues.java b/tool-testsuite/test/org/antlr/v4/test/tool/TestSymbolIssues.java index 16f98d639..8ac0f958f 100644 --- a/tool-testsuite/test/org/antlr/v4/test/tool/TestSymbolIssues.java +++ b/tool-testsuite/test/org/antlr/v4/test/tool/TestSymbolIssues.java @@ -292,21 +292,36 @@ public class TestSymbolIssues extends BaseJavaToolTest { String[] test = { "grammar L;\n" + "\n" + - "rule1 // Correct (Alternatives)\n" + - " : t1 = a #aLabel\n" + - " | t1 = b #bLabel\n" + + "rule1 // Correct (Alternatives)\n" + + " : t1=a #aLabel\n" + + " | t1=b #bLabel\n" + " ;\n" + "rule2 //Incorrect type casting in generated code (RULE_LABEL)\n" + - " : t2 = a | t2 = b\n" + + " : t2=a | t2=b\n" + " ;\n" + "rule3\n" + - " : t3 += a+ b t3 += c+ //Incorrect type casting in generated code (RULE_LIST_LABEL)\n" + + " : t3+=a+ b t3+=c+ //Incorrect type casting in generated code (RULE_LIST_LABEL)\n" + " ;\n" + "rule4\n" + - " : a t4 = A b t4 = B c // Correct (TOKEN_LABEL)\n" + + " : a t4=A b t4=B c // Correct (TOKEN_LABEL)\n" + " ;\n" + "rule5\n" + - " : a t5 += A b t5 += B c // Correct (TOKEN_LIST_LABEL)\n" + + " : a t5+=A b t5+=B c // Correct (TOKEN_LIST_LABEL)\n" + + " ;\n" + + "rule6 // Correct (https://github.com/antlr/antlr4/issues/1543)\n" + + " : t6=a #t6_1_Label\n" + + " | t6=rule6 b (t61=c)? t62=rule6 #t6_2_Label\n" + + " | t6=A a (t61=B)? t62=A #t6_3_Label\n" + + " ;\n" + + "rule7 // Incorrect (https://github.com/antlr/antlr4/issues/1543)\n" + + " : a\n" + + " | t7=rule7 b (t71=c)? t72=rule7 \n" + + " | t7=A a (t71=B)? t72=A \n" + + " ;\n" + + "rule8 // Correct (https://github.com/antlr/antlr4/issues/1543)\n" + + " : a\n" + + " | t8=rule8 a t8=rule8\n" + + " | t8=rule8 b t8=rule8\n" + " ;\n" + "a: A;\n" + "b: B;\n" + @@ -315,8 +330,12 @@ public class TestSymbolIssues extends BaseJavaToolTest { "B: 'b';\n" + "C: 'c';\n", - "error(" + ErrorType.LABEL_TYPE_CONFLICT.code + "): L.g4:8:15: label t2=b type mismatch with previous definition: t2=a\n" + - "error(" + ErrorType.LABEL_TYPE_CONFLICT.code + "): L.g4:11:17: label t3+=c type mismatch with previous definition: t3+=a\n" + "error(" + ErrorType.LABEL_TYPE_CONFLICT.code + "): L.g4:8:13: label t2=b type mismatch with previous definition: t2=a\n" + + "error(" + ErrorType.LABEL_TYPE_CONFLICT.code + "): L.g4:11:15: label t3+=c type mismatch with previous definition: t3+=a\n" + + + "error(" + ErrorType.LABEL_TYPE_CONFLICT.code + "): L.g4:24:0: label t7 type mismatch with previous definition: TOKEN_LABEL!=RULE_LABEL\n" + + "error(" + ErrorType.LABEL_TYPE_CONFLICT.code + "): L.g4:24:0: label t71 type mismatch with previous definition: RULE_LABEL!=TOKEN_LABEL\n" + + "error(" + ErrorType.LABEL_TYPE_CONFLICT.code + "): L.g4:24:0: label t72 type mismatch with previous definition: RULE_LABEL!=TOKEN_LABEL\n" }; testErrors(test, false); @@ -344,7 +363,6 @@ public class TestSymbolIssues extends BaseJavaToolTest { // https://github.com/antlr/antlr4/issues/1543 @Test - @Ignore public void testLabelsForTokensWithMixedTypesLRWithoutLabels() { String[] test = { "grammar L;\n" + @@ -358,8 +376,8 @@ public class TestSymbolIssues extends BaseJavaToolTest { "B: 'b';\n" + "C: 'c';\n", - "error(" + ErrorType.LABEL_TYPE_CONFLICT.code + "): L.g4:5:7: label left=expr type mismatch with previous definition: left=A\n" + - "error(" + ErrorType.LABEL_TYPE_CONFLICT.code + "): L.g4:5:19: label right=expr type mismatch with previous definition: right=A\n" + "error(" + ErrorType.LABEL_TYPE_CONFLICT.code + "): L.g4:3:0: label left type mismatch with previous definition: TOKEN_LABEL!=RULE_LABEL\n" + + "error(" + ErrorType.LABEL_TYPE_CONFLICT.code + "): L.g4:3:0: label right type mismatch with previous definition: RULE_LABEL!=TOKEN_LABEL\n" }; testErrors(test, false); diff --git a/tool/src/org/antlr/v4/semantics/SymbolChecks.java b/tool/src/org/antlr/v4/semantics/SymbolChecks.java index 5caba656d..cf21ddf57 100644 --- a/tool/src/org/antlr/v4/semantics/SymbolChecks.java +++ b/tool/src/org/antlr/v4/semantics/SymbolChecks.java @@ -6,6 +6,7 @@ package org.antlr.v4.semantics; +import org.antlr.runtime.tree.CommonTree; import org.antlr.v4.automata.LexerATNFactory; import org.antlr.v4.parse.ANTLRParser; import org.antlr.v4.runtime.Token; @@ -18,6 +19,7 @@ import org.antlr.v4.tool.Grammar; import org.antlr.v4.tool.LabelElementPair; import org.antlr.v4.tool.LexerGrammar; import org.antlr.v4.tool.Rule; +import org.antlr.v4.tool.ast.AltAST; import org.antlr.v4.tool.ast.GrammarAST; import org.antlr.v4.tool.LabelType; import org.antlr.v4.tool.LeftRecursiveRule; @@ -28,6 +30,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.ArrayList; /** Check for symbol problems; no side-effects. Inefficient to walk rules * and such multiple times, but I like isolating all error checking outside @@ -128,54 +131,107 @@ public class SymbolChecks { public void checkForLabelConflicts(Collection rules) { for (Rule r : rules) { checkForAttributeConflicts(r); - if (r instanceof LeftRecursiveRule) { - // Label conflicts for left recursive rules need to be checked - // prior to the left recursion elimination step. - continue; - } - Map labelNameSpace = - new HashMap(); + Map labelNameSpace = new HashMap<>(); for (int i = 1; i <= r.numberOfAlts; i++) { - if (r.hasAltSpecificContexts()) { - labelNameSpace.clear(); - } - Alternative a = r.alt[i]; for (List pairs : a.labelDefs.values()) { - for (LabelElementPair p : pairs) { - checkForLabelConflict(r, p.label); - String name = p.label.getText(); - LabelElementPair prev = labelNameSpace.get(name); - if (prev == null) labelNameSpace.put(name, p); - else checkForTypeMismatch(prev, p); + if (r.hasAltSpecificContexts()) { + // Collect labelName-labeledRules map for rule with alternative labels. + Map> labelPairs = new HashMap<>(); + for (LabelElementPair p : pairs) { + String labelName = findAltLabelName(p.label); + if (labelName != null) { + List list; + if (labelPairs.containsKey(labelName)) { + list = labelPairs.get(labelName); + } else { + list = new ArrayList<>(); + labelPairs.put(labelName, list); + } + list.add(p); + } + } + + for (List internalPairs : labelPairs.values()) { + labelNameSpace.clear(); + checkLabelPairs(r, labelNameSpace, internalPairs); + } + } + else { + checkLabelPairs(r, labelNameSpace, pairs); } } } } } - void checkForTypeMismatch(LabelElementPair prevLabelPair, LabelElementPair labelPair) { + private void checkLabelPairs(Rule r, Map labelNameSpace, List pairs) { + for (LabelElementPair p : pairs) { + checkForLabelConflict(r, p.label); + String name = p.label.getText(); + LabelElementPair prev = labelNameSpace.get(name); + if (prev == null) { + labelNameSpace.put(name, p); + } + else { + checkForTypeMismatch(r, prev, p); + } + } + } + + private String findAltLabelName(CommonTree label) { + if (label == null) { + return null; + } + else if (label instanceof AltAST) { + AltAST altAST = (AltAST) label; + if (altAST.altLabel != null) { + return altAST.altLabel.toString(); + } + else if (altAST.leftRecursiveAltInfo != null) { + return altAST.leftRecursiveAltInfo.altLabel.toString(); + } + else { + return findAltLabelName(label.parent); + } + } + else { + return findAltLabelName(label.parent); + } + } + + private void checkForTypeMismatch(Rule r, LabelElementPair prevLabelPair, LabelElementPair labelPair) { // label already defined; if same type, no problem if (prevLabelPair.type != labelPair.type) { - String typeMismatchExpr = labelPair.type + "!=" + prevLabelPair.type; + // Current behavior: take a token of rule declaration in case of left-recursive rule + // Desired behavior: take a token of proper label declaration in case of left-recursive rule + // See https://github.com/antlr/antlr4/pull/1585 + // Such behavior is referring to the fact that the warning is typically reported on the actual label redefinition, + // but for left-recursive rules the warning is reported on the enclosing rule. + org.antlr.runtime.Token token = r instanceof LeftRecursiveRule + ? ((GrammarAST) r.ast.getChild(0)).getToken() + : labelPair.label.token; errMgr.grammarError( ErrorType.LABEL_TYPE_CONFLICT, g.fileName, - labelPair.label.token, + token, labelPair.label.getText(), - typeMismatchExpr); + labelPair.type + "!=" + prevLabelPair.type); } if (!prevLabelPair.element.getText().equals(labelPair.element.getText()) && (prevLabelPair.type.equals(LabelType.RULE_LABEL) || prevLabelPair.type.equals(LabelType.RULE_LIST_LABEL)) && (labelPair.type.equals(LabelType.RULE_LABEL) || labelPair.type.equals(LabelType.RULE_LIST_LABEL))) { + org.antlr.runtime.Token token = r instanceof LeftRecursiveRule + ? ((GrammarAST) r.ast.getChild(0)).getToken() + : labelPair.label.token; String prevLabelOp = prevLabelPair.type.equals(LabelType.RULE_LIST_LABEL) ? "+=" : "="; String labelOp = labelPair.type.equals(LabelType.RULE_LIST_LABEL) ? "+=" : "="; errMgr.grammarError( ErrorType.LABEL_TYPE_CONFLICT, g.fileName, - labelPair.label.token, + token, labelPair.label.getText() + labelOp + labelPair.element.getText(), prevLabelPair.label.getText() + prevLabelOp + prevLabelPair.element.getText()); }