1
0
Fork 0
mirror of https://github.com/eclipse-cdt/cdt synced 2025-06-07 17:56:01 +02:00

Bug 432651 - Organize Includes removes an include necessary for defining

a method
This commit is contained in:
Sergey Prigogin 2014-04-11 16:00:30 -07:00
parent 41b9049afa
commit 5c7a64d0fb
7 changed files with 200 additions and 103 deletions

View file

@ -349,17 +349,18 @@ public abstract class ArrayUtil {
* Adds all elements of an array to a collection.
* @since 5.4
*/
public static <T> void addAll(Collection<T> collection, T[] array) {
@SafeVarargs
public static <T> void addAll(Collection<? super T> collection, T... elements) {
if (collection instanceof ArrayList) {
((ArrayList<T>) collection).ensureCapacity(collection.size() + array.length);
((ArrayList<?>) collection).ensureCapacity(collection.size() + elements.length);
}
for (T element : array) {
for (T element : elements) {
collection.add(element);
}
}
/**
* Returns whether the specified array contains the specified object. Comparison is by
* Returns whether the specified array contains the specified object. The comparison is by
* object identity.
*
* @param array the array to search

View file

@ -92,7 +92,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase {
private void assertDeclared(String... names) {
classifyBindings();
assertExpectedBindings(names, fBindingClassifier.getBindingsToDeclare(), "declared");
assertExpectedBindings(names, fBindingClassifier.getBindingsToForwardDeclare(), "declared");
}
private void assertExpectedBindings(String[] expectedNames, Set<IBinding> bindings, String verb) {
@ -176,7 +176,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase {
// c->m();
// }
public void testClassHierarchy() throws Exception {
assertDefined("b", "B", "c", "C");
assertDefined("b", "B", "c", "C", "m");
assertDeclared();
}
@ -186,7 +186,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase {
// a->m();
// }
public void testMethodCall() throws Exception {
assertDefined("A");
assertDefined("A", "m");
assertDeclared();
}
@ -300,7 +300,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase {
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");
assertDefined("B", "A");
assertDeclared();
}
@ -337,6 +337,17 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase {
assertDeclared("A");
}
// class A {
// void m();
// };
// void A::m() {
// }
public void testMethodDefinition() throws Exception {
assertDefined("A");
assertDeclared();
}
// struct A {
// A(const char* s);
// };
@ -384,7 +395,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase {
// a(1);
// }
public void testCallOperator() throws Exception {
assertDefined("A", "a");
assertDefined("A", "a", "operator ()");
assertDeclared();
}
@ -497,7 +508,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase {
// b->m();
// }
public void testTemplatesAllowingIncompleteParameterType_2() throws Exception {
assertDefined("B", "b");
assertDefined("B", "b", "m");
assertDeclared();
}
@ -520,7 +531,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase {
// c->x->m();
// }
public void testTemplatesAllowingIncompleteParameterType_3() throws Exception {
assertDefined("B", "C");
assertDefined("B", "C", "m");
assertDeclared();
}
@ -541,7 +552,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase {
// f()->m();
// }
public void testTemplatesAllowingIncompleteParameterType_4() throws Exception {
assertDefined("B", "f");
assertDefined("B", "f", "m");
assertDeclared();
}
@ -564,7 +575,7 @@ public class BindingClassifierTest extends OneSourceMultipleHeadersTestCase {
// c->f()->m();
// }
public void testTemplatesAllowingIncompleteParameterType_5() throws Exception {
assertDefined("B", "C");
assertDefined("B", "C", "f", "m");
assertDeclared();
}

View file

@ -473,6 +473,37 @@ public class IncludeOrganizerTest extends IncludesTestBase {
assertExpectedResults();
}
//h1.h
//struct A {
// template <typename T>
// void m(const T& p);
//};
//h2.h
//#include "h1.h"
//template<typename T>
//void A::m(const T& p) {
//}
//h3.h
//#include "h1.h"
//typedef A B;
//source.cpp
//void test(B& b) {
// b.m(1);
//}
//====================
//#include "h2.h"
//#include "h3.h"
//
//void test(B& b) {
// b.m(1);
//}
public void testMethodDefinedInHeader() throws Exception {
assertExpectedResults();
}
//h1.h
//namespace ns3 {
//class C {};

View file

@ -69,7 +69,7 @@ public class InclusionContext {
return fCurrentDirectory;
}
public IncludePreferences getPreferences() {
public final IncludePreferences getPreferences() {
return fPreferences;
}

View file

@ -18,9 +18,11 @@ 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.ArrayDeque;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Deque;
import java.util.HashSet;
import java.util.Set;
@ -127,26 +129,28 @@ import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil;
* must be defined and a set of bindings that must be declared.
*/
public class BindingClassifier {
public enum InclusionType { DECLARATION, DEFINITION }
private final IncludeCreationContext fContext;
private final IncludePreferences fPreferences;
/** The bindings which require a full definition. */
private final Set<IBinding> fBindingsToDefine;
/** The bindings which only require a simple forward declaration. */
private final Set<IBinding> fBindingsToDeclare;
private final Set<IBinding> fBindingsToForwardDeclare;
/** The AST that the classifier is working on. */
private IASTTranslationUnit fAst;
private final BindingCollector fBindingCollector;
private final Set<IBinding> fProcessedDefinedBindings;
private final Set<IBinding> fProcessedDeclaredBindings;
private static final Set<String> templatesAllowingIncompleteArgumentType =
Collections.unmodifiableSet(new HashSet<String>(Arrays.asList(new String[] {
private static final String[] TEMPLATES_ALLOWING_INCOMPLETE_ARGUMENT_TYPE = {
// Please keep alphabetical order.
"enable_shared_from_this", // 20.7.2.4 //$NON-NLS-1$
"declval", // 20.2.4 //$NON-NLS-1$
"default_delete", // 20.7.1.1 //$NON-NLS-1$
"shared_ptr", // 20.7.2.2 //$NON-NLS-1$
"unique_ptr", // 20.7.1 //$NON-NLS-1$
"weak_ptr" // 20.7.2.3 //$NON-NLS-1$
})));
};
/**
* @param context the context for binding classification
@ -154,10 +158,10 @@ public class BindingClassifier {
public BindingClassifier(IncludeCreationContext context) {
fContext = context;
fPreferences = context.getPreferences();
fBindingsToDefine = new HashSet<IBinding>();
fBindingsToDeclare = new HashSet<IBinding>();
fProcessedDefinedBindings = new HashSet<IBinding>();
fProcessedDeclaredBindings = new HashSet<IBinding>();
fBindingsToDefine = new HashSet<>();
fBindingsToForwardDeclare = new HashSet<>();
fProcessedDefinedBindings = new HashSet<>();
fProcessedDeclaredBindings = new HashSet<>();
fBindingCollector = new BindingCollector();
}
@ -178,8 +182,8 @@ public class BindingClassifier {
/**
* Returns the bindings which only require a simple forward declaration.
*/
public Set<IBinding> getBindingsToDeclare() {
return fBindingsToDeclare;
public Set<IBinding> getBindingsToForwardDeclare() {
return fBindingsToForwardDeclare;
}
/**
@ -306,46 +310,56 @@ public class BindingClassifier {
* or an empty list if no such binding is available.
*/
private Set<IBinding> getRequiredBindings(IBinding binding) {
Set<IBinding> bindings = new HashSet<IBinding>();
if (binding instanceof ICPPNamespace)
return Collections.emptySet();
if (binding instanceof ICPPMember) {
// If the binding is a member, get its owning composite type.
binding = binding.getOwner();
} else if (binding instanceof IVariable) {
if (binding instanceof ICPPSpecialization) {
bindings.add(((ICPPSpecialization) binding).getSpecializedBinding());
} else {
bindings.add(binding);
}
} else if (binding instanceof IType) {
// Resolve the type.
binding = getTypeBinding((IType) binding);
} else if (binding instanceof ICPPNamespace) {
// Namespaces are neither declared nor defined.
binding = null;
}
Deque<IBinding> queue = new ArrayDeque<>();
addRequiredBindings(binding, queue);
Set<IBinding> bindings = new HashSet<>();
while ((binding = queue.poll()) != null) {
if (!bindings.add(binding))
continue;
if (binding instanceof ICPPSpecialization) {
ICPPTemplateParameterMap parameterMap = ((ICPPSpecialization) binding).getTemplateParameterMap();
for (Integer position : parameterMap.getAllParameterPositions()) {
ICPPTemplateArgument argument = parameterMap.getArgument(position);
if (argument != null) {
IType type = argument.getTypeValue();
// Normally we don't need to define parameters of a template specialization that
// were not specified explicitly. __gnu_cxx::hash is an exception from that rule.
// Normally we don't need to define parameters of a template specialization
// that were not specified explicitly. __gnu_cxx::hash is an exception from
// that rule.
if (type instanceof IBinding && "hash".equals(((IBinding) type).getName())) { //$NON-NLS-1$
IBinding owner = ((IBinding) type).getOwner();
if (owner instanceof ICPPNamespace && "__gnu_cxx".equals(owner.getName())) //$NON-NLS-1$
bindings.add((IBinding) type);
addRequiredBindings((IBinding) type, queue);
}
}
}
// Get the specialized binding - e.g. get the binding for X if the current binding is
// for the template specialization X<Y>.
binding = ((ICPPSpecialization) binding).getSpecializedBinding();
addRequiredBindings(((ICPPSpecialization) binding).getSpecializedBinding(), queue);
}
}
if (binding instanceof IProblemBinding) {
return bindings;
}
private void addRequiredBindings(IBinding binding, Deque<IBinding> 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());
} else if (binding instanceof IType) {
// Remove type qualifiers.
IBinding b = getTypeBinding((IType) binding);
if (b != null)
newBindings.add(b);
} else if (binding instanceof IProblemBinding) {
IProblemBinding problemBinding = (IProblemBinding) binding;
IBinding[] candidateBindings = problemBinding.getCandidateBindings();
@ -354,25 +368,19 @@ public class BindingClassifier {
// different candidates are very often defined within the same target file anyway,
// so it won't affect the list of generated include directives. This therefore
// allows us to be a little more fault tolerant here.
for (IBinding candidateBinding : candidateBindings) {
bindings.add(candidateBinding);
}
Collections.addAll(newBindings, candidateBindings);
} else {
// No candidate bindings available. Check whether this is a macro.
try {
IIndexMacro[] indexMacros = fContext.getIndex().findMacros(binding.getNameCharArray(),
IndexFilter.ALL, null);
for (IIndexMacro indexMacro : indexMacros) {
bindings.add(indexMacro);
}
IIndexMacro[] indexMacros =
fContext.getIndex().findMacros(binding.getNameCharArray(), IndexFilter.ALL, null);
Collections.addAll(newBindings, indexMacros);
} catch (CoreException e) {
}
}
} else if (binding != null) {
bindings.add(binding);
} else {
newBindings.add(binding);
}
return bindings;
}
private void declareType(IType type) {
@ -404,7 +412,8 @@ public class BindingClassifier {
Collection<IBinding> requiredBindings = getRequiredBindings(binding);
for (IBinding requiredBinding : requiredBindings) {
if (fBindingsToDeclare.contains(requiredBinding) || fBindingsToDefine.contains(requiredBinding)) {
if (fBindingsToForwardDeclare.contains(requiredBinding) ||
fBindingsToDefine.contains(requiredBinding)) {
return;
}
if (fAst.getDefinitionsInAST(requiredBinding).length != 0) {
@ -416,7 +425,7 @@ public class BindingClassifier {
if (canForwardDeclare(requiredBinding)) {
if (requiredBinding == binding) {
fBindingsToDeclare.add(requiredBinding);
fBindingsToForwardDeclare.add(requiredBinding);
} else {
declareBinding(requiredBinding);
}
@ -493,18 +502,19 @@ public class BindingClassifier {
private void defineBinding(IBinding binding) {
if (!markAsDefined(binding))
return;
if (fAst.getDefinitionsInAST(binding).length != 0)
return; // Defined locally.
Collection<IBinding> requiredBindings = getRequiredBindings(binding);
for (IBinding requiredBinding : requiredBindings) {
fBindingsToDeclare.remove(requiredBinding);
if (requiredBinding == binding) {
fBindingsToDefine.add(requiredBinding);
} else {
defineBinding(requiredBinding);
fBindingsToForwardDeclare.remove(requiredBinding);
if (requiredBinding != binding) {
if (!markAsDefined(requiredBinding))
continue;
if (fAst.getDefinitionsInAST(requiredBinding).length != 0)
continue; // Defined locally.
}
fBindingsToDefine.add(requiredBinding);
}
}
@ -538,7 +548,7 @@ public class BindingClassifier {
for (ICPPClassType base : bases) {
fProcessedDefinedBindings.add(base);
fBindingsToDefine.remove(base);
fBindingsToDeclare.remove(base);
fBindingsToForwardDeclare.remove(base);
}
}
@ -567,7 +577,7 @@ public class BindingClassifier {
if (type instanceof ICPPTemplateInstance) {
ICPPTemplateInstance instance = (ICPPTemplateInstance) type;
IBinding template = instance.getSpecializedBinding();
if (templatesAllowingIncompleteArgumentType.contains(template.getName())) {
if (isTemplateAllowingIncompleteArgumentType(template)) {
ICPPTemplateArgument[] arguments = instance.getTemplateArguments();
if (arguments.length != 0) {
IType argumentType = arguments[0].getTypeValue();
@ -589,6 +599,17 @@ public class BindingClassifier {
return false;
}
private static boolean isTemplateAllowingIncompleteArgumentType(IBinding binding) {
String name = binding.getName();
int pos = Arrays.binarySearch(TEMPLATES_ALLOWING_INCOMPLETE_ARGUMENT_TYPE, name);
if (pos < 0)
return false;
IBinding owner = binding.getOwner();
if (!(owner instanceof ICPPNamespace))
return false;
return CharArrayUtils.equals(owner.getNameCharArray(), STD) && owner.getOwner() == null;
}
private class BindingCollector extends ASTVisitor {
BindingCollector() {
super(true);
@ -1297,8 +1318,7 @@ public class BindingClassifier {
!CharArrayUtils.equals(owner.getNameCharArray(), STD) || owner.getOwner() != null) {
return true;
}
String templateName = template.getName();
if (!templatesAllowingIncompleteArgumentType.contains(templateName))
if (!isTemplateAllowingIncompleteArgumentType(template))
return true;
// For most templates allowing incomplete argument type a full definition of the argument

View file

@ -54,7 +54,7 @@ public class IncludeCreationContext extends InclusionContext {
return fSourceContents;
}
public IIndex getIndex() {
public final IIndex getIndex() {
return fIndex;
}

View file

@ -78,6 +78,7 @@ import org.eclipse.cdt.core.index.IIndexInclude;
import org.eclipse.cdt.core.index.IIndexName;
import org.eclipse.cdt.core.model.ITranslationUnit;
import org.eclipse.cdt.core.parser.Keywords;
import org.eclipse.cdt.core.parser.util.ArrayUtil;
import org.eclipse.cdt.core.parser.util.CharArrayIntMap;
import org.eclipse.cdt.core.parser.util.CharArrayUtils;
import org.eclipse.cdt.ui.CUIPlugin;
@ -199,7 +200,7 @@ public class IncludeOrganizer {
// Process the given translation unit with the inclusion resolver.
BindingClassifier bindingClassifier = new BindingClassifier(fContext);
bindingClassifier.classifyNodeContents(ast);
Set<IBinding> bindingsToDefine = bindingClassifier.getBindingsToDefine();
Set<IBinding> bindingsToInclude = bindingClassifier.getBindingsToDefine();
IASTPreprocessorIncludeStatement[] existingIncludes = ast.getIncludeDirectives();
fContext.addHeadersIncludedPreviously(existingIncludes);
@ -209,7 +210,7 @@ public class IncludeOrganizer {
// bindings which have to be defined.
IIndexFileSet reachableHeaders = ast.getIndexFileSet();
List<InclusionRequest> requests = createInclusionRequests(ast, bindingsToDefine, false, reachableHeaders);
List<InclusionRequest> requests = createInclusionRequests(ast, bindingsToInclude, false, reachableHeaders);
processInclusionRequests(requests, headerSubstitutor);
// Use a map instead of a set to be able to retrieve existing elements using equal elements.
@ -357,7 +358,7 @@ public class IncludeOrganizer {
IIndexFileSet reachableHeaders = ast.getIndexFileSet();
Set<IBinding> bindings =
removeBindingsDefinedInIncludedHeaders(ast, classifier.getBindingsToDeclare(), reachableHeaders);
removeBindingsDefinedInIncludedHeaders(ast, classifier.getBindingsToForwardDeclare(), reachableHeaders);
for (IBinding binding : bindings) {
// Create the text of the forward declaration of this binding.
StringBuilder declarationText = new StringBuilder();
@ -748,13 +749,12 @@ public class IncludeOrganizer {
private Set<IBinding> removeBindingsDefinedInIncludedHeaders(IASTTranslationUnit ast,
Set<IBinding> bindings, IIndexFileSet reachableHeaders) throws CoreException {
Set<IBinding> filteredBindings = new HashSet<>(bindings);
List<InclusionRequest> requests = createInclusionRequests(ast, bindings, true, reachableHeaders);
Set<IPath> allIncludedHeaders = new HashSet<>();
allIncludedHeaders.addAll(fContext.getHeadersAlreadyIncluded());
allIncludedHeaders.addAll(fContext.getHeadersToInclude());
Set<IBinding> filteredBindings = new HashSet<>(bindings);
for (InclusionRequest request : requests) {
if (isSatisfiedByIncludedHeaders(request, allIncludedHeaders))
filteredBindings.remove(request.getBinding());
@ -992,12 +992,12 @@ public class IncludeOrganizer {
}
private List<InclusionRequest> createInclusionRequests(IASTTranslationUnit ast,
Set<IBinding> bindingsToDefine, boolean allowDeclarations,
Set<IBinding> bindingsToInclude, boolean allowDeclarations,
IIndexFileSet reachableHeaders) throws CoreException {
List<InclusionRequest> requests = new ArrayList<InclusionRequest>(bindingsToDefine.size());
List<InclusionRequest> requests = new ArrayList<InclusionRequest>(bindingsToInclude.size());
IIndex index = fContext.getIndex();
binding_loop: for (IBinding binding : bindingsToDefine) {
binding_loop: for (IBinding binding : bindingsToInclude) {
IIndexName[] indexNames;
if (binding instanceof IMacroBinding) {
indexNames = IIndexName.EMPTY_ARRAY;
@ -1012,12 +1012,30 @@ public class IncludeOrganizer {
}
}
}
} else if (allowDeclarations || binding instanceof IFunction || binding instanceof IVariable) {
// For functions and variables we need to include a declaration.
} else if (allowDeclarations || binding instanceof IVariable) {
// For a variable we need to include a declaration.
indexNames = index.findDeclarations(binding);
} else if (binding instanceof ICPPMethod) {
// Include the headers containing method definitions except the ones also containing
// the definition of the owner class. The headers defining the owner class are taken
// care of separately.
Set<IIndexFile> declarationFiles = new HashSet<>();
IIndexName[] declarations = index.findNames(binding, IIndex.FIND_DECLARATIONS);
for (IIndexName declaration : declarations) {
IIndexFile file = declaration.getFile();
if (file != null) {
declarationFiles.add(file);
}
}
IIndexName[] definitions = index.findDefinitions(binding);
indexNames = filterIncludableNotInBlacklistedFiles(definitions, declarationFiles);
} else {
// For all other bindings we need to include the definition.
indexNames = index.findDefinitions(binding);
if (binding instanceof IFunction) {
// If a function is defined in a header, include that header.
// Otherwise look for declarations.
indexNames = filterIncludableNotInBlacklistedFiles(indexNames, Collections.<IIndexFile>emptySet());
}
if (indexNames.length == 0) {
// If we could not find any definitions, there is still a chance that
// a declaration would be sufficient.
@ -1035,12 +1053,11 @@ public class IncludeOrganizer {
}
}
Map<IIndexFile, IPath> declaringHeaders = new HashMap<IIndexFile, IPath>();
Map<IIndexFile, IPath> reachableDeclaringHeaders = new HashMap<IIndexFile, IPath>();
Map<IIndexFile, IPath> declaringHeaders = new HashMap<>();
Map<IIndexFile, IPath> reachableDeclaringHeaders = new HashMap<>();
for (IIndexName indexName : indexNames) {
IIndexFile indexFile = indexName.getFile();
if (IncludeUtil.isSource(indexFile, fContext.getProject()) &&
index.findIncludedBy(indexFile, 0).length == 0) {
if (!canBeIncluded(indexFile)) {
// The target is a source file which isn't included by any other files.
// Don't include it.
continue;
@ -1064,6 +1081,23 @@ public class IncludeOrganizer {
return requests;
}
private IIndexName[] filterIncludableNotInBlacklistedFiles(IIndexName[] names, Set<IIndexFile> blacklist)
throws CoreException {
IIndexName[] includable = IIndexName.EMPTY_ARRAY;
int pos = 0;
for (IIndexName name : names) {
IIndexFile file = name.getFile();
if (file != null && !blacklist.contains(file) && canBeIncluded(file))
includable = ArrayUtil.appendAt(includable, pos++, name);
}
return ArrayUtil.trim(includable, pos);
}
private boolean canBeIncluded(IIndexFile indexFile) throws CoreException {
return !IncludeUtil.isSource(indexFile, fContext.getProject()) ||
fContext.getIndex().findIncludedBy(indexFile, 0).length != 0;
}
private String createIncludeDirective(IncludePrototype include, String lineComment) {
StringBuilder buf = new StringBuilder();
// Unresolved includes are preserved out of caution. Partner include is always preserved.