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 fe4f9f0fde5..c8082663ae6 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 @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2012, 2013 Google, Inc and others. + * Copyright (c) 2012, 2015 Google, 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 @@ -23,8 +23,10 @@ import org.eclipse.jface.preference.IPreferenceStore; import com.ibm.icu.text.MessageFormat; import org.eclipse.cdt.core.CCorePlugin; +import org.eclipse.cdt.core.dom.ast.DOMException; import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit; import org.eclipse.cdt.core.dom.ast.IBinding; +import org.eclipse.cdt.core.dom.ast.cpp.ICPPBinding; import org.eclipse.cdt.core.index.IIndex; import org.eclipse.cdt.core.index.IIndexManager; import org.eclipse.cdt.core.model.ITranslationUnit; @@ -85,28 +87,29 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { return CUIPlugin.getDefault().getPreferenceStore(); } - private void assertDefined(String... names) { + private void assertDefined(String... names) throws Exception { classifyBindings(); assertExpectedBindings(names, fBindingClassifier.getBindingsToDefine(), "defined"); } - private void assertDeclared(String... names) { + private void assertDeclared(String... names) throws Exception { classifyBindings(); assertExpectedBindings(names, fBindingClassifier.getBindingsToForwardDeclare(), "declared"); } - private void assertExpectedBindings(String[] expectedNames, Set bindings, String verb) { - Set expected = new TreeSet(Arrays.asList(expectedNames)); - Set extra = new TreeSet(); + private void assertExpectedBindings(String[] expectedNames, Set bindings, String verb) + throws Exception { + Set expected = new TreeSet<>(Arrays.asList(expectedNames)); + Set extra = new TreeSet<>(); for (IBinding binding : bindings) { - extra.add(binding.getName()); + extra.add(getQualifiedName(binding)); } - Set missing = new TreeSet(expected); + Set missing = new TreeSet<>(expected); missing.removeAll(extra); extra.removeAll(expected); if (extra.isEmpty() && missing.isEmpty()) return; - List errors = new ArrayList(2); + List errors = new ArrayList<>(2); if (!missing.isEmpty()) { errors.add(MessageFormat.format("{0,choice,1#Binding|1x; public void testClassMember() throws Exception { - assertDefined("f", "A"); + assertDefined("A", "f"); assertDeclared(); } @@ -169,14 +180,14 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { // class B : public A {}; // B b; // class C : public A {}; - // C c; + // C* c; // void test() { // b.m(); // c->m(); // } public void testClassHierarchy() throws Exception { - assertDefined("b", "B", "c", "C", "m"); + assertDefined("B", "b", "C", "c", "A::m"); assertDeclared(); } @@ -186,7 +197,24 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { // a->m(); // } public void testMethodCall() throws Exception { - assertDefined("A", "m"); + assertDefined("A", "A::m"); + assertDeclared(); + } + + // class Base { + // public: + // void m(); + // }; + // + // class Derived : public Base { + // }; + + // class Derived; + // void test(Derived& d) { + // d.m(); + // } + public void testSuperClassMethodCall_436656() throws Exception { + assertDefined("Derived", "Base::m"); assertDeclared(); } @@ -202,19 +230,19 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { public void testFunctionCallWithPointerParameter_1() throws Exception { getPreferenceStore().setValue(PreferenceConstants.FORWARD_DECLARE_FUNCTIONS, true); assertDefined(); - assertDeclared("f", "A", "g"); + assertDeclared("A", "f", "g"); } // typedef int A; - // void f(const A* p); + // void f(const A* p) {} // void test() { // f(nullptr); // } public void testFunctionCallWithPointerParameter_2() throws Exception { getPreferenceStore().setValue(PreferenceConstants.FORWARD_DECLARE_FUNCTIONS, true); - assertDefined("A"); - assertDeclared("f"); + assertDefined("f"); // Inline definition has to be included. + assertDeclared(); } // class A {}; @@ -227,7 +255,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { public void testFunctionCallWithReferenceParameter() throws Exception { getPreferenceStore().setValue(PreferenceConstants.FORWARD_DECLARE_FUNCTIONS, true); assertDefined(); - assertDeclared("f", "A", "g"); + assertDeclared("A", "f", "g"); } // struct A { @@ -239,11 +267,10 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { // f(""); // } public void testFunctionCallWithTypeConversion_1() throws Exception { - getPreferenceStore().setValue(PreferenceConstants.FORWARD_DECLARE_FUNCTIONS, true); // A header declaring the function is responsible for defining the parameter type that // provides constructor that can be used for implicit conversion. - assertDefined(); - assertDeclared("f", "A"); + assertDefined("f"); + assertDeclared(); } // struct A {}; @@ -254,11 +281,10 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { // f(b); // } public void testFunctionCallWithTypeConversion_2() throws Exception { - getPreferenceStore().setValue(PreferenceConstants.FORWARD_DECLARE_FUNCTIONS, true); // A header declaring the function is not responsible for defining the parameter type since // the implicit conversion from B to A is provided externally to parameter type. - assertDefined("A", "B"); - assertDeclared("f"); + assertDefined("A", "B", "f"); + assertDeclared(); } // typedef int int32; @@ -282,9 +308,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { // } public void testConstructorCall() throws Exception { getPreferenceStore().setValue(PreferenceConstants.FORWARD_DECLARE_FUNCTIONS, true); - // A header declaring the function is not responsible for defining the parameter type since - // the implicit conversion from B to A is provided externally to parameter type. - assertDefined("A"); + assertDefined("A", "A::A"); assertDeclared(); } @@ -298,9 +322,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { // } public void testConstructorCallWithTypedef() throws Exception { getPreferenceStore().setValue(PreferenceConstants.FORWARD_DECLARE_FUNCTIONS, true); - // A header declaring the function is not responsible for defining the parameter type since - // the implicit conversion from B to A is provided externally to parameter type. - assertDefined("B", "A"); + assertDefined("B", "A::A"); assertDeclared(); } @@ -357,7 +379,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { // ns::A a; public void testNamespaceAlias() throws Exception { - assertDefined("A", "ns"); + assertDefined("ns1::ns2::A", "ns"); assertDeclared(); } @@ -368,7 +390,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { // using ns::A; public void testUsingDeclaration() throws Exception { assertDefined(); - assertDeclared("A"); + assertDeclared("ns::A"); } // struct A { @@ -418,7 +440,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { // a(1); // } public void testCallOperator() throws Exception { - assertDefined("A", "a", "operator ()"); + assertDefined("A", "a", "A::operator ()"); assertDeclared(); } @@ -430,7 +452,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { // } // bool test(const A& a, const A& b) { - // return a == b; + // return a == b; // } public void testOverloadedOperator() throws Exception { assertDefined("operator =="); @@ -510,7 +532,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { // unique_ptr y; // } public void testTemplatesAllowingIncompleteParameterType_1() throws Exception { - assertDefined("B", "C", "shared_ptr", "unique_ptr"); + assertDefined("B", "C", "std::shared_ptr", "std::unique_ptr"); assertDeclared("A"); } @@ -531,7 +553,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { // b->m(); // } public void testTemplatesAllowingIncompleteParameterType_2() throws Exception { - assertDefined("B", "b", "m"); + assertDefined("B", "b", "A::m"); assertDeclared(); } @@ -554,7 +576,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { // c->x->m(); // } public void testTemplatesAllowingIncompleteParameterType_3() throws Exception { - assertDefined("B", "C", "m"); + assertDefined("B", "C", "A::m"); assertDeclared(); } @@ -575,7 +597,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { // f()->m(); // } public void testTemplatesAllowingIncompleteParameterType_4() throws Exception { - assertDefined("B", "f", "m"); + assertDefined("B", "f", "A::m"); assertDeclared(); } @@ -598,7 +620,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { // c->f()->m(); // } public void testTemplatesAllowingIncompleteParameterType_5() throws Exception { - assertDefined("B", "C", "f", "m"); + assertDefined("B", "C", "C::f", "A::m"); assertDeclared(); } @@ -622,8 +644,25 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase { // void test(C* c) { // c->b(); // } - public void testFieldAccess_442841() throws Exception { - assertDefined("C", "operator ()"); + public void testFieldAccess_442841_1() throws Exception { + assertDefined("C", "A::operator ()"); + assertDeclared(); + } + + // struct A { + // void operator()(); + // }; + // struct B : public A { + // }; + // struct C { + // B& b; + // }; + + // void test(C* c) { + // c->b(); + // } + public void testFieldAccess_442841_2() throws Exception { + assertDefined("B", "C", "A::operator ()"); assertDeclared(); } diff --git a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/includes/IncludeOrganizerTest.java b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/includes/IncludeOrganizerTest.java index 936005bbdf2..98670290a3d 100644 --- a/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/includes/IncludeOrganizerTest.java +++ b/core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/refactoring/includes/IncludeOrganizerTest.java @@ -605,7 +605,7 @@ public class IncludeOrganizerTest extends IncludesTestBase { //class A {}; //class B {}; //namespace ns1 { - //C* f(const A& a, B* b) { return nullptr; } + //C* f(const A& a, B* b); //} // ns1 //} // ns2 //} // ns3 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 2692be1e7ba..7c1aaf71d6c 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 @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2012, 2013 Google, Inc and others. + * Copyright (c) 2012, 2015 Google, 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 @@ -113,7 +113,9 @@ 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.dom.ast.cpp.ICPPUsingDeclaration; +import org.eclipse.cdt.core.index.IIndexFile; import org.eclipse.cdt.core.index.IIndexMacro; +import org.eclipse.cdt.core.index.IIndexName; import org.eclipse.cdt.core.index.IndexFilter; import org.eclipse.cdt.core.parser.util.CharArrayUtils; @@ -557,7 +559,7 @@ public class BindingClassifier { if (unaryExpression instanceof ICPPASTUnaryExpression) { ICPPFunction overload = ((ICPPASTUnaryExpression) unaryExpression).getOverload(); if (overload != null) { - defineFunction(overload, new IASTInitializerClause[] { operand }); + defineForFunctionCall(overload, new IASTInitializerClause[] { operand }); return PROCESS_CONTINUE; } } @@ -614,8 +616,7 @@ public class BindingClassifier { if (binaryExpression instanceof ICPPASTBinaryExpression) { ICPPFunction overload = ((ICPPASTBinaryExpression) binaryExpression).getOverload(); if (overload != null) { - defineFunction(overload, - new IASTInitializerClause[] { binaryExpression.getOperand1(), binaryExpression.getOperand2() }); + defineForFunctionCall(overload, new IASTInitializerClause[] { binaryExpression.getOperand1(), binaryExpression.getOperand2() }); return PROCESS_CONTINUE; } } @@ -700,19 +701,23 @@ public class BindingClassifier { IBinding binding = getBindingOfExpression(functionNameExpression); if (binding != null) { if (binding instanceof IFunction) { - declareFunction((IFunction) binding, functionCallExpression.getArguments()); - } else { - if (binding instanceof IType) - defineBinding(binding); - - if (functionCallExpression instanceof IASTImplicitNameOwner) { - IASTImplicitName[] implicitNames = ((IASTImplicitNameOwner) functionCallExpression).getImplicitNames(); - for (IASTName name : implicitNames) { - binding = name.resolveBinding(); - if (binding instanceof IFunction) { - defineFunction((IFunction) binding, functionCallExpression.getArguments()); - } - } + defineForFunctionCall((IFunction) binding, functionCallExpression.getArguments()); + } else if (binding instanceof ICPPMember) { + try { + IType memberType = ((ICPPMember) binding).getType(); + defineIndirectTypes(memberType); + } catch (DOMException e) { + } + } else if (binding instanceof ITypedef) { + defineBinding(binding); + } + } + if (functionCallExpression instanceof IASTImplicitNameOwner) { + IASTImplicitName[] implicitNames = ((IASTImplicitNameOwner) functionCallExpression).getImplicitNames(); + for (IASTName name : implicitNames) { + binding = name.resolveBinding(); + if (binding instanceof IFunction) { + defineForFunctionCall((IFunction) binding, functionCallExpression.getArguments()); } } } @@ -729,17 +734,11 @@ public class BindingClassifier { IASTExpression fieldOwner = ((IASTFieldReference) expression).getFieldOwner(); IType expressionType = fieldOwner.getExpressionType(); - if (expressionType instanceof IPointerType || expressionType instanceof ICPPReferenceType) { - defineTypeExceptTypedefOrNonFixedEnum(expressionType); - } else if (!(fieldOwner instanceof IASTIdExpression || fieldOwner instanceof IASTFunctionCallExpression)) { - IBinding binding = getBindingOfExpression(fieldOwner); - if (binding != null) { - defineTypeForBinding(binding, expressionType); - } else { - defineTypeExceptTypedefOrNonFixedEnum(expressionType); - } + defineIndirectTypes(expressionType); + IBinding binding = getBindingOfExpression(fieldOwner); + if (binding != null) { + defineTypeForBinding(binding, expressionType); } - // Id expressions and function call expressions are handled elsewhere. } else if (expression instanceof ICPPASTNewExpression) { /* * The type specifier of a "new" expression always requires a definition. @@ -808,7 +807,7 @@ public class BindingClassifier { if (isTemplateArgumentRequiringCompleteType(name)) { // The name is part of a template argument - define the corresponding binding. defineBinding(binding); - } else { + } else if (name.getPropertyInParent() != IASTFieldReference.FIELD_NAME) { // Field references are handled separately. IBinding owner = binding.getOwner(); if (owner instanceof IType) { defineBinding(owner); // Member access requires definition of the containing type. @@ -829,7 +828,7 @@ public class BindingClassifier { markAsDefined((IBinding) type); } } - } else { + } else { // ID expressions are handled separately. declareBinding(binding); // Declare the binding of this name. } } @@ -920,7 +919,7 @@ public class BindingClassifier { * comparing the declared parameters with the actual arguments. */ private void processFunctionParameters(IFunction function, IASTInitializerClause[] arguments) { - boolean functionIsDefined = fProcessedDefinedBindings.contains(function); + boolean functionIsDeclared = fProcessedDefinedBindings.contains(function); IParameter[] parameters = function.getParameters(); for (int i = 0; i < parameters.length && i < arguments.length; i++) { IType parameterType = parameters[i].getType(); @@ -931,7 +930,7 @@ public class BindingClassifier { // A declaration is sufficient if the argument type matches the parameter type. // We don't need to provide a declaration of the parameter type since it is // a responsibility of the header declaring the function. - if (!functionIsDefined) { + if (!functionIsDeclared) { declareType(parameterType); } continue; @@ -947,7 +946,7 @@ public class BindingClassifier { fAst.getDeclarationsInAST(function).length != 0 || !hasConvertingConstructor((ICPPClassType) parameterType, argument)) { defineTypeExceptTypedefOrNonFixedEnum(parameterType); - } else if (!functionIsDefined) { + } else if (!functionIsDeclared) { declareType(parameterType); } } @@ -1086,12 +1085,10 @@ public class BindingClassifier { } private void addRequiredBindings(IBinding binding, Deque newBindings) { - if (binding instanceof ICPPMember) { - if (binding instanceof ICPPMethod) { - newBindings.add(binding); // Include the method in case we need its definition. - } - // If the binding is a member, get its owning composite type. - newBindings.add(binding.getOwner()); + if (binding instanceof ICPPMethod) { + newBindings.add(binding); // Include the method in case we need its inline definition. + if (binding instanceof ICPPConstructor) + newBindings.add(binding.getOwner()); } else if (binding instanceof IType) { // Remove type qualifiers. IBinding b = getTypeBinding((IType) binding); @@ -1174,7 +1171,7 @@ public class BindingClassifier { } /** - * Checks whether the binding can be forward declared or not according to preferences. + * Checks whether the binding can be forward declared according to preferences. */ private boolean canForwardDeclare(IBinding binding) { boolean canDeclare = false; @@ -1309,14 +1306,16 @@ public class BindingClassifier { return true; } - private void defineFunction(IFunction function, IASTInitializerClause[] arguments) { - defineBinding(function); - declareFunction(function, arguments); - } - - private void declareFunction(IFunction function, IASTInitializerClause[] arguments) { - if (!canForwardDeclare(function)) - defineBinding(function); + private void defineForFunctionCall(IFunction function, IASTInitializerClause[] arguments) { + if (!fProcessedDefinedBindings.contains(function)) { + if (!(function instanceof ICPPMethod) && (!canForwardDeclare(function) || isDefinedInHeaderFile(function))) { + // Since the function is defined in a header file, its definition has to be + // reachable through includes to make a function call. + defineBinding(function); + } else { + declareBinding(function); + } + } // Handle return or expression type of the function or constructor call. IType returnType = function.getType().getReturnType(); @@ -1324,10 +1323,46 @@ public class BindingClassifier { // Handle parameters. processFunctionParameters(function, arguments); + + fProcessedDefinedBindings.add(function); + } + + private boolean isDefinedInHeaderFile(IFunction function) { + try { + IIndexName[] definitions = fContext.getIndex().findDefinitions(function); + for (IIndexName name : definitions) { + IIndexFile file = name.getFile(); + if (file != null && canBeIncluded(file)) + return true; + } + } catch (CoreException e) { + // Ignore to return false. + } + return false; + } + + private boolean canBeIncluded(IIndexFile indexFile) throws CoreException { + return !IncludeUtil.isSource(indexFile, fContext.getProject()) || + fContext.getIndex().findIncludedBy(indexFile, 0).length != 0; } private void defineTypeForBinding(IBinding binding, IType type) { if (isDefined(binding) && !mayBeForwardDeclared(type)) { + defineIndirectTypes(type); + } else if (!(type instanceof IPointerType || type instanceof ICPPReferenceType)) { + defineTypeExceptTypedefOrNonFixedEnum(type); + } + } + + /** + * For a pointer or a reference type, defines the contained type. For an instance of a template + * allowing incomplete argument types, defines the argument type. + */ + private void defineIndirectTypes(IType type) { + IType resolvedType = resolveTypedef(type); + if (resolvedType instanceof IPointerType || resolvedType instanceof ICPPReferenceType) { + defineTypeExceptTypedefOrNonFixedEnum(resolvedType); + } else { if (type instanceof ICPPTemplateInstance) { ICPPTemplateInstance instance = (ICPPTemplateInstance) type; IBinding template = instance.getSpecializedBinding(); @@ -1339,8 +1374,6 @@ public class BindingClassifier { } } } - } else if (!(type instanceof IPointerType || type instanceof ICPPReferenceType)) { - defineTypeExceptTypedefOrNonFixedEnum(type); } } @@ -1394,10 +1427,21 @@ public class BindingClassifier { return null; } + /** + * If the given type is a typedef, returns the ultimate type the typedef is pointing to. + * Otherwise returns the given type. + */ + private IType resolveTypedef(IType type) { + while (type instanceof ITypedef) { + type = ((ITypedef) type).getType(); + } + return type; + } + /** * Checks if the given name is part of a template argument. */ - public boolean isTemplateArgumentRequiringCompleteType(IASTName name) { + private boolean isTemplateArgumentRequiringCompleteType(IASTName name) { ICPPASTTypeId typeId = CPPVisitor.findAncestorWithType(name, ICPPASTTypeId.class); if (typeId == null || typeId.getPropertyInParent() != ICPPASTTemplateId.TEMPLATE_ID_ARGUMENT) return false;