From 056db856579b53f195965fd00d522de5ea745835 Mon Sep 17 00:00:00 2001 From: Orgad Shaneh Date: Sun, 10 Nov 2013 23:14:17 +0200 Subject: CppEditor: Accept LookupContext in virtual function lookup Required for correct resolving of first virtual appearance Change-Id: I2307027f769fb2f4c0942f4aa4e0d2b5327562b5 Reviewed-by: Nikolai Kosjar --- .../cppeditor/cppvirtualfunctionassistprovider.cpp | 47 ++++++++++------------ 1 file changed, 22 insertions(+), 25 deletions(-) (limited to 'src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp') diff --git a/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp b/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp index 1e0f45334f..4848275466 100644 --- a/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp +++ b/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp @@ -210,7 +210,7 @@ IAssistProcessor *VirtualFunctionAssistProvider::createProcessor() const enum VirtualType { Virtual, PureVirtual }; static bool isVirtualFunction_helper(const Function *function, - const Snapshot &snapshot, + const LookupContext &context, VirtualType virtualType) { if (!function) @@ -222,24 +222,20 @@ static bool isVirtualFunction_helper(const Function *function, if (function->isVirtual()) return true; - const QString filePath = QString::fromUtf8(function->fileName(), function->fileNameLength()); - if (Document::Ptr document = snapshot.document(filePath)) { - LookupContext context(document, snapshot); - QList results = context.lookup(function->name(), function->enclosingScope()); - if (!results.isEmpty()) { - const bool isDestructor = function->name()->isDestructorNameId(); - foreach (const LookupItem &item, results) { - if (Symbol *symbol = item.declaration()) { - if (Function *functionType = symbol->type()->asFunctionType()) { - if (functionType->name()->isDestructorNameId() != isDestructor) - continue; - if (functionType == function) // already tested - continue; - if (functionType->isFinal()) - return false; - if (functionType->isVirtual()) - return true; - } + QList results = context.lookup(function->name(), function->enclosingScope()); + if (!results.isEmpty()) { + const bool isDestructor = function->name()->isDestructorNameId(); + foreach (const LookupItem &item, results) { + if (Symbol *symbol = item.declaration()) { + if (Function *functionType = symbol->type()->asFunctionType()) { + if (functionType->name()->isDestructorNameId() != isDestructor) + continue; + if (functionType == function) // already tested + continue; + if (functionType->isFinal()) + return false; + if (functionType->isVirtual()) + return true; } } } @@ -248,14 +244,14 @@ static bool isVirtualFunction_helper(const Function *function, return false; } -bool FunctionHelper::isVirtualFunction(const Function *function, const Snapshot &snapshot) +bool FunctionHelper::isVirtualFunction(const Function *function, const LookupContext &context) { - return isVirtualFunction_helper(function, snapshot, Virtual); + return isVirtualFunction_helper(function, context, Virtual); } -bool FunctionHelper::isPureVirtualFunction(const Function *function, const Snapshot &snapshot) +bool FunctionHelper::isPureVirtualFunction(const Function *function, const LookupContext &context) { - return isVirtualFunction_helper(function, snapshot, PureVirtual); + return isVirtualFunction_helper(function, context, PureVirtual); } QList FunctionHelper::overrides(Function *function, Class *functionsClass, @@ -342,6 +338,7 @@ void CppEditorPlugin::test_functionhelper_virtualFunctions() // Iterate through Function symbols Snapshot snapshot; snapshot.insert(document); + const LookupContext context(document, snapshot); Control *control = document->translationUnit()->control(); Symbol **end = control->lastSymbol(); for (Symbol **it = control->firstSymbol(); it != end; ++it) { @@ -349,8 +346,8 @@ void CppEditorPlugin::test_functionhelper_virtualFunctions() if (const Function *function = symbol->asFunction()) { QTC_ASSERT(!virtualityList.isEmpty(), return); Virtuality virtuality = virtualityList.takeFirst(); - if (FunctionHelper::isVirtualFunction(function, snapshot)) { - if (FunctionHelper::isPureVirtualFunction(function, snapshot)) + if (FunctionHelper::isVirtualFunction(function, context)) { + if (FunctionHelper::isPureVirtualFunction(function, context)) QCOMPARE(virtuality, PureVirtual); else QCOMPARE(virtuality, Virtual); -- cgit v1.2.1 From 49b9cc9883d4074f1c34db80bf7b4ba80550b678 Mon Sep 17 00:00:00 2001 From: Orgad Shaneh Date: Thu, 31 Oct 2013 20:50:49 +0200 Subject: CppEditor: Return first virtual function on is[Pure]VirtualFunction Change-Id: I086076cc58c78430e025a78703a028610024ed23 Reviewed-by: Nikolai Kosjar --- .../cppeditor/cppvirtualfunctionassistprovider.cpp | 103 +++++++++++++++------ 1 file changed, 77 insertions(+), 26 deletions(-) (limited to 'src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp') diff --git a/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp b/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp index 4848275466..86f802119e 100644 --- a/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp +++ b/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp @@ -211,16 +211,29 @@ enum VirtualType { Virtual, PureVirtual }; static bool isVirtualFunction_helper(const Function *function, const LookupContext &context, - VirtualType virtualType) + VirtualType virtualType, + const Function **firstVirtual) { + enum { Unknown, False, True } res = Unknown; + + if (firstVirtual) + *firstVirtual = 0; + if (!function) return false; if (virtualType == PureVirtual) - return function->isPureVirtual(); + res = function->isPureVirtual() ? True : False; + + if (function->isVirtual()) { + if (firstVirtual) + *firstVirtual = function; + if (res == Unknown) + res = True; + } - if (function->isVirtual()) - return true; + if (!firstVirtual && res != Unknown) + return res == True; QList results = context.lookup(function->name(), function->enclosingScope()); if (!results.isEmpty()) { @@ -233,25 +246,34 @@ static bool isVirtualFunction_helper(const Function *function, if (functionType == function) // already tested continue; if (functionType->isFinal()) - return false; - if (functionType->isVirtual()) - return true; + return res == True; + if (functionType->isVirtual()) { + if (!firstVirtual) + return true; + if (res == Unknown) + res = True; + *firstVirtual = functionType; + } } } } } - return false; + return res == True; } -bool FunctionHelper::isVirtualFunction(const Function *function, const LookupContext &context) +bool FunctionHelper::isVirtualFunction(const Function *function, + const LookupContext &context, + const Function **firstVirtual) { - return isVirtualFunction_helper(function, context, Virtual); + return isVirtualFunction_helper(function, context, Virtual, firstVirtual); } -bool FunctionHelper::isPureVirtualFunction(const Function *function, const LookupContext &context) +bool FunctionHelper::isPureVirtualFunction(const Function *function, + const LookupContext &context, + const Function **firstVirtual) { - return isVirtualFunction_helper(function, context, PureVirtual); + return isVirtualFunction_helper(function, context, PureVirtual, firstVirtual); } QList FunctionHelper::overrides(Function *function, Class *functionsClass, @@ -320,6 +342,7 @@ typedef QList VirtualityList; Q_DECLARE_METATYPE(CppEditor::Internal::Virtuality) Q_DECLARE_METATYPE(CppEditor::Internal::VirtualityList) +Q_DECLARE_METATYPE(QList) namespace CppEditor { namespace Internal { @@ -329,11 +352,14 @@ void CppEditorPlugin::test_functionhelper_virtualFunctions() // Create and parse document QFETCH(QByteArray, source); QFETCH(VirtualityList, virtualityList); + QFETCH(QList, firstVirtualList); Document::Ptr document = Document::create(QLatin1String("virtuals")); document->setUtf8Source(source); document->check(); // calls parse(); QCOMPARE(document->diagnosticMessages().size(), 0); QVERIFY(document->translationUnit()->ast()); + QList allFunctions; + const Function *firstVirtual = 0; // Iterate through Function symbols Snapshot snapshot; @@ -342,21 +368,35 @@ void CppEditorPlugin::test_functionhelper_virtualFunctions() Control *control = document->translationUnit()->control(); Symbol **end = control->lastSymbol(); for (Symbol **it = control->firstSymbol(); it != end; ++it) { - const CPlusPlus::Symbol *symbol = *it; - if (const Function *function = symbol->asFunction()) { + if (const Function *function = (*it)->asFunction()) { + allFunctions.append(function); QTC_ASSERT(!virtualityList.isEmpty(), return); Virtuality virtuality = virtualityList.takeFirst(); - if (FunctionHelper::isVirtualFunction(function, context)) { - if (FunctionHelper::isPureVirtualFunction(function, context)) + QTC_ASSERT(!firstVirtualList.isEmpty(), return); + int firstVirtualIndex = firstVirtualList.takeFirst(); + bool isVirtual = FunctionHelper::isVirtualFunction(function, context, &firstVirtual); + bool isPureVirtual = FunctionHelper::isPureVirtualFunction(function, context, + &firstVirtual); + + // Test for regressions introduced by firstVirtual + QCOMPARE(FunctionHelper::isVirtualFunction(function, context), isVirtual); + QCOMPARE(FunctionHelper::isPureVirtualFunction(function, context), isPureVirtual); + if (isVirtual) { + if (isPureVirtual) QCOMPARE(virtuality, PureVirtual); else QCOMPARE(virtuality, Virtual); } else { QCOMPARE(virtuality, NotVirtual); } + if (firstVirtualIndex == -1) + QVERIFY(!firstVirtual); + else + QCOMPARE(firstVirtual, allFunctions.at(firstVirtualIndex)); } } QVERIFY(virtualityList.isEmpty()); + QVERIFY(firstVirtualList.isEmpty()); } void CppEditorPlugin::test_functionhelper_virtualFunctions_data() @@ -364,55 +404,66 @@ void CppEditorPlugin::test_functionhelper_virtualFunctions_data() typedef QByteArray _; QTest::addColumn("source"); QTest::addColumn("virtualityList"); + QTest::addColumn >("firstVirtualList"); QTest::newRow("none") << _("struct None { void foo() {} };\n") - << (VirtualityList() << NotVirtual); + << (VirtualityList() << NotVirtual) + << (QList() << -1); QTest::newRow("single-virtual") << _("struct V { virtual void foo() {} };\n") - << (VirtualityList() << Virtual); + << (VirtualityList() << Virtual) + << (QList() << 0); QTest::newRow("single-pure-virtual") << _("struct PV { virtual void foo() = 0; };\n") - << (VirtualityList() << PureVirtual); + << (VirtualityList() << PureVirtual) + << (QList() << 0); QTest::newRow("virtual-derived-with-specifier") << _("struct Base { virtual void foo() {} };\n" "struct Derived : Base { virtual void foo() {} };\n") - << (VirtualityList() << Virtual << Virtual); + << (VirtualityList() << Virtual << Virtual) + << (QList() << 0 << 0); QTest::newRow("virtual-derived-implicit") << _("struct Base { virtual void foo() {} };\n" "struct Derived : Base { void foo() {} };\n") - << (VirtualityList() << Virtual << Virtual); + << (VirtualityList() << Virtual << Virtual) + << (QList() << 0 << 0); QTest::newRow("not-virtual-then-virtual") << _("struct Base { void foo() {} };\n" "struct Derived : Base { virtual void foo() {} };\n") - << (VirtualityList() << NotVirtual << Virtual); + << (VirtualityList() << NotVirtual << Virtual) + << (QList() << -1 << 1); QTest::newRow("virtual-final-not-virtual") << _("struct Base { virtual void foo() {} };\n" "struct Derived : Base { void foo() final {} };\n" "struct Derived2 : Derived { void foo() {} };") - << (VirtualityList() << Virtual << Virtual << NotVirtual); + << (VirtualityList() << Virtual << Virtual << NotVirtual) + << (QList() << 0 << 0 << -1); QTest::newRow("virtual-then-pure") << _("struct Base { virtual void foo() {} };\n" "struct Derived : Base { virtual void foo() = 0; };\n" "struct Derived2 : Derived { void foo() {} };") - << (VirtualityList() << Virtual << PureVirtual << Virtual); + << (VirtualityList() << Virtual << PureVirtual << Virtual) + << (QList() << 0 << 0 << 0); QTest::newRow("virtual-virtual-final-not-virtual") << _("struct Base { virtual void foo() {} };\n" "struct Derived : Base { virtual void foo() final {} };\n" "struct Derived2 : Derived { void foo() {} };") - << (VirtualityList() << Virtual << Virtual << NotVirtual); + << (VirtualityList() << Virtual << Virtual << NotVirtual) + << (QList() << 0 << 0 << -1); QTest::newRow("ctor-virtual-dtor") << _("struct Base { Base() {} virtual ~Base() {} };\n") - << (VirtualityList() << NotVirtual << Virtual); + << (VirtualityList() << NotVirtual << Virtual) + << (QList() << -1 << 1); } } // namespace Internal -- cgit v1.2.1 From e20391ef6e9a8497f95d88fdb73b0fe4b3ab30bc Mon Sep 17 00:00:00 2001 From: Orgad Shaneh Date: Mon, 11 Nov 2013 22:38:03 +0200 Subject: CppEditor: Add failing tests for virtual destructor lookup Change-Id: I2fdf1c72b3e5ffe25b5184c1161a803c4427945b Reviewed-by: Nikolai Kosjar --- .../cppeditor/cppvirtualfunctionassistprovider.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp') diff --git a/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp b/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp index 86f802119e..4530a47b98 100644 --- a/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp +++ b/src/plugins/cppeditor/cppvirtualfunctionassistprovider.cpp @@ -387,6 +387,9 @@ void CppEditorPlugin::test_functionhelper_virtualFunctions() else QCOMPARE(virtuality, Virtual); } else { + QEXPECT_FAIL("virtual-dtor-dtor", "Not implemented", Abort); + if (allFunctions.size() == 3) + QEXPECT_FAIL("dtor-virtual-dtor-dtor", "Not implemented", Abort); QCOMPARE(virtuality, NotVirtual); } if (firstVirtualIndex == -1) @@ -464,6 +467,19 @@ void CppEditorPlugin::test_functionhelper_virtualFunctions_data() << _("struct Base { Base() {} virtual ~Base() {} };\n") << (VirtualityList() << NotVirtual << Virtual) << (QList() << -1 << 1); + + QTest::newRow("virtual-dtor-dtor") + << _("struct Base { virtual ~Base() {} };\n" + "struct Derived : Base { ~Derived() {} };\n") + << (VirtualityList() << Virtual << Virtual) + << (QList() << 0 << 0); + + QTest::newRow("dtor-virtual-dtor-dtor") + << _("struct Base { ~Base() {} };\n" + "struct Derived : Base { virtual ~Derived() {} };\n" + "struct Derived2 : Derived { ~Derived2() {} };\n") + << (VirtualityList() << NotVirtual << Virtual << Virtual) + << (QList() << -1 << 1 << 1); } } // namespace Internal -- cgit v1.2.1