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 8ce664366d0..33af5c62ce0 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 @@ -6086,6 +6086,13 @@ public class AST2TemplateTests extends AST2TestBase { public void testTemplateAmbiguityInDeleteExpression_364225() throws Exception { parseAndCheckBindings(); } + + // template struct a {}; + // const int b = 0, c = 1; + // int a::*mp6; // syntax error here + public void testTemplateIDAmbiguity_445177() throws Exception { + parseAndCheckBindings(); + } // template void foo(T); // template void foo(T, typename T::type* = 0); diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/AbstractGNUSourceCodeParser.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/AbstractGNUSourceCodeParser.java index d9b17cd83ec..0924eacc4dc 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/AbstractGNUSourceCodeParser.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/AbstractGNUSourceCodeParser.java @@ -99,6 +99,7 @@ import org.eclipse.cdt.internal.core.parser.scanner.ILocationResolver; * Base class for the c- and c++ parser. */ public abstract class AbstractGNUSourceCodeParser implements ISourceCodeParser { + // see TemplateIdStrategy for an explanation of what this does public interface ITemplateIdStrategy { boolean shallParseAsTemplateID(IASTName name); } 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 d80e7421d13..257647d3413 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 @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2002, 2014 IBM Corporation and others. + * Copyright (c) 2002, 2015 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -15,6 +15,7 @@ * Sergey Prigogin (Google) * Thomas Corbat (IFS) * Anders Dahlberg (Ericsson) - bug 84144 + * Nathan Ridge *******************************************************************************/ package org.eclipse.cdt.internal.core.dom.parser.cpp; @@ -261,7 +262,7 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { try { return nameSpecifier(ctx, strat); } catch (BacktrackException e) { - if (strat.setNextAlternative()) { + if (strat.setNextAlternative(true /* previous alternative failed to parse */)) { backup(m); } else { throw e; @@ -342,14 +343,14 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { final int haveArgs = haveTemplateArguments(inBinaryExpression); boolean templateID= true; if (!keywordTemplate) { - if (haveArgs == -1) { + if (haveArgs == NO_TEMPLATE_ID) { templateID= false; - } else if (haveArgs == 0) { + } else if (haveArgs == AMBIGUOUS_TEMPLATE_ID) { templateID= strat.shallParseAsTemplateID(name); } } if (templateID) { - if (haveArgs == -1) + if (haveArgs == NO_TEMPLATE_ID) throwBacktrack(LA(1)); nameSpec= (ICPPASTName) addTemplateArguments(name, strat); @@ -560,7 +561,7 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { break; } } - return 0; + return AMBIGUOUS_TEMPLATE_ID; } finally { backup(mark); } @@ -1156,11 +1157,11 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { if (variants != null) { variants = new Variant(variants, e, strat.getTemplateNames(), lastToken.getOffset()); } - if (!strat.setNextAlternative()) { + if (!strat.setNextAlternative(false /* previous alternative parsed ok */)) { break; } } catch (BacktrackException e) { - if (!strat.setNextAlternative()) { + if (!strat.setNextAlternative(true /* previous alternative failed to parse */)) { if (lastToken == null) throw e; @@ -1839,6 +1840,11 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { case IToken.tBITCOMPLEMENT: case IToken.t_decltype: { IASTName name = qualifiedName(ctx, strat); + // A qualified-name's last name can sometimes be empty in a declaration (e.g. in "int A::*x", + // "A::" is a valid qualified-name with an empty last name), but not in an expression + // (unless we are invoking code completion at the '::'). + if (name.getLookupKey().length == 0 && LT(1) != IToken.tEOC) + throwBacktrack(LA(1)); IASTIdExpression idExpression = nodeFactory.newIdExpression(name); return setRange(idExpression, name); } @@ -2228,7 +2234,7 @@ public class GNUCPPSourceParser extends AbstractGNUSourceCodeParser { try { return templateParameterList(result); } catch (BacktrackException e) { - if (!fTemplateParameterListStrategy.setNextAlternative()) { + if (!fTemplateParameterListStrategy.setNextAlternative(true /* previous alternative failed to parse */)) { fTemplateParameterListStrategy= null; throw e; } diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/TemplateIdStrategy.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/TemplateIdStrategy.java index efa6ef4908b..28eaeb970c5 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/TemplateIdStrategy.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/TemplateIdStrategy.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2011 Wind River Systems, Inc. and others. + * Copyright (c) 2011, 2015 Wind River Systems, Inc. and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -7,6 +7,7 @@ * * Contributors: * Markus Schorn - initial API and implementation + * Nathan Ridge - added comments and fixed bug 445177 *******************************************************************************/ package org.eclipse.cdt.internal.core.dom.parser.cpp; @@ -20,23 +21,53 @@ import org.eclipse.cdt.core.parser.util.ArrayUtil; import org.eclipse.cdt.internal.core.dom.parser.AbstractGNUSourceCodeParser.ITemplateIdStrategy; /** - * Governs backtracking through multiple variants due to the ambiguous meaning of '<'. - * @see NameOrTemplateIDVariants + * This class is used to track alternatives for parsing segments of code that involve '<' tokens. + * + * The '<' token can be either a less-than operator or part of a template-id. + * When parsing, we potentially need to consider both possibilities for each use of '<'. + * + * An instance of this class is used to track alternative parses in a segment of code that includes one or + * more uses of '<' preceded by names. An alternative consists of a choices (template-id or not) for each + * name. At a given point in time, the instance has a notion of a current alternative, and a current + * position within that alternative. + * + * @see also NameOrTemplateIDVariants, which is used together with this class to deal with ambiguities + * involving '<' when parsing in an expression context. */ final class TemplateIdStrategy implements ITemplateIdStrategy { + // The current position in the current alternative. + // The next call to shallParseAsTemplateID() will return whether in the current alternative, + // the name at index (fCurrentBranchPoint + 1) should be parsed as a template-id. private int fCurrentBranchPoint; + + // The current alternative, represented as a bitset with one bit for each name. + // A bit corresponding to a name is clear if the name should be parsed as a template-id, and set if + // it should not. + // For the first alternative, this bitset is null, and this is interpreted as all zeros (i.e. every name + // is parsed as a template-id). private BitSet fSimpleIDs; + + // The set of names which are parsed as template-ids in the current alternative. private IASTName[] fTemplateNames; public TemplateIdStrategy() { fCurrentBranchPoint= -1; fTemplateNames= IASTName.EMPTY_NAME_ARRAY; } - + + /** + * Returns whether 'name' should be parsed as a template-id according to the current alternative. + * For each alternative, this is expected to be called once for each name in a segment code for which + * this choice is ambiguous. + * For the first alternative, these calls are used to initially populate the list of names. + * For subsequent alternatives, these calls are expected for the same names in the same order. + */ @Override public boolean shallParseAsTemplateID(IASTName name) { fCurrentBranchPoint++; + // 'fSimpleIDs == null' means we're on the first alternative. + // On the first alternative, everything is parsed as a template-id. boolean templateID= fSimpleIDs == null || !fSimpleIDs.get(fCurrentBranchPoint); if (templateID) { fTemplateNames= ArrayUtil.append(fTemplateNames, name); @@ -44,23 +75,52 @@ final class TemplateIdStrategy implements ITemplateIdStrategy { return templateID; } - public boolean setNextAlternative() { - int bp = fCurrentBranchPoint; - if (bp < 0) + /** + * Advance to the next alternative parse, or return false is there is none. + * After a call to this, the current position is reset to the beginning of the (next) alternative. + * @param previousAlternativeFailedToParse whether this function is being called because the previous + * alternative failed to parse + */ + public boolean setNextAlternative(boolean previousAlternativeFailedToParse) { + // No one has called shallParseAsTemplateID() for the current alternative, so there are no + // ambiguous names. Therefore, there are no more alternatives. + if (fCurrentBranchPoint < 0) { return false; + } + // Reset the current position, saving the old one, which should point to the last name in the + // bitset for which parsing was attempted during the previous alternative. + int bp = fCurrentBranchPoint; fCurrentBranchPoint= -1; + + // Reset the list of names that were parsed as template-ids, saving the list for the previous + // alternative. IASTName[] names = getTemplateNames(); + // Note that 'names' here contains the list of names for which there is a '0' in the bitset. int nameLen= names.length; fTemplateNames= IASTName.EMPTY_NAME_ARRAY; + + // If the previous alternative was the first, the bitset is still null. Create it. if (fSimpleIDs == null) { fSimpleIDs= new BitSet(); } - // Set a new branch as far right as possible. + // Advance to the next alternative by finding the right-most '0' in the bitset, and setting it to '1', + // and bits to the right of it to '0'. In this way, successive calls to this function will iterate + // over all possible alternatives. + // Note that in searching for the right-most '0', we start at 'bp', not the last element of the + // bitset. The reason is that if 'bp' is not the last element of the bitset, it means that during the + // previous alternative, we failed the parse before getting beyond 'bp'. This means that there is a + // problem with one of the choices up to and including 'bp', so there's no point in trying another + // alternatives that keeps these choices the same. while (bp >= 0) { if (!fSimpleIDs.get(bp)) { - if (nameLen == 0 || !hasMultipleArgs(names[--nameLen])) { + // Optimization (bug 363609): if during the previous alternative, a name was parsed as a + // template-id with multiple template arguments, it's not going to be parsed differently in + // a subsequent alternative, so keep it as a template-id. + // Of course, this optimization is only sound if the previous alternative was parsed + // successfully (bug 445177). + if (previousAlternativeFailedToParse || nameLen == 0 || !hasMultipleArgs(names[--nameLen])) { fSimpleIDs.clear(bp+1, Integer.MAX_VALUE); fSimpleIDs.set(bp); return true; @@ -68,6 +128,8 @@ final class TemplateIdStrategy implements ITemplateIdStrategy { } bp--; } + + // The bitset was all ones - there are no more alternatives. return false; }