From 73968cb5f283e2906dcde721de89a3cec660abe7 Mon Sep 17 00:00:00 2001 From: Markus Schorn Date: Fri, 9 Nov 2012 11:58:55 +0100 Subject: [PATCH] Bug 393959: Unhandled template id ambiguity. --- .../parser/tests/ast2/AST2TemplateTests.java | 10 ++ .../parser/cpp/CPPASTTemplateIDAmbiguity.java | 20 +-- .../dom/parser/cpp/GNUCPPSourceParser.java | 163 +++++++++--------- .../parser/cpp/NameOrTemplateIDVariants.java | 82 +++++---- 4 files changed, 146 insertions(+), 129 deletions(-) diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2TemplateTests.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2TemplateTests.java index 36398f1d018..3646fcb57c3 100644 --- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2TemplateTests.java +++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2TemplateTests.java @@ -6181,4 +6181,14 @@ public class AST2TemplateTests extends AST2BaseTest { public void testAddressAsTemplateArgument_391190() throws Exception { parseAndCheckBindings(getAboveComment(), CPP, true); } + + // template struct CT { + // const static int const_min= 1; + // }; + // void test(int off) { + // off < CT::const_min || off > CT::const_min; + // } + public void testTemplateIDAmbiguity_393959() throws Exception { + parseAndCheckBindings(getAboveComment(), CPP, true); + } } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTTemplateIDAmbiguity.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTTemplateIDAmbiguity.java index 959973e4038..1d03067fe9e 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTTemplateIDAmbiguity.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/CPPASTTemplateIDAmbiguity.java @@ -17,7 +17,6 @@ import java.util.List; import org.eclipse.cdt.core.dom.ast.ASTVisitor; import org.eclipse.cdt.core.dom.ast.IASTExpression; -import org.eclipse.cdt.core.dom.ast.IASTInitializerClause; import org.eclipse.cdt.core.dom.ast.IASTName; import org.eclipse.cdt.core.dom.ast.IASTNode; import org.eclipse.cdt.core.dom.ast.IBinding; @@ -43,17 +42,14 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.NameOrTemplateIDVariants.Var */ public class CPPASTTemplateIDAmbiguity extends ASTAmbiguousNode implements IASTAmbiguousExpression, ICPPASTExpression { - private BinaryOperator fLastOperator; - private IASTInitializerClause fLastExpression; + private final BinaryOperator fEndOperator; private final BranchPoint fVariants; private IASTNode[] fNodes; private final AbstractGNUSourceCodeParser fParser; - public CPPASTTemplateIDAmbiguity(AbstractGNUSourceCodeParser parser, BinaryOperator lastOperator, IASTInitializerClause expr, - BranchPoint variants) { + public CPPASTTemplateIDAmbiguity(AbstractGNUSourceCodeParser parser, BinaryOperator endOperator, BranchPoint variants) { fParser= parser; - fLastOperator= lastOperator; - fLastExpression= expr; + fEndOperator= endOperator; fVariants= variants; } @@ -92,10 +88,7 @@ public class CPPASTTemplateIDAmbiguity extends ASTAmbiguousNode implements IASTA if (selected != null) { minOffset= selected.getRightOffset(); BinaryOperator targetOp = selected.getTargetOperator(); - if (targetOp == null) { - fLastExpression= selected.getExpression(); - fLastOperator= v.getLeftOperator(); - } else { + if (targetOp != null) { targetOp.exchange(selected.getExpression()); targetOp.setNext(v.getLeftOperator()); } @@ -106,7 +99,7 @@ public class CPPASTTemplateIDAmbiguity extends ASTAmbiguousNode implements IASTA owner.replace(nodeToReplace, this); // Create the expression and replace it - IASTExpression expr = fParser.buildExpression(fLastOperator, fLastExpression); + IASTExpression expr = fParser.buildExpression(fEndOperator.getNext(), fEndOperator.getExpression()); owner.replace(this, expr); // Resolve further ambiguities within the new expression. @@ -149,8 +142,7 @@ public class CPPASTTemplateIDAmbiguity extends ASTAmbiguousNode implements IASTA public IASTNode[] getNodes() { if (fNodes == null) { List nl= new ArrayList(); - nl.add(fLastExpression); - BinaryOperator op= fLastOperator; + BinaryOperator op= fEndOperator; while (op != null) { nl.add(op.getExpression()); op= op.getNext(); diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/GNUCPPSourceParser.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/GNUCPPSourceParser.java index 5cc638281ce..606c748ec0f 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/GNUCPPSourceParser.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/GNUCPPSourceParser.java @@ -764,6 +764,7 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { BinaryOperator lastOperator= null; NameOrTemplateIDVariants variants= null; + IToken variantMark= mark(); if (expr == null) { Object e = castExpressionForBinaryExpression(strat); if (e instanceof IASTExpression) { @@ -773,20 +774,21 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { final Variant variant = (Variant) e; expr= variant.getExpression(); - variants.addBranchPoint(variant.getNext(), lastOperator, allowAssignment, conditionCount); + variants.addBranchPoint(variant.getNext(), null, allowAssignment, conditionCount); } } - boolean doneExpression= false; - do { + boolean stopWithNextOperator= false; + castExprLoop: for(;;) { // Typically after a binary operator there cannot be a throw expression boolean allowThrow= false; // Brace initializers are allowed on the right hand side of an expression boolean allowBraceInitializer= false; - BacktrackException tryRecovery= null; - final int operatorOffset= LA().getOffset(); - lt1= LT(1); + boolean doneExpression= false; + BacktrackException failure= null; + final int opOffset= LA().getOffset(); + lt1= stopWithNextOperator ? IToken.tSEMI : LT(1); switch (lt1) { case IToken.tQUESTION: conditionCount++; @@ -879,7 +881,7 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { if (LT(2) != IToken.tGT_in_SHIFTR) { IToken token = LA(1); backtrack.initialize(token.getOffset(), token.getLength()); - tryRecovery= backtrack; + failure= backtrack; break; } @@ -907,35 +909,46 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { doneExpression= true; break; } - - if (!doneExpression && tryRecovery == null) { - consume(); // consumes the operator - - // Link variants that are closed by the new operator - if (variants != null) { - variants.closeVariants(operatorOffset, lastOperator); + + // Close variants + if (failure == null) { + if (doneExpression) { + if (variants != null && !variants.hasRightBound(opOffset)) { + // We have a longer variant, ignore this one. + backtrack.initialize(opOffset, 1); + failure= backtrack; + } else { + break castExprLoop; + } + } + // Close variants with matching end + if (variants != null && lastOperator != null) { + variants.closeVariants(opOffset, lastOperator); } + } - // Determine next sub-expression - if (lt1 == IToken.tQUESTION && LT(1) == IToken.tCOLON) { - // Missing sub-expression after '?' (gnu-extension) - expr= null; - } else if (allowThrow && LT(1) == IToken.t_throw) { - // Throw expression - expr= throwExpression(); - lt1= LT(1); - if (lt1 != IToken.tCOLON && lt1 != IToken.tCOMMA) + if (failure == null && !doneExpression) { + // Determine next cast-expression + consume(); // consumes the operator + stopWithNextOperator= false; + try { + if (lt1 == IToken.tQUESTION && LT(1) == IToken.tCOLON) { + // Missing sub-expression after '?' (gnu-extension) + expr= null; + } else if (allowThrow && LT(1) == IToken.t_throw) { + // Throw expression + expr= throwExpression(); + lt1= LT(1); + if (lt1 != IToken.tCOLON && lt1 != IToken.tCOMMA) + stopWithNextOperator= true; break; - } else if (allowBraceInitializer && LT(1) == IToken.tLBRACE) { - // Brace initializer - expr= bracedInitList(true); - lt1= LT(1); - if (lt1 != IToken.tCOLON && lt1 != IToken.tCOMMA) - break; - } else { - // Cast expression - IToken m= mark(); - try { + } else if (allowBraceInitializer && LT(1) == IToken.tLBRACE) { + // Brace initializer + expr= bracedInitList(true); + lt1= LT(1); + if (lt1 != IToken.tCOLON && lt1 != IToken.tCOMMA) + stopWithNextOperator= true; + } else { Object e = castExpressionForBinaryExpression(strat); if (e instanceof IASTExpression) { expr= (IASTExpression) e; @@ -947,55 +960,49 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { variants.addBranchPoint(ae.getNext(), lastOperator, allowAssignment, conditionCount); } - } catch (BacktrackException e) { - if (variants == null) - throw e; - tryRecovery= e; - backup(m); } + continue castExprLoop; + } catch (BacktrackException e) { + failure= e; } - } + } - if (tryRecovery != null || doneExpression) { - if (variants != null) { - if (lt1 == IToken.tEOC) { - variants.discardOpenVariants(operatorOffset); - } else { - // Try fall-back to an open variant - Variant fallback= variants.findFallback(operatorOffset); - if (fallback == null) { - if (tryRecovery != null) - throw tryRecovery; - variants.discardOpenVariants(operatorOffset); - } else { - // Restore state and continue - doneExpression= false; - BranchPoint varPoint= fallback.getOwner(); - allowAssignment= varPoint.isAllowAssignment(); - conditionCount= varPoint.getConditionCount(); - lastOperator= varPoint.getLeftOperator(); - expr= fallback.getExpression(); - variants.useFallback(fallback); + // We need a new variant + Variant variant= variants == null ? null : variants.selectFallback(); + if (variant == null) { + if (failure != null) + throw failure; + throwBacktrack(LA(1)); + } else { + // Restore variant and continue + BranchPoint varPoint= variant.getOwner(); + allowAssignment= varPoint.isAllowAssignment(); + conditionCount= varPoint.getConditionCount(); + lastOperator= varPoint.getLeftOperator(); + expr= variant.getExpression(); - // Advance to the right token - int offset= fallback.getRightOffset(); - while (LA().getOffset() < offset) { - consume(); - } - } - } + backup(variantMark); + int offset= variant.getRightOffset(); + while (LA().getOffset() < offset) { + consume(); } + variantMark= mark(); } - } while (!doneExpression); + } // Check for incomplete conditional expression if (lt1 != IToken.tEOC && conditionCount > 0) throwBacktrack(LA(1)); - - if (variants != null && !variants.isEmpty()) { - CPPASTTemplateIDAmbiguity result = new CPPASTTemplateIDAmbiguity(this, lastOperator, expr, variants.getOrderedBranchPoints()); - setRange(result, startOffset, calculateEndOffset(expr)); - return result; + + if (variants != null) { + BinaryOperator end = new BinaryOperator(lastOperator, expr, -1, 0, 0); + variants.closeVariants(LA(1).getOffset(), end); + variants.removeInvalid(end); + if (!variants.isEmpty()) { + CPPASTTemplateIDAmbiguity result = new CPPASTTemplateIDAmbiguity(this, end, variants.getOrderedBranchPoints()); + setRange(result, startOffset, calculateEndOffset(expr)); + return result; + } } return buildExpression(lastOperator, expr); @@ -1009,7 +1016,7 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { TemplateIdStrategy strat= new TemplateIdStrategy(); Variant variants= null; - IASTExpression singleExpression= null; + IASTExpression singleResult= null; IASTName[] firstNames= null; final IToken mark= mark(); @@ -1018,12 +1025,12 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { try { IASTExpression e = castExpression(CastExprCtx.eDirectlyInBExpr, strat); if (variants == null) { - if (singleExpression == null || lastToken == null) { - singleExpression= e; + if (singleResult == null || lastToken == null) { + singleResult= e; firstNames= strat.getTemplateNames(); } else { - variants= new Variant(null, singleExpression, firstNames, lastToken.getOffset()); - singleExpression= null; + variants= new Variant(null, singleResult, firstNames, lastToken.getOffset()); + singleResult= null; firstNames= null; } } @@ -1045,7 +1052,7 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { } backup(mark); } - return variants != null ? variants : singleExpression; + return variants != null ? variants : singleResult; } @Override diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/NameOrTemplateIDVariants.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/NameOrTemplateIDVariants.java index 6bc530b875d..489d610c054 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/NameOrTemplateIDVariants.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/NameOrTemplateIDVariants.java @@ -13,7 +13,6 @@ package org.eclipse.cdt.internal.core.dom.parser.cpp; import org.eclipse.cdt.core.dom.ast.IASTExpression; import org.eclipse.cdt.core.dom.ast.IASTName; -import org.eclipse.cdt.internal.core.dom.parser.ASTNode; import org.eclipse.cdt.internal.core.dom.parser.AbstractGNUSourceCodeParser.BinaryOperator; /** @@ -132,59 +131,31 @@ public class NameOrTemplateIDVariants { if (v.getTargetOperator() == null) { if (offset == v.getRightOffset()) { v.setTargetOperator(lastOperator); - } else if (offset > v.getRightOffset()) { - // Should not happen - assert false; - remove(v); - } + } } } } } - public void discardOpenVariants(int operatorOffset) { - for (BranchPoint p = fFirst; p != null; p= p.getNext()) { - for (Variant v= p.getFirstVariant(); v != null; v= v.getNext()) { - if (v.getTargetOperator() == null && v.getRightOffset() != operatorOffset) { - remove(v); - } - } - } - } - - public Variant findFallback(int operatorOffset) { + public Variant selectFallback() { // Search for an open variant, with a small right offset and a large left offset - Variant best= null; for (BranchPoint p = fFirst; p != null; p= p.getNext()) { + Variant best= null; for (Variant v= p.getFirstVariant(); v != null; v= v.getNext()) { - if (v.fRightOffset > operatorOffset) { + if (v.getTargetOperator() == null) { if (best == null || v.fRightOffset < best.fRightOffset) { best= v; } } } - } - return best; - } - - public void useFallback(Variant fallback) { - // Discard variants that end within the fallback - int begin= ((ASTNode) fallback.getExpression()).getOffset(); - int end= fallback.getRightOffset(); - for (BranchPoint p = fFirst; p != null; p= p.getNext()) { - for (Variant v= p.getFirstVariant(); v != null; v= v.getNext()) { - if (v == fallback) { - remove(v); - } else { - int vend= v.getRightOffset(); - if (vend > begin && vend < end) - remove(v); - } + if (best != null) { + remove(best); + return best; } } + return null; } - private void remove(Variant remove) { final BranchPoint owner = remove.fOwner; final Variant next = remove.getNext(); @@ -236,4 +207,41 @@ public class NameOrTemplateIDVariants { fFirst= null; return prev; } + + public boolean hasRightBound(int opOffset) { + // Search for an open variant, with a small right offset and a large left offset + for (BranchPoint p = fFirst; p != null; p= p.getNext()) { + for (Variant v= p.getFirstVariant(); v != null; v= v.getNext()) { + if (v.fRightOffset > opOffset) + return false; + } + } + return true; + } + + public void removeInvalid(BinaryOperator lastOperator) { + for (BranchPoint p = fFirst; p != null; p= p.getNext()) { + if (!isReachable(p, lastOperator)) { + remove(p); + } else { + for (Variant v= p.getFirstVariant(); v != null; v= v.getNext()) { + if (v.getTargetOperator() == null) { + remove(v); + } + } + } + } + } + + private boolean isReachable(BranchPoint bp, BinaryOperator endOperator) { + BinaryOperator op = bp.getLeftOperator(); + if (op == null) + return true; + + for(; endOperator != null; endOperator= endOperator.getNext()) { + if (endOperator == op) + return true; + } + return false; + } }