From 2cc39c2f9b0111b0adf27d2421ec8eba01eee434 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Sat, 10 Mar 2012 16:16:39 -0600 Subject: [PATCH] Evaluate predicates for all ambiguous alternatives and report if still ambiguous after predicate evaluation. Remove misleading message insufficient predicates. Fixes antlr/antlr4#31. Partially addresses antlr/antlr4#39. --- .../antlr/v4/runtime/ANTLRErrorStrategy.java | 12 -- .../v4/runtime/DefaultErrorStrategy.java | 11 -- .../v4/runtime/DiagnosticErrorStrategy.java | 18 --- .../v4/runtime/atn/ParserATNSimulator.java | 143 ++++++++---------- .../antlr/v4/test/TestSemPredEvalParser.java | 6 +- 5 files changed, 64 insertions(+), 126 deletions(-) diff --git a/runtime/Java/src/org/antlr/v4/runtime/ANTLRErrorStrategy.java b/runtime/Java/src/org/antlr/v4/runtime/ANTLRErrorStrategy.java index d3aed46e1..c577b5ee1 100644 --- a/runtime/Java/src/org/antlr/v4/runtime/ANTLRErrorStrategy.java +++ b/runtime/Java/src/org/antlr/v4/runtime/ANTLRErrorStrategy.java @@ -140,16 +140,4 @@ public interface ANTLRErrorStrategy { @NotNull DFA dfa, int startIndex, int stopIndex, @NotNull ATNConfigSet configs); - - /** Called by the parser when it finds less than n-1 predicates for n ambiguous alternatives. - * If there are n-1, we assume that the missing predicate is !(the "or" of the other predicates). - * If there are fewer than n-1, then we don't know which make it alternative to protect - * if the predicates fail. - */ - void reportInsufficientPredicates(@NotNull Parser recognizer, - @NotNull DFA dfa, - int startIndex, int stopIndex, @NotNull IntervalSet ambigAlts, - DecisionState decState, - @NotNull SemanticContext[] altToPred, - @NotNull ATNConfigSet configs, boolean fullContextParse); } diff --git a/runtime/Java/src/org/antlr/v4/runtime/DefaultErrorStrategy.java b/runtime/Java/src/org/antlr/v4/runtime/DefaultErrorStrategy.java index f4f7cf87e..6b2385541 100644 --- a/runtime/Java/src/org/antlr/v4/runtime/DefaultErrorStrategy.java +++ b/runtime/Java/src/org/antlr/v4/runtime/DefaultErrorStrategy.java @@ -570,15 +570,4 @@ public class DefaultErrorStrategy implements ANTLRErrorStrategy { int startIndex, int stopIndex, @NotNull ATNConfigSet configs) { } - - @Override - public void reportInsufficientPredicates(@NotNull Parser recognizer, - @NotNull DFA dfa, - int startIndex, int stopIndex, - @NotNull IntervalSet ambigAlts, - DecisionState decState, - @NotNull SemanticContext[] altToPred, - @NotNull ATNConfigSet configs, boolean fullContextParse) - { - } } diff --git a/runtime/Java/src/org/antlr/v4/runtime/DiagnosticErrorStrategy.java b/runtime/Java/src/org/antlr/v4/runtime/DiagnosticErrorStrategy.java index d05944ca3..a2571e362 100644 --- a/runtime/Java/src/org/antlr/v4/runtime/DiagnosticErrorStrategy.java +++ b/runtime/Java/src/org/antlr/v4/runtime/DiagnosticErrorStrategy.java @@ -30,14 +30,10 @@ package org.antlr.v4.runtime; import org.antlr.v4.runtime.atn.ATNConfigSet; -import org.antlr.v4.runtime.atn.DecisionState; -import org.antlr.v4.runtime.atn.SemanticContext; import org.antlr.v4.runtime.dfa.DFA; import org.antlr.v4.runtime.misc.IntervalSet; import org.antlr.v4.runtime.misc.NotNull; -import java.util.Arrays; - public class DiagnosticErrorStrategy extends DefaultErrorStrategy { @Override public void reportAmbiguity(@NotNull Parser recognizer, @@ -65,18 +61,4 @@ public class DiagnosticErrorStrategy extends DefaultErrorStrategy { recognizer.notifyErrorListeners("reportContextSensitivity d=" + dfa.decision + ": " + configs + ", input='" + recognizer.getInputString(startIndex, stopIndex) + "'"); } - - @Override - public void reportInsufficientPredicates(@NotNull Parser recognizer, - @NotNull DFA dfa, - int startIndex, int stopIndex, - @NotNull IntervalSet ambigAlts, - DecisionState decState, - @NotNull SemanticContext[] altToPred, - @NotNull ATNConfigSet configs, boolean fullContextParse) - { - recognizer.notifyErrorListeners("reportInsufficientPredicates d=" + dfa.decision + ", decState=" + decState + - ", ambigAlts=" + ambigAlts + ":" + Arrays.toString(altToPred) + - ", " + configs + ", input='" + recognizer.getInputString(startIndex, stopIndex) + "'"); - } } diff --git a/runtime/Java/src/org/antlr/v4/runtime/atn/ParserATNSimulator.java b/runtime/Java/src/org/antlr/v4/runtime/atn/ParserATNSimulator.java index 61ae46fff..1628f4d58 100755 --- a/runtime/Java/src/org/antlr/v4/runtime/atn/ParserATNSimulator.java +++ b/runtime/Java/src/org/antlr/v4/runtime/atn/ParserATNSimulator.java @@ -417,11 +417,12 @@ public class ParserATNSimulator extends ATNSimulator { if ( s.predicates!=null ) { // rewind input so pred's LT(i) calls make sense input.seek(startIndex); - int predictedAlt = evalSemanticContext(s.predicates, outerContext); - if ( predictedAlt!=ATN.INVALID_ALT_NUMBER ) { - return predictedAlt; + IntervalSet alts = evalSemanticContext(s.predicates, outerContext); + if (alts.isNil()) { + throw noViableAlt(input, outerContext, s.configset, startIndex); } - throw noViableAlt(input, outerContext, s.configset, startIndex); + + return alts.getMinElement(); } if ( dfa_debug ) System.out.println("DFA decision "+dfa.decision+ @@ -566,26 +567,23 @@ public class ParserATNSimulator extends ATNSimulator { List predPredictions = predicateDFAState(D, D.configset, outerContext, nalts); if ( predPredictions!=null ) { - IntervalSet conflictingAlts = getConflictingAltsFromConfigSet(D.configset); - if ( D.predicates.size() < conflictingAlts.size() ) { - reportInsufficientPredicates(dfa, startIndex, input.index(), - conflictingAlts, - decState, - getPredsForAmbigAlts(conflictingAlts, D.configset, nalts), - D.configset, - false); - } + int stopIndex = input.index(); input.seek(startIndex); - predictedAlt = evalSemanticContext(predPredictions, outerContext); - if ( predictedAlt!=ATN.INVALID_ALT_NUMBER ) { - return predictedAlt; + IntervalSet alts = evalSemanticContext(predPredictions, outerContext); + D.prediction = ATN.INVALID_ALT_NUMBER; + switch (alts.size()) { + case 0: + throw noViableAlt(input, outerContext, D.configset, startIndex); + + case 1: + return alts.getMinElement(); + + default: + // report ambiguity after predicate evaluation to make sure the correct + // set of ambig alts is reported. + reportAmbiguity(dfa, D, startIndex, stopIndex, alts, D.configset); + return alts.getMinElement(); } - - // Consistency check - the DFAState should not have a "fallback" - // prediction specified for the case where no predicates succeed. - assert D.prediction == ATN.INVALID_ALT_NUMBER; - - throw noViableAlt(input, outerContext, D.configset, startIndex); } } @@ -642,24 +640,26 @@ public class ParserATNSimulator extends ATNSimulator { if ( altToPred!=null ) { // we have a validating predicate; test it predPredictions = getPredicatePredictions(reach.conflictingAlts, altToPred); - if ( predPredictions.size() < nalts ) { - IntervalSet conflictingAlts = getConflictingAltsFromConfigSet(reach); - DecisionState decState = atn.getDecisionState(dfa.decision); - reportInsufficientPredicates(dfa, startIndex, input.index(), - conflictingAlts, - decState, altToPred, - reach, true); - } input.seek(startIndex); - reach.uniqueAlt = evalSemanticContext(predPredictions, outerContext); - if ( reach.uniqueAlt != ATN.INVALID_ALT_NUMBER ) { + IntervalSet alts = evalSemanticContext(predPredictions, outerContext); + reach.uniqueAlt = ATN.INVALID_ALT_NUMBER; + switch (alts.size()) { + case 0: + throw noViableAlt(input, outerContext, reach, startIndex); + + case 1: + reach.uniqueAlt = alts.getMinElement(); return reach; + + default: + // reach.conflictingAlts holds the post-evaluation set of ambig alts + reach.conflictingAlts = alts; + break; } - throw noViableAlt(input, outerContext, reach, startIndex); } } - // must have conflict and no semantic preds + // must have conflict reportAmbiguity(dfa, D, startIndex, input.index(), reach.conflictingAlts, reach); reach.uniqueAlt = reach.conflictingAlts.getMinElement(); @@ -786,7 +786,7 @@ public class ParserATNSimulator extends ATNSimulator { // Optimize away p||p and p&&p for (int i = 0; i < altToPred.length; i++) { - if ( altToPred[i]!=null ) altToPred[i] = altToPred[i].optimize(); + altToPred[i] = altToPred[i].optimize(); } // nonambig alts are null in altToPred @@ -797,64 +797,62 @@ public class ParserATNSimulator extends ATNSimulator { public List getPredicatePredictions(IntervalSet ambigAlts, SemanticContext[] altToPred) { List pairs = new ArrayList(); - int firstUnpredicated = ATN.INVALID_ALT_NUMBER; - // keep track of the position in pairs where the unpredicated choice should go - int firstUnpredicatedIndex = -1; + boolean containsPredicate = false; for (int i = 1; i < altToPred.length; i++) { SemanticContext pred = altToPred[i]; + + // unpredicated is indicated by SemanticContext.NONE + assert pred != null; + // find first unpredicated but ambig alternative, if any. // Only ambiguous alternatives will have SemanticContext.NONE. // Any unambig alts or ambig naked alts after first ambig naked are ignored // (null, i) means alt i is the default prediction // if no (null, i), then no default prediction. - if ( ambigAlts!=null && ambigAlts.contains(i) && - pred==SemanticContext.NONE && firstUnpredicated==ATN.INVALID_ALT_NUMBER ) - { - firstUnpredicated = i; - firstUnpredicatedIndex = pairs.size(); + if (ambigAlts!=null && ambigAlts.contains(i) && pred==SemanticContext.NONE) { + pairs.add(new DFAState.PredPrediction(null, i)); } - if ( pred!=null && pred!=SemanticContext.NONE ) { + else if ( pred!=SemanticContext.NONE ) { + containsPredicate = true; pairs.add(new DFAState.PredPrediction(pred, i)); } } - if ( pairs.isEmpty() ) pairs = null; - else if ( firstUnpredicated!=ATN.INVALID_ALT_NUMBER ) { - // add default prediction if we found null predicate - pairs.add(firstUnpredicatedIndex, new DFAState.PredPrediction(null, firstUnpredicated)); + + if ( !containsPredicate ) { + pairs = null; } + // System.out.println(Arrays.toString(altToPred)+"->"+pairs); return pairs; } - /** Look through a list of predicate/alt pairs, returning alt for the - * first pair that wins. A null predicate indicates the default - * prediction for disambiguating predicates. + /** Look through a list of predicate/alt pairs, returning alts for the + * pairs that win. A {@code null} predicate indicates an alt containing an + * unpredicated config which behaves as "always true." */ - public int evalSemanticContext(List predPredictions, - ParserRuleContext outerContext) + public IntervalSet evalSemanticContext(List predPredictions, + ParserRuleContext outerContext) { - int predictedAlt = ATN.INVALID_ALT_NUMBER; -// List predPredictions = D.predicates; -// if ( D.isCtxSensitive ) predPredictions = D.ctxToPredicates.get(outerContext); + IntervalSet predictions = new IntervalSet(); for (DFAState.PredPrediction pair : predPredictions) { if ( pair.pred==null ) { - predictedAlt = pair.alt; // default prediction - break; + predictions.add(pair.alt); + continue; } boolean evaluatedResult = pair.pred.eval(parser, outerContext); if ( debug || dfa_debug ) { System.out.println("eval pred "+pair+"="+evaluatedResult); } + if ( evaluatedResult ) { if ( debug || dfa_debug ) System.out.println("PREDICT "+pair.alt); - predictedAlt = pair.alt; - break; + predictions.add(pair.alt); + continue; } } - // if no prediction: either all predicates are false and - // all alternatives were guarded, or a validating predicate failed. - return predictedAlt; + + return predictions; } @@ -1413,23 +1411,4 @@ public class ParserATNSimulator extends ATNSimulator { if ( parser!=null ) parser.getErrorHandler().reportAmbiguity(parser, dfa, startIndex, stopIndex, ambigAlts, configs); } - - public void reportInsufficientPredicates(@NotNull DFA dfa, int startIndex, int stopIndex, - @NotNull IntervalSet ambigAlts, - DecisionState decState, - @NotNull SemanticContext[] altToPred, - @NotNull ATNConfigSet configs, - boolean fullContextParse) - { - if ( debug || retry_debug ) { - System.out.println("reportInsufficientPredicates "+ - ambigAlts+", decState="+decState+": "+Arrays.toString(altToPred)+ - parser.getInputString(startIndex, stopIndex)); - } - if ( parser!=null ) { - parser.getErrorHandler().reportInsufficientPredicates(parser, dfa, startIndex, stopIndex, ambigAlts, - decState, altToPred, configs, fullContextParse); - } - } - } diff --git a/tool/test/org/antlr/v4/test/TestSemPredEvalParser.java b/tool/test/org/antlr/v4/test/TestSemPredEvalParser.java index 7d0ed955f..a1cf21039 100644 --- a/tool/test/org/antlr/v4/test/TestSemPredEvalParser.java +++ b/tool/test/org/antlr/v4/test/TestSemPredEvalParser.java @@ -153,7 +153,7 @@ public class TestSemPredEvalParser extends BaseTest { "alt 1\n" + "alt 1\n"; assertEquals(expecting, found); - assertEquals("line 1:0 reportInsufficientPredicates d=0, decState=24, ambigAlts={1..3}:[{-1:-1}?, {-1:-1}?, {-1:-1}?, {1:0}?], [(6,1,[],up=1), (1,1,[],up=1), (6,2,[],up=1), (1,2,[],up=1), (6,3,[],{1:0}?,up=1), (1,3,[],{1:0}?,up=1)],hasSemanticContext=true,conflictingAlts={1..3},dipsIntoOuterContext, input='x'\n", + assertEquals("line 1:0 reportAmbiguity d=0: ambigAlts={1..2}:[(6,1,[],up=1), (1,1,[],up=1), (6,2,[],up=1), (1,2,[],up=1), (6,3,[],{1:0}?,up=1), (1,3,[],{1:0}?,up=1)],hasSemanticContext=true,conflictingAlts={1..3},dipsIntoOuterContext, input='x'\n", this.stderrDuringParse); } @@ -184,8 +184,8 @@ public class TestSemPredEvalParser extends BaseTest { "alt 2\n" + "alt 2\n"; assertEquals(expecting, found); - assertEquals("line 1:4 reportInsufficientPredicates d=0, decState=32, ambigAlts={2..4}:[{-1:-1}?, {-1:-1}?, {-1:-1}?, {-1:-1}?, {1:0}?], [(6,2,[],up=1), (10,2,[],up=1), (1,2,[],up=1), (6,3,[],up=1), (10,3,[],up=1), (1,3,[],up=1), (6,4,[],{1:0}?,up=1), (10,4,[],{1:0}?,up=1), (1,4,[],{1:0}?,up=1)],hasSemanticContext=true,conflictingAlts={2..4},dipsIntoOuterContext, input='x'\n", - this.stderrDuringParse); + assertEquals("line 1:4 reportAmbiguity d=0: ambigAlts={2..3}:[(6,2,[],up=1), (10,2,[],up=1), (1,2,[],up=1), (6,3,[],up=1), (10,3,[],up=1), (1,3,[],up=1), (6,4,[],{1:0}?,up=1), (10,4,[],{1:0}?,up=1), (1,4,[],{1:0}?,up=1)],hasSemanticContext=true,conflictingAlts={2..4},dipsIntoOuterContext, input='x'\n", + this.stderrDuringParse); } @Test public void testRewindBeforePredEval() throws Exception {