diff --git a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/AbstractClassInstantiationCheckerTest.java b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/AbstractClassInstantiationCheckerTest.java index aa705df443b..8833857aecc 100644 --- a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/AbstractClassInstantiationCheckerTest.java +++ b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/AbstractClassInstantiationCheckerTest.java @@ -313,6 +313,24 @@ public class AbstractClassInstantiationCheckerTest extends CheckerTestCase { checkErrorLine(13); } + // struct MyInterface { + // virtual void doIt() = 0; + // }; + // + // struct Empty: virtual public MyInterface {}; + // + // struct Implementer: virtual public MyInterface { + // virtual void doIt(); + // }; + // + // struct Multiple: virtual public Implementer, virtual public Empty {}; + // + // static Multiple sharedMultiple; + public void testDiamondInheritanceWithOneImplementor_bug419938() { + loadCodeAndRun(getAboveComment()); + checkNoErrors(); + } + // struct A { // virtual void test(int) = 0; // virtual ~A(); diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/SemanticQueries.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/SemanticQueries.java index 18e0af64b33..7d397029c6c 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/SemanticQueries.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/dom/ast/cpp/SemanticQueries.java @@ -23,6 +23,7 @@ import java.util.Set; import org.eclipse.cdt.core.dom.ast.IASTNode; import org.eclipse.cdt.core.dom.ast.IBinding; import org.eclipse.cdt.core.dom.ast.IType; +import org.eclipse.cdt.core.parser.util.CollectionUtils; import org.eclipse.cdt.internal.core.dom.parser.cpp.ClassTypeHelper; import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.CPPTemplates; import org.eclipse.cdt.internal.core.dom.parser.cpp.semantics.SemanticUtil; @@ -107,51 +108,57 @@ public class SemanticQueries { * can contain multiple subobjects of the same type (if multiple, non-virtual * inheritance is used), and the pure virtual methods of each subobject must * be implemented independently, we give each subobject of a given type a - * number, and for each method we keep track of the final overrider for each - * subobject number. + * number, and for each method we keep track of the final overriders for each + * subobject number. Generally, there should be only one final overrider per + * subobject (in fact the program is ill-formed if there is more than one), + * but to accurately detect pure virtual methods that haven't been overridden, + * we need to be able to keep track of more than one at a time. */ private static class FinalOverriderMap { - private Map> fMap = new HashMap>(); + private Map>> fMap + = new HashMap>>(); /** - * Record 'overrider' as being the final ovverider of 'method' in subobject - * 'subobjectNumber'. + * Add 'overrider' as a final ovverider of 'method' in subobject + * 'subobjectNumber'. */ public void add(ICPPMethod method, int subobjectNumber, ICPPMethod overrider) { - Map overriders = fMap.get(method); + Map> overriders = fMap.get(method); if (overriders == null) { - overriders = new HashMap(); + overriders = new HashMap>(); fMap.put(method, overriders); } - overriders.put(subobjectNumber, overrider); + CollectionUtils.listMapGet(overriders, subobjectNumber).add(overrider); } /** - * For each subobject for which 'method' has been overridden, update - * its final overrider to 'overrider'. + * For each subobject for which 'method' has been overridden, set + * 'overrider' to be its (only) final overrider. */ public void replaceForAllSubobjects(ICPPMethod method, ICPPMethod overrider) { - Map overriders = fMap.get(method); + Map> overriders = fMap.get(method); if (overriders == null) return; - for (Integer i : overriders.keySet()) - overriders.put(i, overrider); + for (Integer i : overriders.keySet()) { + List overridersForSubobject = CollectionUtils.listMapGet(overriders, i); + overridersForSubobject.clear(); + overridersForSubobject.add(overrider); + } } /** - * Merge the final overriders from another FinalOverriderMap into this one. + * Merge the final overriders from another FinalOverriderMap into this one. */ public void addOverriders(FinalOverriderMap other) { for (ICPPMethod method : other.fMap.keySet()) { - Map overriders = fMap.get(method); + Map> overriders = fMap.get(method); if (overriders == null) { - overriders = new HashMap(); + overriders = new HashMap>(); fMap.put(method, overriders); } - Map otherOverriders = other.fMap.get(method); + Map> otherOverriders = other.fMap.get(method); for (Integer i : otherOverriders.keySet()) { - ICPPMethod overrider = otherOverriders.get(i); - overriders.put(i, overrider); + CollectionUtils.listMapGet(overriders, i).addAll(otherOverriders.get(i)); } } } @@ -159,17 +166,17 @@ public class SemanticQueries { /** * Go through the final overrider map and find functions which are * pure virtual in the hierarchy's root. These are functions which - * are declared pure virtual, and whose final overrider is themself, - * meaning they have not been overridden. + * are declared pure virtual, and which have a single final overrider + * which is themself, meaning they have not been overridden. */ public ICPPMethod[] collectPureVirtualMethods() { List pureVirtualMethods = new ArrayList(); for (ICPPMethod method : fMap.keySet()) { if (method.isPureVirtual()) { - Map finalOverriders = fMap.get(method); + Map> finalOverriders = fMap.get(method); for (Integer subobjectNumber : finalOverriders.keySet()) { - ICPPMethod finalOverrider = finalOverriders.get(subobjectNumber); - if (finalOverrider == method) { + List overridersForSubobject = finalOverriders.get(subobjectNumber); + if (overridersForSubobject.size() == 1 && overridersForSubobject.get(0) == method) { pureVirtualMethods.add(method); } } @@ -237,6 +244,7 @@ public class SemanticQueries { baseOverriderMap = virtualBaseCache.get(baseType); if (baseOverriderMap == null) { baseOverriderMap = collectFinalOverriders(baseType, true, inheritanceChain, point); + virtualBaseCache.put(baseType, baseOverriderMap); } } else { baseOverriderMap = collectFinalOverriders(baseType, false, inheritanceChain, point); diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/parser/util/CollectionUtils.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/parser/util/CollectionUtils.java index 9f879d86beb..c4df0f61692 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/parser/util/CollectionUtils.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/core/parser/util/CollectionUtils.java @@ -11,10 +11,12 @@ *******************************************************************************/ package org.eclipse.cdt.core.parser.util; +import java.util.ArrayList; import java.util.Collection; import java.util.Iterator; import java.util.List; import java.util.ListIterator; +import java.util.Map; /** * Useful utility methods for dealing with Collections. @@ -133,4 +135,19 @@ public final class CollectionUtils { c1.addAll(c2); return c1; } + + /** + * Gets a List corresponding to a T in a Map>. + * If the mapping doesn't exist, create it, with the empty + * list as the initial value. + * @since 5.6 + */ + static public List listMapGet(Map> m, T t) { + List result = m.get(t); + if (result == null) { + result = new ArrayList(); + m.put(t, result); + } + return result; + } }