From b62408067ee86bc78c871c1e74afe4085488a853 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Thu, 28 Aug 2014 23:21:32 -0500 Subject: [PATCH] Update precedence filter to properly handle stepping out of left-recursive rules (fixes #679) --- .../org/antlr/v4/runtime/atn/ATNConfig.java | 54 ++++++++++++++++--- .../antlr/v4/runtime/atn/ATNConfigSet.java | 8 ++- .../antlr/v4/runtime/atn/ATNDeserializer.java | 10 +++- .../v4/runtime/atn/EpsilonTransition.java | 24 ++++++++- .../v4/runtime/atn/ParserATNSimulator.java | 44 ++++++++++----- 5 files changed, 119 insertions(+), 21 deletions(-) diff --git a/runtime/Java/src/org/antlr/v4/runtime/atn/ATNConfig.java b/runtime/Java/src/org/antlr/v4/runtime/atn/ATNConfig.java index 78fd88ec5..932864084 100644 --- a/runtime/Java/src/org/antlr/v4/runtime/atn/ATNConfig.java +++ b/runtime/Java/src/org/antlr/v4/runtime/atn/ATNConfig.java @@ -43,6 +43,13 @@ import org.antlr.v4.runtime.misc.Nullable; * an ATN state. */ public class ATNConfig { + /** + * This field stores the bit mask for implementing the + * {@link #isPrecedenceFilterSuppressed} property as a bit within the + * existing {@link #reachesIntoOuterContext} field. + */ + private static final int SUPPRESS_PRECEDENCE_FILTER = 0x40000000; + /** The ATN state associated with this configuration */ @NotNull public final ATNState state; @@ -64,9 +71,21 @@ public class ATNConfig { * dependent predicates unless we are in the rule that initially * invokes the ATN simulator. * - * closure() tracks the depth of how far we dip into the - * outer context: depth > 0. Note that it may not be totally - * accurate depth since I don't ever decrement. TODO: make it a boolean then + *

+ * closure() tracks the depth of how far we dip into the outer context: + * depth > 0. Note that it may not be totally accurate depth since I + * don't ever decrement. TODO: make it a boolean then

+ * + *

+ * For memory efficiency, the {@link #isPrecedenceFilterSuppressed} method + * is also backed by this field. Since the field is publicly accessible, the + * highest bit which would not cause the value to become negative is used to + * store this field. This choice minimizes the risk that code which only + * compares this value to 0 would be affected by the new purpose of the + * flag. It also ensures the performance of the existing {@link ATNConfig} + * constructors as well as certain operations like + * {@link ATNConfigSet#add(ATNConfig, DoubleKeyMap)} method are + * completely unaffected by the change.

*/ public int reachesIntoOuterContext; @@ -132,6 +151,28 @@ public class ATNConfig { this.reachesIntoOuterContext = c.reachesIntoOuterContext; } + /** + * This method gets the value of the {@link #reachesIntoOuterContext} field + * as it existed prior to the introduction of the + * {@link #isPrecedenceFilterSuppressed} method. + */ + public final int getOuterContextDepth() { + return reachesIntoOuterContext & ~SUPPRESS_PRECEDENCE_FILTER; + } + + public final boolean isPrecedenceFilterSuppressed() { + return (reachesIntoOuterContext & SUPPRESS_PRECEDENCE_FILTER) != 0; + } + + public final void setPrecedenceFilterSuppressed(boolean value) { + if (value) { + this.reachesIntoOuterContext |= 0x40000000; + } + else { + this.reachesIntoOuterContext &= ~SUPPRESS_PRECEDENCE_FILTER; + } + } + /** An ATN configuration is equal to another if both have * the same state, they predict the same alternative, and * syntactic/semantic contexts are the same. @@ -155,7 +196,8 @@ public class ATNConfig { return this.state.stateNumber==other.state.stateNumber && this.alt==other.alt && (this.context==other.context || (this.context != null && this.context.equals(other.context))) - && this.semanticContext.equals(other.semanticContext); + && this.semanticContext.equals(other.semanticContext) + && this.isPrecedenceFilterSuppressed() == other.isPrecedenceFilterSuppressed(); } @Override @@ -195,8 +237,8 @@ public class ATNConfig { buf.append(","); buf.append(semanticContext); } - if ( reachesIntoOuterContext>0 ) { - buf.append(",up=").append(reachesIntoOuterContext); + if ( getOuterContextDepth()>0 ) { + buf.append(",up=").append(getOuterContextDepth()); } buf.append(')'); return buf.toString(); diff --git a/runtime/Java/src/org/antlr/v4/runtime/atn/ATNConfigSet.java b/runtime/Java/src/org/antlr/v4/runtime/atn/ATNConfigSet.java index 6cdadcfee..938cebd29 100755 --- a/runtime/Java/src/org/antlr/v4/runtime/atn/ATNConfigSet.java +++ b/runtime/Java/src/org/antlr/v4/runtime/atn/ATNConfigSet.java @@ -161,7 +161,7 @@ public class ATNConfigSet implements Set { if ( config.semanticContext!=SemanticContext.NONE ) { hasSemanticContext = true; } - if (config.reachesIntoOuterContext > 0) { + if (config.getOuterContextDepth() > 0) { dipsIntoOuterContext = true; } ATNConfig existing = configLookup.getOrAdd(config); @@ -179,6 +179,12 @@ public class ATNConfigSet implements Set { // cache at both places. existing.reachesIntoOuterContext = Math.max(existing.reachesIntoOuterContext, config.reachesIntoOuterContext); + + // make sure to preserve the precedence filter suppression during the merge + if (config.isPrecedenceFilterSuppressed()) { + existing.setPrecedenceFilterSuppressed(true); + } + existing.context = merged; // replace context; no need to alt mapping return true; } diff --git a/runtime/Java/src/org/antlr/v4/runtime/atn/ATNDeserializer.java b/runtime/Java/src/org/antlr/v4/runtime/atn/ATNDeserializer.java index f1cb7193c..edb71ee51 100644 --- a/runtime/Java/src/org/antlr/v4/runtime/atn/ATNDeserializer.java +++ b/runtime/Java/src/org/antlr/v4/runtime/atn/ATNDeserializer.java @@ -320,7 +320,15 @@ public class ATNDeserializer { } RuleTransition ruleTransition = (RuleTransition)t; - atn.ruleToStopState[ruleTransition.target.ruleIndex].addTransition(new EpsilonTransition(ruleTransition.followState)); + int outermostPrecedenceReturn = -1; + if (atn.ruleToStartState[ruleTransition.target.ruleIndex].isPrecedenceRule) { + if (ruleTransition.precedence == 0) { + outermostPrecedenceReturn = ruleTransition.target.ruleIndex; + } + } + + EpsilonTransition returnTransition = new EpsilonTransition(ruleTransition.followState, outermostPrecedenceReturn); + atn.ruleToStopState[ruleTransition.target.ruleIndex].addTransition(returnTransition); } } diff --git a/runtime/Java/src/org/antlr/v4/runtime/atn/EpsilonTransition.java b/runtime/Java/src/org/antlr/v4/runtime/atn/EpsilonTransition.java index 546286ddd..d729c027e 100644 --- a/runtime/Java/src/org/antlr/v4/runtime/atn/EpsilonTransition.java +++ b/runtime/Java/src/org/antlr/v4/runtime/atn/EpsilonTransition.java @@ -33,7 +33,29 @@ package org.antlr.v4.runtime.atn; import org.antlr.v4.runtime.misc.NotNull; public final class EpsilonTransition extends Transition { - public EpsilonTransition(@NotNull ATNState target) { super(target); } + + private final int outermostPrecedenceReturn; + + public EpsilonTransition(@NotNull ATNState target) { + this(target, -1); + } + + public EpsilonTransition(@NotNull ATNState target, int outermostPrecedenceReturn) { + super(target); + this.outermostPrecedenceReturn = outermostPrecedenceReturn; + } + + /** + * @return the rule index of a precedence rule for which this transition is + * returning from, where the precedence value is 0; otherwise, -1. + * + * @see ATNConfig#isPrecedenceFilterSuppressed() + * @see ParserATNSimulator#applyPrecedenceFilter(ATNConfigSet) + * @since 4.4.1 + */ + public int outermostPrecedenceReturn() { + return outermostPrecedenceReturn; + } @Override public int getSerializationType() { 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 a363ffcbc..13d2e03de 100755 --- a/runtime/Java/src/org/antlr/v4/runtime/atn/ParserATNSimulator.java +++ b/runtime/Java/src/org/antlr/v4/runtime/atn/ParserATNSimulator.java @@ -316,6 +316,7 @@ public class ParserATNSimulator extends ATNSimulator { protected TokenStream _input; protected int _startIndex; protected ParserRuleContext _outerContext; + protected DFA _dfa; /** Testing only! */ public ParserATNSimulator(@NotNull ATN atn, @NotNull DFA[] decisionToDFA, @@ -360,6 +361,7 @@ public class ParserATNSimulator extends ATNSimulator { _startIndex = input.index(); _outerContext = outerContext; DFA dfa = decisionToDFA[decision]; + _dfa = dfa; int m = input.mark(); int index = _startIndex; @@ -426,6 +428,7 @@ public class ParserATNSimulator extends ATNSimulator { } finally { mergeCache = null; // wack cache after each prediction + _dfa = null; input.seek(index); input.release(m); } @@ -999,8 +1002,9 @@ public class ParserATNSimulator extends ATNSimulator { *
    *
  1. Evaluate the precedence predicates for each configuration using * {@link SemanticContext#evalPrecedence}.
    2. - *
  2. Remove all configurations which predict an alternative greater than - * 1, for which another configuration that predicts alternative 1 is in the + *
  3. When {@link ATNConfig#isPrecedenceFilterSuppressed} is {@code false}, + * remove all configurations which predict an alternative greater than 1, + * for which another configuration that predicts alternative 1 is in the * same ATN state with the same prediction context. This transformation is * valid for the following reasons: *
      @@ -1012,7 +1016,10 @@ public class ParserATNSimulator extends ATNSimulator { * epsilon transition, so the only way an alternative other than 1 can exist * in a state that is also reachable via alternative 1 is by nesting calls * to the left-recursive rule, with the outer calls not being at the - * preferred precedence level. + * preferred precedence level. The + * {@link ATNConfig#isPrecedenceFilterSuppressed} property marks ATN + * configurations which do not meet this condition, and therefore are not + * eligible for elimination during the filtering process. *
    *
    4. *
@@ -1076,14 +1083,16 @@ public class ParserATNSimulator extends ATNSimulator { continue; } - /* In the future, this elimination step could be updated to also - * filter the prediction context for alternatives predicting alt>1 - * (basically a graph subtraction algorithm). - */ - PredictionContext context = statesFromAlt1.get(config.state.stateNumber); - if (context != null && context.equals(config.context)) { - // eliminated - continue; + if (!config.isPrecedenceFilterSuppressed()) { + /* In the future, this elimination step could be updated to also + * filter the prediction context for alternatives predicting alt>1 + * (basically a graph subtraction algorithm). + */ + PredictionContext context = statesFromAlt1.get(config.state.stateNumber); + if (context != null && context.equals(config.context)) { + // eliminated + continue; + } } configSet.add(config, mergeCache); @@ -1240,7 +1249,7 @@ public class ParserATNSimulator extends ATNSimulator { protected int getAltThatFinishedDecisionEntryRule(ATNConfigSet configs) { IntervalSet alts = new IntervalSet(); for (ATNConfig c : configs) { - if ( c.reachesIntoOuterContext>0 || (c.state instanceof RuleStopState && c.context.hasEmptyPath()) ) { + if ( c.getOuterContextDepth()>0 || (c.state instanceof RuleStopState && c.context.hasEmptyPath()) ) { alts.add(c.alt); } } @@ -1409,6 +1418,10 @@ public class ParserATNSimulator extends ATNSimulator { // While we have context to pop back from, we may have // gotten that context AFTER having falling off a rule. // Make sure we track that we are now out of context. + // + // This assignment also propagates the + // isPrecedenceFilterSuppressed() value to the new + // configuration. c.reachesIntoOuterContext = config.reachesIntoOuterContext; assert depth > Integer.MIN_VALUE; closureCheckingStopState(c, configs, closureBusy, collectPredicates, @@ -1476,6 +1489,13 @@ public class ParserATNSimulator extends ATNSimulator { continue; } + if (_dfa != null && _dfa.isPrecedenceDfa()) { + int outermostPrecedenceReturn = ((EpsilonTransition)t).outermostPrecedenceReturn(); + if (outermostPrecedenceReturn == _dfa.atnStartState.ruleIndex) { + c.setPrecedenceFilterSuppressed(true); + } + } + c.reachesIntoOuterContext++; configs.dipsIntoOuterContext = true; // TODO: can remove? only care when we add to set per middle of this method assert newDepth > Integer.MIN_VALUE;