jasder
/
antlr
1
0
Fork 1
Code Issues Pull Requests Releases 1 Wiki Activity

Merge pull request #1585 from KvanTTT/left-recursive-rule-labels 

The final fix (hopefully) for alternative labels check in left recursive rules
This commit is contained in:
Terence Parr 2017-01-29 13:34:32 -08:00 committed by GitHub
parent e7d6331632 d71c2157ce
commit 0901851719
2 changed files with 108 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
tool-testsuite/test/org/antlr/v4/test/tool/TestSymbolIssues.java
View File

@ -308,6 +308,21 @@ public class TestSymbolIssues extends BaseJavaToolTest {
"rule5\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" +
"c: C;\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);

94
tool/src/org/antlr/v4/semantics/SymbolChecks.java
View File

@ -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<Rule> 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<String, LabelElementPair> labelNameSpace =
new HashMap<String, LabelElementPair>();
Map<String, LabelElementPair> labelNameSpace = new HashMap<>();
for (int i = 1; i <= r.numberOfAlts; i++) {
if (r.hasAltSpecificContexts()) {
labelNameSpace.clear();
}
Alternative a = r.alt[i];
for (List<LabelElementPair> pairs : a.labelDefs.values()) {
if (r.hasAltSpecificContexts()) {
// Collect labelName-labeledRules map for rule with alternative labels.
Map<String, List<LabelElementPair>> labelPairs = new HashMap<>();
for (LabelElementPair p : pairs) {
String labelName = findAltLabelName(p.label);
if (labelName != null) {
List<LabelElementPair> list;
if (labelPairs.containsKey(labelName)) {
list = labelPairs.get(labelName);
} else {
list = new ArrayList<>();
labelPairs.put(labelName, list);
}
list.add(p);
}
}
for (List<LabelElementPair> internalPairs : labelPairs.values()) {
labelNameSpace.clear();
checkLabelPairs(r, labelNameSpace, internalPairs);
}
}
else {
checkLabelPairs(r, labelNameSpace, pairs);
}
}
}
}
}
private void checkLabelPairs(Rule r, Map<String, LabelElementPair> labelNameSpace, List<LabelElementPair> 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(prev, p);
}
if (prev == null) {
labelNameSpace.put(name, p);
}
else {
checkForTypeMismatch(r, prev, p);
}
}
}
void checkForTypeMismatch(LabelElementPair prevLabelPair, LabelElementPair labelPair) {
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());
}