diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/includes/BindingClassifierTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/includes/BindingClassifierTest.java index c81e0433be6..7cb72e241bf 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/includes/BindingClassifierTest.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/includes/BindingClassifierTest.java @@ -168,12 +168,15 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { // struct A { void m(); }; // class B : public A {}; // B b; + // class C : public A {}; + // C c; // void test() { // b.m(); + // c->m(); // } public void testClassHierarchy() throws Exception { - assertDefined("b", "B"); + assertDefined("b", "B", "c", "C"); assertDeclared(); } @@ -472,11 +475,53 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { // shared_ptr x; // unique_ptr y; // } - public void testTemplatesAllowingIncompleteParameterType() throws Exception { + public void testTemplatesAllowingIncompleteParameterType_1() throws Exception { assertDefined("B", "C", "shared_ptr", "unique_ptr"); assertDeclared("A"); } + // namespace std { + // template + // struct unique_ptr { + // T* operator->(); + // }; + // } + // struct A { + // void m(); + // }; + // class B : public A { + // }; + // std::unique_ptr b; + + // void test() { + // b->m(); + // } + public void testTemplatesAllowingIncompleteParameterType_2() throws Exception { + assertDefined("B", "b"); + assertDeclared(); + } + + // namespace std { + // template + // struct shared_ptr { + // T* operator->(); + // }; + // } + // struct A { + // void m(); + // }; + // class B : public A { + // }; + // std::shared_ptr f(); + + // void test() { + // f()->m(); + // } + public void testTemplatesAllowingIncompleteParameterType_3() throws Exception { + assertDefined("B", "f"); + assertDeclared(); + } + // struct A {}; // struct B {}; // struct C {}; diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/BindingClassifier.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/BindingClassifier.java index e41b14ffdfb..9fef7370c4a 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/BindingClassifier.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/BindingClassifier.java @@ -18,11 +18,10 @@ import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUti import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil.REF; import static org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil.getNestedType; -import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashSet; -import java.util.List; import java.util.Set; import org.eclipse.core.runtime.CoreException; @@ -105,6 +104,7 @@ import org.eclipse.cdt.core.dom.ast.cpp.ICPPReferenceType; import org.eclipse.cdt.core.dom.ast.cpp.ICPPSpecialization; import org.eclipse.cdt.core.dom.ast.cpp.ICPPTemplateArgument; import org.eclipse.cdt.core.dom.ast.cpp.ICPPTemplateDefinition; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPTemplateInstance; import org.eclipse.cdt.core.dom.ast.cpp.ICPPTemplateParameterMap; import org.eclipse.cdt.core.index.IIndexMacro; import org.eclipse.cdt.core.index.IndexFilter; @@ -305,8 +305,8 @@ public class BindingClassifier { * @return The binding(s) which is/are suitable for either declaration or definition, * or an empty list if no such binding is available. */ - private List getRequiredBindings(IBinding binding) { - List bindings = new ArrayList(); + private Set getRequiredBindings(IBinding binding) { + Set bindings = new HashSet(); if (binding instanceof ICPPMember) { // If the binding is a member, get its owning composite type. @@ -401,7 +401,7 @@ public class BindingClassifier { return; } - List requiredBindings = getRequiredBindings(binding); + Collection requiredBindings = getRequiredBindings(binding); for (IBinding requiredBinding : requiredBindings) { if (fBindingsToDeclare.contains(requiredBinding) || fBindingsToDefine.contains(requiredBinding)) { @@ -431,14 +431,14 @@ public class BindingClassifier { } /** - * Checks whether the binding can be forward declared or not. + * Checks whether the binding can be forward declared or not according to preferences. */ private boolean canForwardDeclare(IBinding binding) { boolean canDeclare = false; if (binding instanceof ICompositeType) { canDeclare = fPreferences.forwardDeclareCompositeTypes; } else if (binding instanceof IEnumeration) { - canDeclare = fPreferences.forwardDeclareEnums; + canDeclare = fPreferences.forwardDeclareEnums && isEnumerationWithoutFixedUnderlyingType(binding); } else if (binding instanceof IFunction && !(binding instanceof ICPPMethod)) { canDeclare = fPreferences.forwardDeclareFunctions; } else if (binding instanceof IVariable) { @@ -453,6 +453,22 @@ public class BindingClassifier { return canDeclare; } + /** + * Checks whether the type may be forward declared according to language rules. + */ + private boolean mayBeForwardDeclared(IType type) { + IBinding binding = getTypeBinding(type); + if (binding == null) + return false; + if (!fPreferences.assumeTemplatesMayBeForwardDeclared && + (binding instanceof ICPPTemplateDefinition || binding instanceof ICPPSpecialization)) { + return false; + } + if (binding instanceof ICompositeType) + return true; + return binding instanceof ICPPEnumeration && ((ICPPEnumeration) binding).getFixedType() != null; + } + /** * Adds the given type to the list of bindings which have to be defined. Typedefs and * enumerations without fixed underlying type are skipped since they must be defined in the file @@ -481,7 +497,7 @@ public class BindingClassifier { if (fAst.getDefinitionsInAST(binding).length != 0) return; // Defined locally. - List requiredBindings = getRequiredBindings(binding); + Collection requiredBindings = getRequiredBindings(binding); for (IBinding requiredBinding : requiredBindings) { fBindingsToDeclare.remove(requiredBinding); if (requiredBinding == binding) { @@ -540,22 +556,37 @@ public class BindingClassifier { // Handle return or expression type of the function or constructor call. IType returnType = function.getType().getReturnType(); - if (!(returnType instanceof IPointerType) && !(returnType instanceof ICPPReferenceType)) { - // The return type needs a full definition. - defineTypeExceptTypedefOrNonFixedEnum(returnType); - } + defineTypeOfBinding(function, returnType); // Handle parameters. processFunctionParameters(function, arguments); } + private void defineTypeOfBinding(IBinding binding, IType type) { + if (fBindingsToDefine.contains(binding) && !mayBeForwardDeclared(type)) { + if (type instanceof ICPPTemplateInstance) { + ICPPTemplateInstance instance = (ICPPTemplateInstance) type; + IBinding template = instance.getSpecializedBinding(); + if (templatesAllowingIncompleteArgumentType.contains(template.getName())) { + ICPPTemplateArgument[] arguments = instance.getTemplateArguments(); + if (arguments.length != 0) { + IType argumentType = arguments[0].getTypeValue(); + defineTypeExceptTypedefOrNonFixedEnum(argumentType); + } + } + } + } else if (!(type instanceof IPointerType) && !(type instanceof ICPPReferenceType)) { + defineTypeExceptTypedefOrNonFixedEnum(type); + } + } + private class BindingCollector extends ASTVisitor { BindingCollector() { super(true); } @Override - public int visit(IASTDeclaration declaration) { + public int leave(IASTDeclaration declaration) { if (declaration instanceof IASTSimpleDeclaration) { /* * The type specifier of a simple declaration of a variable must always be defined @@ -614,7 +645,7 @@ public class BindingClassifier { } @Override - public int visit(IASTDeclarator declarator) { + public int leave(IASTDeclarator declarator) { if (declarator instanceof IASTFunctionDeclarator) { /* * The type specifier of a function definition doesn't need to be defined if it is @@ -668,7 +699,7 @@ public class BindingClassifier { } @Override - public int visit(IASTDeclSpecifier declSpec) { + public int leave(IASTDeclSpecifier declSpec) { if (declSpec instanceof IASTElaboratedTypeSpecifier) { /* * The type specifier of an elaborated type neither needs to be defined nor needs to be @@ -684,7 +715,7 @@ public class BindingClassifier { } @Override - public int visit(ICPPASTBaseSpecifier baseSpecifier) { + public int leave(ICPPASTBaseSpecifier baseSpecifier) { /* * The type of a base specifier must always be defined. * @@ -696,7 +727,7 @@ public class BindingClassifier { } @Override - public int visit(IASTInitializer initializer) { + public int leave(IASTInitializer initializer) { /* * The type of the member of a constructor chain initializer doesn't need to be defined * if it is a pointer or reference type and if the argument type matches. @@ -799,7 +830,7 @@ public class BindingClassifier { } @Override - public int visit(IASTStatement statement) { + public int leave(IASTStatement statement) { if (statement instanceof IASTReturnStatement) { /* * The type of the return value expression doesn't need to be defined if the actual @@ -869,7 +900,7 @@ public class BindingClassifier { } @Override - public int visit(IASTExpression expression) { + public int leave(IASTExpression expression) { ASTNodeProperty propertyInParent = expression.getPropertyInParent(); if (propertyInParent == IASTIfStatement.CONDITION || propertyInParent == IASTForStatement.CONDITION @@ -906,10 +937,8 @@ public class BindingClassifier { IBinding binding = idExpression.getName().resolveBinding(); if (binding instanceof IVariable) { // Get the declared type. - IType expressionType = ((IVariable) binding).getType(); - if (!(expressionType instanceof IPointerType) && !(expressionType instanceof ICPPReferenceType)) { - defineTypeExceptTypedefOrNonFixedEnum(expressionType); - } + IType variableType = ((IVariable) binding).getType(); + defineTypeOfBinding(binding, variableType); } } else if (expression instanceof IASTUnaryExpression) { /* @@ -1104,7 +1133,14 @@ public class BindingClassifier { * } */ - defineTypeExceptTypedefOrNonFixedEnum(((IASTFieldReference) expression).getFieldOwner().getExpressionType()); + IASTExpression fieldOwner = ((IASTFieldReference) expression).getFieldOwner(); + IType expressionType = fieldOwner.getExpressionType(); + if (!(fieldOwner instanceof IASTIdExpression || fieldOwner instanceof IASTFunctionCallExpression) || + expressionType instanceof IPointerType || expressionType instanceof ICPPReferenceType) { + // Id expressions and function call expressions returning non pointer and + // non reference types are processed separately. + defineTypeExceptTypedefOrNonFixedEnum(expressionType); + } } else if (expression instanceof ICPPASTNewExpression) { /* * The type specifier of a "new" expression always requires a definition. @@ -1153,7 +1189,7 @@ public class BindingClassifier { } @Override - public int visit(IASTName name) { + public int leave(IASTName name) { if (isPartOfExternalMacroDefinition(name)) return PROCESS_CONTINUE; @@ -1188,7 +1224,7 @@ public class BindingClassifier { } @Override - public int visit(IASTTranslationUnit tu) { + public int leave(IASTTranslationUnit tu) { for (IASTPreprocessorMacroExpansion macroExpansion : tu.getMacroExpansions()) { IASTPreprocessorMacroDefinition macroDefinition = macroExpansion.getMacroDefinition(); IASTName name = macroDefinition.getName(); diff --git a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludePreferences.java b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludePreferences.java index fa2ce41bf5c..6da4416bfa2 100644 --- a/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludePreferences.java +++ b/core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/refactoring/includes/IncludePreferences.java @@ -45,6 +45,7 @@ public class IncludePreferences implements Comparator { public final boolean forwardDeclareExternalVariables; public final boolean forwardDeclareTemplates; public final boolean forwardDeclareNamespaceElements; + public final boolean assumeTemplatesMayBeForwardDeclared; public final UnusedStatementDisposition unusedStatementsDisposition; public final String[] partnerFileSuffixes; @@ -84,6 +85,11 @@ public class IncludePreferences implements Comparator { forwardDeclareNamespaceElements = PreferenceConstants.getPreference( PreferenceConstants.FORWARD_DECLARE_NAMESPACE_ELEMENTS, project, true); + // Although templates may be forward declared, it is done so rarely that we assume that it + // never happens. + // TODO(sprigogin): Create a preference for this. + assumeTemplatesMayBeForwardDeclared = false; + String value = PreferenceConstants.getPreference( PreferenceConstants.INCLUDES_PARTNER_FILE_SUFFIXES, project, DEFAULT_PARTNER_FILE_SUFFIXES); partnerFileSuffixes = value.split(","); //$NON-NLS-1$