From 4a06dab30ab9889e31eb67f9221d7fcd1e6e86d8 Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar Date: Mon, 28 Oct 2013 15:12:18 +0100 Subject: [PATCH] CppEditor: Handle classes that do not override ...when searching the overrides for virtual functions. In case there is no override for the static type of a function call expression, make sure to: 1) include the last provided override (look up bases) 2) and all overrides whose classes are derived from that static type Task-number: QTCREATORBUG-10470 Change-Id: I2c01bfdc6cb35c5a01a000ebd81a2b322ce2b795 Reviewed-by: Orgad Shaneh Reviewed-by: Erik Verbruggen --- src/plugins/cppeditor/cppeditorplugin.h | 5 + .../cppeditor/cppfollowsymbolundercursor.cpp | 116 +++++++++++---- .../cppvirtualfunctionassistprovider.cpp | 51 ++++++- .../cppvirtualfunctionassistprovider.h | 4 +- .../followsymbol_switchmethoddecldef_test.cpp | 139 +++++++++++++++++- 5 files changed, 276 insertions(+), 39 deletions(-) diff --git a/src/plugins/cppeditor/cppeditorplugin.h b/src/plugins/cppeditor/cppeditorplugin.h index 1a16db679a6..96339e10406 100644 --- a/src/plugins/cppeditor/cppeditorplugin.h +++ b/src/plugins/cppeditor/cppeditorplugin.h @@ -141,6 +141,11 @@ private slots: void test_FollowSymbolUnderCursor_virtualFunctionCall_fallbackToDeclaration(); void test_FollowSymbolUnderCursor_virtualFunctionCall_itemOrder(); void test_FollowSymbolUnderCursor_virtualFunctionCall_instantiatedSymbols(); + void test_FollowSymbolUnderCursor_virtualFunctionCall_QSharedPointer(); + void test_FollowSymbolUnderCursor_virtualFunctionCall_multipeDocuments(); + void test_FollowSymbolUnderCursor_virtualFunctionCall_noSiblings_references(); + void test_FollowSymbolUnderCursor_virtualFunctionCall_noSiblings_pointers(); + void test_FollowSymbolUnderCursor_virtualFunctionCall_noSiblings_noBaseExpression(); void test_FollowSymbolUnderCursor_virtualFunctionCall_onDotMemberAccessOfReferenceTypes(); void test_FollowSymbolUnderCursor_virtualFunctionCall_notOnDotMemberAccessOfNonReferenceType(); void test_FollowSymbolUnderCursor_virtualFunctionCall_notOnQualified(); diff --git a/src/plugins/cppeditor/cppfollowsymbolundercursor.cpp b/src/plugins/cppeditor/cppfollowsymbolundercursor.cpp index 10ddd4d4937..fa1a40d6996 100644 --- a/src/plugins/cppeditor/cppfollowsymbolundercursor.cpp +++ b/src/plugins/cppeditor/cppfollowsymbolundercursor.cpp @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -60,75 +61,85 @@ public: VirtualFunctionHelper(const TypeOfExpression &typeOfExpression, Scope *scope, const Document::Ptr &document, - const Snapshot &snapshot); + const Snapshot &snapshot, + SymbolFinder *symbolFinder); - bool canLookupVirtualFunctionOverrides(const Function *function) const; + bool canLookupVirtualFunctionOverrides(Function *function); + + /// Returns != 0 if canLookupVirtualFunctionOverrides() succeeded. + Class *staticClassOfFunctionCallExpression() const + { return m_staticClassOfFunctionCallExpression; } private: VirtualFunctionHelper(); Q_DISABLE_COPY(VirtualFunctionHelper) - ExpressionAST *getBaseExpressionAST() const - { - if (!m_expressionAST) - return 0; - CallAST *callAST = m_expressionAST->asCall(); - if (!callAST) - return 0; - if (ExpressionAST *baseExpressionAST = callAST->base_expression) - return baseExpressionAST; - return 0; - } + Class *staticClassOfFunctionCallExpression_internal() const; private: + // Provided const QSharedPointer m_typeOfExpression; - ExpressionAST *m_expressionAST; const Document::Ptr m_expressionDocument; Scope *m_scope; const Document::Ptr &m_document; const Snapshot &m_snapshot; + SymbolFinder *m_finder; + + // Determined + ExpressionAST *m_baseExpressionAST; + Function *m_function; + int m_accessTokenKind; + Class *m_staticClassOfFunctionCallExpression; // Output }; VirtualFunctionHelper::VirtualFunctionHelper(const TypeOfExpression &typeOfExpression, Scope *scope, const Document::Ptr &document, - const Snapshot &snapshot) - : m_expressionAST(typeOfExpression.expressionAST()) - , m_expressionDocument(typeOfExpression.context().expressionDocument()) + const Snapshot &snapshot, + SymbolFinder *finder) + : m_expressionDocument(typeOfExpression.context().expressionDocument()) , m_scope(scope) , m_document(document) , m_snapshot(snapshot) + , m_finder(finder) + , m_baseExpressionAST(0) + , m_function(0) + , m_accessTokenKind(0) + , m_staticClassOfFunctionCallExpression(0) { + if (ExpressionAST *expressionAST = typeOfExpression.expressionAST()) { + if (CallAST *callAST = expressionAST->asCall()) { + if (ExpressionAST *baseExpressionAST = callAST->base_expression) + m_baseExpressionAST = baseExpressionAST; + } + } } -bool VirtualFunctionHelper::canLookupVirtualFunctionOverrides(const Function *function) const +bool VirtualFunctionHelper::canLookupVirtualFunctionOverrides(Function *function) { - if (!m_expressionDocument || !m_document || !function || !m_scope || m_scope->isClass() - || m_snapshot.isEmpty()) { + m_function = function; + if (!m_function || !m_baseExpressionAST || !m_expressionDocument || !m_document || !m_scope + || m_scope->isClass() || m_snapshot.isEmpty()) { return false; } - ExpressionAST *baseExpressionAST = getBaseExpressionAST(); - if (!baseExpressionAST) - return false; - bool result = false; - if (IdExpressionAST *idExpressionAST = baseExpressionAST->asIdExpression()) { + if (IdExpressionAST *idExpressionAST = m_baseExpressionAST->asIdExpression()) { NameAST *name = idExpressionAST->name; const bool nameIsQualified = name && name->asQualifiedName(); result = !nameIsQualified && FunctionHelper::isVirtualFunction(function, m_snapshot); - } else if (MemberAccessAST *memberAccessAST = baseExpressionAST->asMemberAccess()) { + } else if (MemberAccessAST *memberAccessAST = m_baseExpressionAST->asMemberAccess()) { NameAST *name = memberAccessAST->member_name; const bool nameIsQualified = name && name->asQualifiedName(); if (!nameIsQualified && FunctionHelper::isVirtualFunction(function, m_snapshot)) { TranslationUnit *unit = m_expressionDocument->translationUnit(); QTC_ASSERT(unit, return false); - const int accessTokenKind = unit->tokenKind(memberAccessAST->access_token); + m_accessTokenKind = unit->tokenKind(memberAccessAST->access_token); - if (accessTokenKind == T_ARROW) { + if (m_accessTokenKind == T_ARROW) { result = true; - } else if (accessTokenKind == T_DOT) { + } else if (m_accessTokenKind == T_DOT) { TypeOfExpression typeOfExpression; typeOfExpression.init(m_document, m_snapshot); typeOfExpression.setExpandTemplates(true); @@ -143,6 +154,50 @@ bool VirtualFunctionHelper::canLookupVirtualFunctionOverrides(const Function *fu } } + if (!result) + return false; + return (m_staticClassOfFunctionCallExpression = staticClassOfFunctionCallExpression_internal()); +} + +/// For "f()" in "class C { void g() { f(); };" return class C. +/// For "c->f()" in "{ C *c; c->f(); }" return class C. +Class *VirtualFunctionHelper::staticClassOfFunctionCallExpression_internal() const +{ + if (!m_finder) + return 0; + + Class *result = 0; + + if (m_baseExpressionAST->asIdExpression()) { + for (Scope *s = m_scope; s ; s = s->enclosingScope()) { + if (Function *function = s->asFunction()) { + result = m_finder->findMatchingClassDeclaration(function, m_snapshot); + break; + } + } + } else if (MemberAccessAST *memberAccessAST = m_baseExpressionAST->asMemberAccess()) { + QTC_ASSERT(m_accessTokenKind == T_ARROW || m_accessTokenKind == T_DOT, return result); + TypeOfExpression typeOfExpression; + typeOfExpression.init(m_document, m_snapshot); + typeOfExpression.setExpandTemplates(true); + const QList items = typeOfExpression(memberAccessAST->base_expression, + m_document, m_scope); + ResolveExpression resolveExpression(typeOfExpression.context()); + ClassOrNamespace *binding = resolveExpression.baseExpression(items, m_accessTokenKind); + if (binding) { + if (Class *klass = binding->rootClass()) { + result = klass; + } else { + const QList symbols = binding->symbols(); + if (!symbols.isEmpty()) { + Symbol * const first = symbols.first(); + if (first->isForwardClassDeclaration()) + result = m_finder->findMatchingClassDeclaration(first, m_snapshot); + } + } + } + } + return result; } @@ -590,11 +645,12 @@ BaseTextEditorWidget::Link FollowSymbolUnderCursor::findLink(const QTextCursor & // Consider to show a pop-up displaying overrides for the function Function *function = symbol->type()->asFunctionType(); - VirtualFunctionHelper helper(*typeOfExpression, scope, doc, snapshot); + VirtualFunctionHelper helper(*typeOfExpression, scope, doc, snapshot, symbolFinder); if (helper.canLookupVirtualFunctionOverrides(function)) { VirtualFunctionAssistProvider::Parameters params; params.function = function; + params.staticClass = helper.staticClassOfFunctionCallExpression(); params.typeOfExpression = typeOfExpression; params.snapshot = snapshot; params.cursorPosition = cursor.position(); diff --git a/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp b/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp index dcf2718bc05..f3151d64873 100644 --- a/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp +++ b/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp @@ -133,6 +133,7 @@ public: IAssistProposal *perform(const IAssistInterface *) { QTC_ASSERT(m_params.function, return 0); + QTC_ASSERT(m_params.staticClass, return 0); QTC_ASSERT(!m_params.snapshot.isEmpty(), return 0); Class *functionsClass = m_finder.findMatchingClassDeclaration(m_params.function, @@ -140,8 +141,8 @@ public: if (!functionsClass) return 0; - const QList overrides - = FunctionHelper::overrides(m_params.function, functionsClass, m_params.snapshot); + const QList overrides = FunctionHelper::overrides( + m_params.function, functionsClass, m_params.staticClass, m_params.snapshot); if (overrides.isEmpty()) return 0; @@ -254,11 +255,40 @@ bool FunctionHelper::isPureVirtualFunction(const Function *function, const Snaps return isVirtualFunction_helper(function, snapshot, PureVirtual); } +static bool isDerivedOf(Class *derivedClassCandidate, Class *baseClass, + const Snapshot &snapshot) +{ + QTC_ASSERT(derivedClassCandidate && baseClass, return false); + + QList l = QList() << CppClass(derivedClassCandidate); + + while (!l.isEmpty()) { + CppClass clazz = l.takeFirst(); + QTC_ASSERT(clazz.declaration, continue); + + const QString fileName = QString::fromUtf8(clazz.declaration->fileName()); + const Document::Ptr document = snapshot.document(fileName); + if (!document) + continue; + const LookupContext context(document, snapshot); + clazz.lookupBases(clazz.declaration, context); + + foreach (const CppClass &base, clazz.bases) { + if (base.declaration == baseClass) + return true; + if (!l.contains(base)) + l << base; + } + } + + return false; +} + QList FunctionHelper::overrides(Function *function, Class *functionsClass, - const Snapshot &snapshot) + Class *staticClass, const Snapshot &snapshot) { QList result; - QTC_ASSERT(function && functionsClass, return result); + QTC_ASSERT(function && functionsClass && staticClass, return result); FullySpecifiedType referenceType = function->type(); const Name *referenceName = function->name(); @@ -274,15 +304,22 @@ QList FunctionHelper::overrides(Function *function, Class *functionsCl while (!l.isEmpty()) { // Add derived CppClass clazz = l.takeFirst(); + + QTC_ASSERT(clazz.declaration, continue); + Class *c = clazz.declaration->asClass(); + QTC_ASSERT(c, continue); + + if (c != functionsClass && c != staticClass) { + if (!isDerivedOf(c, staticClass, snapshot)) + continue; + } + foreach (const CppClass &d, clazz.derived) { if (!l.contains(d)) l << d; } // Check member functions - QTC_ASSERT(clazz.declaration, continue); - Class *c = clazz.declaration->asClass(); - QTC_ASSERT(c, continue); for (int i = 0, total = c->memberCount(); i < total; ++i) { Symbol *candidate = c->memberAt(i); const Name *candidateName = candidate->name(); diff --git a/src/plugins/cppeditor/cppvirtualfunctionassistprovider.h b/src/plugins/cppeditor/cppvirtualfunctionassistprovider.h index 3aa0dfb68fa..3a99fc37f86 100644 --- a/src/plugins/cppeditor/cppvirtualfunctionassistprovider.h +++ b/src/plugins/cppeditor/cppvirtualfunctionassistprovider.h @@ -48,9 +48,10 @@ public: VirtualFunctionAssistProvider(); struct Parameters { - Parameters() : function(0), cursorPosition(-1), openInNextSplit(false) {} + Parameters() : function(0), staticClass(0), cursorPosition(-1), openInNextSplit(false) {} CPlusPlus::Function *function; + CPlusPlus::Class *staticClass; QSharedPointer typeOfExpression; // Keeps instantiated symbols. CPlusPlus::Snapshot snapshot; int cursorPosition; @@ -80,6 +81,7 @@ public: static QList overrides(CPlusPlus::Function *function, CPlusPlus::Class *functionsClass, + CPlusPlus::Class *staticClass, const CPlusPlus::Snapshot &snapshot); }; diff --git a/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp b/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp index 6f9cff10c7f..e34f3f97d3b 100644 --- a/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp +++ b/src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp @@ -176,7 +176,9 @@ public: , editor(0) , editorWidget(0) { - QVERIFY(initialCursorPosition != targetCursorPosition); + if (initialCursorPosition != -1 || targetCursorPosition != -1) + QVERIFY(initialCursorPosition != targetCursorPosition); + if (initialCursorPosition > targetCursorPosition) { source.remove(initialCursorPosition, 1); if (targetCursorPosition != -1) { @@ -1420,6 +1422,141 @@ void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_instantia test.run(); } +/// Check: Static type is nicely resolved, especially for QSharedPointers. +void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_QSharedPointer() +{ + const QByteArray source = + "template \n" + "class Basic\n" + "{\n" + "public:\n" + " inline T &operator*() const;\n" + " inline T *operator->() const;\n" + "};\n" + "\n" + "template class ExternalRefCount: public Basic {};\n" + "template class QSharedPointer: public ExternalRefCount {};\n" + "\n" + "struct A { virtual void virt() {} };\n" + "struct B : public A { void virt() {} };\n" + "\n" + "int f()\n" + "{\n" + " QSharedPointer p(new A);\n" + " p->$@virt();\n" + "}\n" + ; + + const OverrideItemList immediateResults = OverrideItemList() + << OverrideItem(QLatin1String("A::virt"), 12) + << OverrideItem(QLatin1String("...searching overrides")); + const OverrideItemList finalResults = OverrideItemList() + << OverrideItem(QLatin1String("A::virt"), 12) + << OverrideItem(QLatin1String("B::virt"), 13); + + TestCase test(TestCase::FollowSymbolUnderCursorAction, source, immediateResults, finalResults); + test.run(); +} + +/// Check: Base classes can be found although these might be defined in distinct documents. +void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_multipeDocuments() +{ + QList testFiles = QList() + << TestDocument::create("struct A { virtual void virt(int) = 0; };\n", + QLatin1String("a.h")) + << TestDocument::create("#include \"a.h\"\n" + "struct B : A { void virt(int) {} };\n", + QLatin1String("b.h")) + << TestDocument::create("#include \"a.h\"\n" + "void f(A *o) { o->$@virt(42); }\n", + QLatin1String("u.cpp")) + ; + + const OverrideItemList immediateResults = OverrideItemList() + << OverrideItem(QLatin1String("A::virt"), 1) + << OverrideItem(QLatin1String("...searching overrides")); + const OverrideItemList finalResults = OverrideItemList() + << OverrideItem(QLatin1String("A::virt"), 1) + << OverrideItem(QLatin1String("B::virt"), 2); + + TestCase test(TestCase::FollowSymbolUnderCursorAction, testFiles, immediateResults, + finalResults); + test.run(); +} + +/// Check: In case there is no override for the static type of a function call expression, +/// make sure to: +/// 1) include the last provided override (look up bases) +/// 2) and all overrides whose classes are derived from that static type +void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_noSiblings_references() +{ + const QByteArray source = + "struct A { virtual void virt(); };\n" + "struct B : A { void virt() {} };\n" + "struct C1 : B { void virt() {} };\n" + "struct C2 : B { };\n" + "struct D : C2 { void virt() {} };\n" + "\n" + "void f(C2 &o) { o.$@virt(); }\n" + ; + + const OverrideItemList immediateResults = OverrideItemList() + << OverrideItem(QLatin1String("B::virt"), 2) + << OverrideItem(QLatin1String("...searching overrides")); + const OverrideItemList finalResults = OverrideItemList() + << OverrideItem(QLatin1String("B::virt"), 2) + << OverrideItem(QLatin1String("D::virt"), 5); + + TestCase test(TestCase::FollowSymbolUnderCursorAction, source, immediateResults, finalResults); + test.run(); +} + +/// Variation of test_FollowSymbolUnderCursor_virtualFunctionCall_noSiblings_references +void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_noSiblings_pointers() +{ + const QByteArray source = + "struct A { virtual void virt(); };\n" + "struct B : A { void virt() {} };\n" + "struct C1 : B { void virt() {} };\n" + "struct C2 : B { };\n" + "struct D : C2 { void virt() {} };\n" + "\n" + "void f(C2 *o) { o->$@virt(); }\n" + ; + + const OverrideItemList immediateResults = OverrideItemList() + << OverrideItem(QLatin1String("B::virt"), 2) + << OverrideItem(QLatin1String("...searching overrides")); + const OverrideItemList finalResults = OverrideItemList() + << OverrideItem(QLatin1String("B::virt"), 2) + << OverrideItem(QLatin1String("D::virt"), 5); + + TestCase test(TestCase::FollowSymbolUnderCursorAction, source, immediateResults, finalResults); + test.run(); +} + +/// Variation of test_FollowSymbolUnderCursor_virtualFunctionCall_noSiblings_references +void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_noSiblings_noBaseExpression() +{ + const QByteArray source = + "struct A { virtual void virt() {} };\n" + "struct B : A { void virt() {} };\n" + "struct C1 : B { void virt() {} };\n" + "struct C2 : B { void g() { $@virt(); } };\n" + "struct D : C2 { void virt() {} };\n" + ; + + const OverrideItemList immediateResults = OverrideItemList() + << OverrideItem(QLatin1String("B::virt"), 2) + << OverrideItem(QLatin1String("...searching overrides")); + const OverrideItemList finalResults = OverrideItemList() + << OverrideItem(QLatin1String("B::virt"), 2) + << OverrideItem(QLatin1String("D::virt"), 5); + + TestCase test(TestCase::FollowSymbolUnderCursorAction, source, immediateResults, finalResults); + test.run(); +} + /// Check: Trigger on a.virt() if a is of type &A. void CppEditorPlugin::test_FollowSymbolUnderCursor_virtualFunctionCall_onDotMemberAccessOfReferenceTypes() {