From b82275a4c0aeb937cd4df6d1a6dc8175a9519267 Mon Sep 17 00:00:00 2001 From: Nathan Ridge Date: Sun, 19 Jul 2015 02:17:15 -0400 Subject: [PATCH] Bug 86654 - During binding resolution, replace virtual methods with their final overrider where appropriate Change-Id: I5d6ef9ca5cf8dd4461255ef59ee3384f5060ee4e Signed-off-by: Nathan Ridge --- .../core/parser/tests/ast2/AST2CPPTests.java | 29 ++++++++++ .../parser/cpp/semantics/CPPInheritance.java | 53 ++++++++++++++++++- .../parser/cpp/semantics/CPPSemantics.java | 15 ++++++ 3 files changed, 95 insertions(+), 2 deletions(-) diff --git a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2CPPTests.java b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2CPPTests.java index 69a35126ab4..b09f9037b91 100644 --- a/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2CPPTests.java +++ b/core/org.eclipse.cdt.core.tests/parser/org/eclipse/cdt/core/parser/tests/ast2/AST2CPPTests.java @@ -9235,6 +9235,35 @@ public class AST2CPPTests extends AST2TestBase { assertEquals(0, ors.length); } + // struct A { + // virtual void f(); // A + // }; + // + // struct B : virtual A { + // virtual void f(); // B + // }; + // + // struct C : B , virtual A { + // using A::f; + // }; + // + // void foo() { + // C c; + // c.f(); + // c.C::f(); + // } + public void testResolutionToFinalOverrider_86654() throws Exception { + BindingAssertionHelper helper = getAssertionHelper(); + + ICPPMethod actual = helper.assertNonProblem("c.f()", "f"); + ICPPMethod expected = helper.assertNonProblem("virtual void f(); // B", "f"); + assertEquals(expected, actual); + + actual = helper.assertNonProblem("c.C::f()", "f"); + expected = helper.assertNonProblem("virtual void f(); // A", "f"); + assertEquals(expected, actual); + } + // struct X { // X(); // }; diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPInheritance.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPInheritance.java index dce110964db..8ca076c5029 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPInheritance.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPInheritance.java @@ -7,6 +7,7 @@ *******************************************************************************/ package org.eclipse.cdt.internal.core.dom.parser.cpp.semantics; +import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -89,10 +90,30 @@ public class CPPInheritance { } Map> otherOverriders = other.fMap.get(method); for (Integer i : otherOverriders.keySet()) { - CollectionUtils.listMapGet(overriders, i).addAll(otherOverriders.get(i)); + mergeOverriders(CollectionUtils.listMapGet(overriders, i), otherOverriders.get(i)); } } } + + /** + * Merges the overriders from 'source' into 'target'. + * Excludes methods in 'source' which themselves have an overrider in 'target'. + */ + private void mergeOverriders(List target, List source) { + List toAdd = new ArrayList<>(); + for (ICPPMethod candidate : source) { + boolean superseded = false; + for (ICPPMethod existing : target) { + if (existing != candidate && ClassTypeHelper.isOverrider(existing, candidate)) { + superseded = true; + } + } + if (!superseded) { + toAdd.add(candidate); + } + } + target.addAll(toAdd); + } } /** @@ -116,12 +137,34 @@ public class CPPInheritance { if (result == null) { result = FinalOverriderAnalysis.computeFinalOverriderMap(classType, point); } - if (result != null && cache != null) { + if (cache != null) { cache.put(classType, result); } return result; } + + /** + * If a given virtual method has a unique final overrider in the class hierarchy rooted at the + * given class, returns that final overrider. Otherwise, returns null. + * @param point The point of template instantiation, if applicable. + * Also used to access the final overrider map cache in the AST. + */ + public static ICPPMethod getFinalOverrider(ICPPMethod method, ICPPClassType hierarchyRoot, + IASTNode point) { + FinalOverriderMap map = getFinalOverriderMap(hierarchyRoot, point); + Map> finalOverriders = map.getMap().get(method); + if (finalOverriders != null && finalOverriders.size() == 1) { + for (Integer subobjectNumber : finalOverriders.keySet()) { + List overridersForSubobject = finalOverriders.get(subobjectNumber); + if (overridersForSubobject.size() == 1) { + return overridersForSubobject.get(0); + } + } + } + return null; + } + private static class FinalOverriderAnalysis { /** * Computes the final overrider map for a class hierarchy. @@ -207,6 +250,12 @@ public class CPPInheritance { // Go through our own methods. for (ICPPMethod method : ClassTypeHelper.getOwnMethods(classType, point)) { + // Skip methods that don't actually belong to us, such as methods brought + // into scope via a using-declaration. + if (method.getOwner() != classType) { + continue; + } + // For purposes of this computation, every virtual method is // deemed for override itself. result.add(method, subobjectNumber, method); diff --git a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPSemantics.java b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPSemantics.java index 3c9a481d66d..2ba3db190be 100644 --- a/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPSemantics.java +++ b/core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/semantics/CPPSemantics.java @@ -559,6 +559,21 @@ public class CPPSemantics { } } } + + // If the result is a virtual method called without explicit qualification, and we can determine a + // unique final overrider for it in the hierarchy of the method call's implied object type, replace + // the method with its final overrider. + if (!(lookupName.getParent() instanceof ICPPASTQualifiedName) && binding instanceof ICPPMethod && + ((ICPPMethod) binding).isVirtual()) { + IType impliedObjectType = data.getImpliedObjectType(); + if (impliedObjectType instanceof ICPPClassType) { + ICPPMethod finalOverrider = CPPInheritance.getFinalOverrider((ICPPMethod) binding, + (ICPPClassType) impliedObjectType, lookupName); + if (finalOverrider != null) { + binding = finalOverrider; + } + } + } // If we're still null... if (binding == null) {