feedc0de/qt-creator
1
0
Fork 0
forked from qt-creator/qt-creator
Code Pull Requests Activity

CppEditor: Properly handle multiple inheritance

Browse Source 
... in "Insert Virtual Functions" quickfix.

Fixes: QTCREATORBUG-12223
Change-Id: I7dad7c219017a8c7b10b08190e35d1899ca5dfe6
Reviewed-by: Christian Stenger <christian.stenger@qt.io>
This commit is contained in:
Christian Kandeler
2020-08-17 12:02:47 +02:00
parent 919dc86c37
commit 86728b84f1
3 changed files with 175 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

147
src/plugins/cppeditor/cppinsertvirtualmethods.cpp
View File

@@ -584,21 +584,23 @@ public:
if (!name || name->asDestructorNameId()) if (!name || name->asDestructorNameId())
continue; continue;
const Function *firstVirtual = nullptr; QList<const Function * > firstVirtuals;
const bool isVirtual = FunctionUtils::isVirtualFunction( const bool isVirtual = FunctionUtils::isVirtualFunction(
func, interface.context(), &firstVirtual); func, interface.context(), &firstVirtuals);
if (!isVirtual) if (!isVirtual)
continue; continue;
if (func->isFinal()) { if (func->isFinal()) {
if (FunctionItem *first = virtualFunctions[firstVirtual]) { for (const Function *firstVirtual : qAsConst(firstVirtuals)) {
FunctionItem *next = nullptr; if (FunctionItem *first = virtualFunctions[firstVirtual]) {
for (FunctionItem *removed = first; next != first; removed = next) { FunctionItem *next = nullptr;
next = removed->nextOverride; for (FunctionItem *removed = first; next != first; removed = next) {
m_factory->classFunctionModel->removeFunction(removed); next = removed->nextOverride;
delete removed; m_factory->classFunctionModel->removeFunction(removed);
}; delete removed;
virtualFunctions.remove(firstVirtual); };
virtualFunctions.remove(firstVirtual);
}
} }
continue; continue;
} }
@@ -606,15 +608,21 @@ public:
// - virtual const QMetaObject *metaObject() const; // - virtual const QMetaObject *metaObject() const;
// - virtual void *qt_metacast(const char *); // - virtual void *qt_metacast(const char *);
// - virtual int qt_metacall(QMetaObject::Call, int, void **); // - virtual int qt_metacall(QMetaObject::Call, int, void **);
if (printer.prettyName(firstVirtual->enclosingClass()->name()) bool skip = false;
== QLatin1String("QObject")) { for (const Function *firstVirtual : qAsConst(firstVirtuals)) {
const QString funcName = printer.prettyName(func->name()); if (printer.prettyName(firstVirtual->enclosingClass()->name())
if (funcName == QLatin1String("metaObject") == QLatin1String("QObject")) {
|| funcName == QLatin1String("qt_metacast") const QString funcName = printer.prettyName(func->name());
|| funcName == QLatin1String("qt_metacall")) { if (funcName == QLatin1String("metaObject")
continue; || funcName == QLatin1String("qt_metacast")
|| funcName == QLatin1String("qt_metacall")) {
skip = true;
break;
}
} }
} }
if (skip)
continue;
// Do not implement existing functions inside target class // Do not implement existing functions inside target class
bool funcExistsInClass = false; bool funcExistsInClass = false;
@@ -634,7 +642,7 @@ public:
} }
// Construct function item // Construct function item
const bool isReimplemented = (func != firstVirtual); const bool isReimplemented = !firstVirtuals.contains(func);
const bool isPureVirtual = func->isPureVirtual(); const bool isPureVirtual = func->isPureVirtual();
QString itemName = printer.prettyType(func->type(), func->name()); QString itemName = printer.prettyType(func->type(), func->name());
if (isPureVirtual) if (isPureVirtual)
@@ -648,16 +656,24 @@ public:
factory->setHasReimplementedFunctions(true); factory->setHasReimplementedFunctions(true);
funcItem->reimplemented = true; funcItem->reimplemented = true;
funcItem->alreadyFound = funcExistsInClass; funcItem->alreadyFound = funcExistsInClass;
if (FunctionItem *first = virtualFunctions[firstVirtual]) { for (const Function *firstVirtual : qAsConst(firstVirtuals)) {
if (!first->alreadyFound) { if (FunctionItem *first = virtualFunctions[firstVirtual]) {
while (first->checked != isPureVirtual) { if (!first->alreadyFound) {
first->checked = isPureVirtual; while (first->checked != isPureVirtual) {
first = first->nextOverride; first->checked = isPureVirtual;
first = first->nextOverride;
}
} }
funcItem->checked = first->checked;
FunctionItem *prev = funcItem;
for (FunctionItem *next = funcItem->nextOverride;
next && next != funcItem; next = next->nextOverride) {
prev = next;
}
prev->nextOverride = first->nextOverride;
first->nextOverride = funcItem;
} }
funcItem->checked = first->checked;
funcItem->nextOverride = first->nextOverride;
first->nextOverride = funcItem;
} }
} else { } else {
if (!funcExistsInClass) { if (!funcExistsInClass) {
@@ -758,6 +774,7 @@ public:
targetCoN = targetContext.globalNamespace(); targetCoN = targetContext.globalNamespace();
UseMinimalNames useMinimalNames(targetCoN); UseMinimalNames useMinimalNames(targetCoN);
Control *control = context().bindings()->control().data(); Control *control = context().bindings()->control().data();
QList<const Function *> insertedFunctions;
foreach (ClassItem *classItem, m_factory->classFunctionModel->classes) { foreach (ClassItem *classItem, m_factory->classFunctionModel->classes) {
if (classItem->checkState() == Qt::Unchecked) if (classItem->checkState() == Qt::Unchecked)
continue; continue;
@@ -769,6 +786,14 @@ public:
if (funcItem->reimplemented || funcItem->alreadyFound || !funcItem->checked) if (funcItem->reimplemented || funcItem->alreadyFound || !funcItem->checked)
continue; continue;
const auto cmp = [funcItem](const Function *f) {
return f->name()->match(funcItem->function->name())
&& f->type().match(funcItem->function->type());
};
if (Utils::contains(insertedFunctions, cmp))
continue;
insertedFunctions.append(funcItem->function);
if (first) { if (first) {
// Add comment // Add comment
const QString comment = QLatin1String("\n// ") + const QString comment = QLatin1String("\n// ") +
@@ -824,10 +849,13 @@ public:
} }
// Write header file // Write header file
headerFile->setChangeSet(headerChangeSet); if (!headerChangeSet.isEmpty()) {
headerFile->appendIndentRange(Utils::ChangeSet::Range(m_insertPosDecl, m_insertPosDecl + 1)); headerFile->setChangeSet(headerChangeSet);
headerFile->setOpenEditor(true, m_insertPosDecl); headerFile->appendIndentRange(Utils::ChangeSet::Range(m_insertPosDecl,
headerFile->apply(); m_insertPosDecl + 1));
headerFile->setOpenEditor(true, m_insertPosDecl);
headerFile->apply();
}
// Insert in implementation file // Insert in implementation file
if (m_factory->settings()->implementationMode if (m_factory->settings()->implementationMode
@@ -877,9 +905,12 @@ public:
implementationChangeSet.insert(insertPos, QLatin1String("\n\n") + defText); implementationChangeSet.insert(insertPos, QLatin1String("\n\n") + defText);
} }
implementationFile->setChangeSet(implementationChangeSet); if (!implementationChangeSet.isEmpty()) {
implementationFile->appendIndentRange(Utils::ChangeSet::Range(insertPos, insertPos + 1)); implementationFile->setChangeSet(implementationChangeSet);
implementationFile->apply(); implementationFile->appendIndentRange(Utils::ChangeSet::Range(insertPos,
insertPos + 1));
implementationFile->apply();
}
} }
} }
@@ -1761,6 +1792,56 @@ void CppEditorPlugin::test_quickfix_InsertVirtualMethods_data()
" virtual int d();\n" " virtual int d();\n"
"};\n" "};\n"
); );
// Check: Insert multiply-inherited virtual function only once.
QTest::newRow("multiple_inheritance_insert")
<< InsertVirtualMethodsDialog::ModeOnlyDeclarations << false << true << _(
"struct Base1 {\n"
" virtual void virt() = 0;\n"
"};\n\n"
"struct Base2 {\n"
" virtual void virt() = 0;\n"
"};\n\n"
"struct @Derived : Base1, Base2 {\n"
"};\n") << _(
"struct Base1 {\n"
" virtual void virt() = 0;\n"
"};\n\n"
"struct Base2 {\n"
" virtual void virt() = 0;\n"
"};\n\n"
"struct Derived : Base1, Base2 {\n\n"
" // Base2 interface\n"
"public:\n"
" void virt() override;\n"
"};\n");
// Check: Do not insert multiply-inherited virtual function that has been re-implemented
// along the way.
QTest::newRow("multiple_inheritance_no_insert")
<< InsertVirtualMethodsDialog::ModeOnlyDeclarations << false << true << _(
"struct Base1 {\n"
" virtual void virt() = 0;\n"
"};\n\n"
"struct Base2 {\n"
" virtual void virt() = 0;\n"
"};\n\n"
"struct Derived1 : Base1, Base2 {\n"
" void virt() override;\n"
"};\n\n"
"struct @Derived2 : Derived1\n"
"};\n") << _(
"struct Base1 {\n"
" virtual void virt() = 0;\n"
"};\n\n"
"struct Base2 {\n"
" virtual void virt() = 0;\n"
"};\n\n"
"struct Derived1 : Base1, Base2 {\n"
" void virt() override;\n"
"};\n\n"
"struct Derived2 : Derived1\n"
"};\n");
} }
void CppEditorPlugin::test_quickfix_InsertVirtualMethods() void CppEditorPlugin::test_quickfix_InsertVirtualMethods()

76
src/plugins/cpptools/functionutils.cpp
View File

@@ -33,6 +33,11 @@
#include <utils/qtcassert.h> #include <utils/qtcassert.h>
#include <QList> #include <QList>
#include <QPair>
#include <limits>
#include <cplusplus/TypePrettyPrinter.h>
using namespace CPlusPlus; using namespace CPlusPlus;
using namespace CppTools; using namespace CppTools;
@@ -42,12 +47,12 @@ enum VirtualType { Virtual, PureVirtual };
static bool isVirtualFunction_helper(const Function *function, static bool isVirtualFunction_helper(const Function *function,
const LookupContext &context, const LookupContext &context,
VirtualType virtualType, VirtualType virtualType,
const Function **firstVirtual) QList<const Function *> *firstVirtuals)
{ {
enum { Unknown, False, True } res = Unknown; enum { Unknown, False, True } res = Unknown;
if (firstVirtual) if (firstVirtuals)
*firstVirtual = nullptr; firstVirtuals->clear();
if (!function) if (!function)
return false; return false;
@@ -55,14 +60,51 @@ static bool isVirtualFunction_helper(const Function *function,
if (virtualType == PureVirtual) if (virtualType == PureVirtual)
res = function->isPureVirtual() ? True : False; res = function->isPureVirtual() ? True : False;
const Class * const klass = function->enclosingScope()
? function->enclosingScope()->asClass() : nullptr;
if (!klass)
return false;
int hierarchyDepthOfFirstVirtuals = -1;
const auto updateFirstVirtualsList
= [&hierarchyDepthOfFirstVirtuals, &context, firstVirtuals, klass](Function *candidate) {
const Class * const candidateClass = candidate->enclosingScope()
? candidate->enclosingScope()->asClass() : nullptr;
if (!candidateClass)
return;
QList<QPair<const Class *, int>> classes{{klass, 0}};
while (!classes.isEmpty()) {
const auto c = classes.takeFirst();
if (c.first == candidateClass) {
QTC_ASSERT(c.second != 0, return);
if (c.second >= hierarchyDepthOfFirstVirtuals) {
if (c.second > hierarchyDepthOfFirstVirtuals) {
firstVirtuals->clear();
hierarchyDepthOfFirstVirtuals = c.second;
}
firstVirtuals->append(candidate);
}
return;
}
for (int i = 0; i < c.first->baseClassCount(); ++i) {
const ClassOrNamespace * const base = context.lookupType(
c.first->baseClassAt(i)->name(), c.first->enclosingScope());
if (base && base->rootClass())
classes.append({base->rootClass(), c.second + 1});
}
}
};
if (function->isVirtual()) { if (function->isVirtual()) {
if (firstVirtual) if (firstVirtuals) {
*firstVirtual = function; hierarchyDepthOfFirstVirtuals = 0;
firstVirtuals->append(function);
}
if (res == Unknown) if (res == Unknown)
res = True; res = True;
} }
if (!firstVirtual && res != Unknown) if (!firstVirtuals && res != Unknown)
return res == True; return res == True;
QList<LookupItem> results = context.lookup(function->name(), function->enclosingScope()); QList<LookupItem> results = context.lookup(function->name(), function->enclosingScope());
@@ -80,11 +122,11 @@ static bool isVirtualFunction_helper(const Function *function,
if (functionType->isFinal()) if (functionType->isFinal())
return res == True; return res == True;
if (functionType->isVirtual()) { if (functionType->isVirtual()) {
if (!firstVirtual) if (!firstVirtuals)
return true; return true;
if (res == Unknown) if (res == Unknown)
res = True; res = True;
*firstVirtual = functionType; updateFirstVirtualsList(functionType);
} }
} }
} }
@@ -96,16 +138,16 @@ static bool isVirtualFunction_helper(const Function *function,
bool FunctionUtils::isVirtualFunction(const Function *function, bool FunctionUtils::isVirtualFunction(const Function *function,
const LookupContext &context, const LookupContext &context,
const Function **firstVirtual) QList<const Function *> *firstVirtuals)
{ {
return isVirtualFunction_helper(function, context, Virtual, firstVirtual); return isVirtualFunction_helper(function, context, Virtual, firstVirtuals);
} }
bool FunctionUtils::isPureVirtualFunction(const Function *function, bool FunctionUtils::isPureVirtualFunction(const Function *function,
const LookupContext &context, const LookupContext &context,
const Function **firstVirtual) QList<const Function *> *firstVirtuals)
{ {
return isVirtualFunction_helper(function, context, PureVirtual, firstVirtual); return isVirtualFunction_helper(function, context, PureVirtual, firstVirtuals);
} }
QList<Function *> FunctionUtils::overrides(Function *function, Class *functionsClass, QList<Function *> FunctionUtils::overrides(Function *function, Class *functionsClass,
@@ -191,7 +233,7 @@ void CppToolsPlugin::test_functionutils_virtualFunctions()
QCOMPARE(document->diagnosticMessages().size(), 0); QCOMPARE(document->diagnosticMessages().size(), 0);
QVERIFY(document->translationUnit()->ast()); QVERIFY(document->translationUnit()->ast());
QList<const Function *> allFunctions; QList<const Function *> allFunctions;
const Function *firstVirtual = nullptr; QList<const Function *> firstVirtuals;
// Iterate through Function symbols // Iterate through Function symbols
Snapshot snapshot; Snapshot snapshot;
@@ -206,9 +248,9 @@ void CppToolsPlugin::test_functionutils_virtualFunctions()
Virtuality virtuality = virtualityList.takeFirst(); Virtuality virtuality = virtualityList.takeFirst();
QTC_ASSERT(!firstVirtualList.isEmpty(), return); QTC_ASSERT(!firstVirtualList.isEmpty(), return);
int firstVirtualIndex = firstVirtualList.takeFirst(); int firstVirtualIndex = firstVirtualList.takeFirst();
bool isVirtual = FunctionUtils::isVirtualFunction(function, context, &firstVirtual); bool isVirtual = FunctionUtils::isVirtualFunction(function, context, &firstVirtuals);
bool isPureVirtual = FunctionUtils::isPureVirtualFunction(function, context, bool isPureVirtual = FunctionUtils::isPureVirtualFunction(function, context,
&firstVirtual); &firstVirtuals);
// Test for regressions introduced by firstVirtual // Test for regressions introduced by firstVirtual
QCOMPARE(FunctionUtils::isVirtualFunction(function, context), isVirtual); QCOMPARE(FunctionUtils::isVirtualFunction(function, context), isVirtual);
@@ -225,9 +267,9 @@ void CppToolsPlugin::test_functionutils_virtualFunctions()
QCOMPARE(virtuality, NotVirtual); QCOMPARE(virtuality, NotVirtual);
} }
if (firstVirtualIndex == -1) if (firstVirtualIndex == -1)
QVERIFY(!firstVirtual); QVERIFY(firstVirtuals.isEmpty());
else else
QCOMPARE(firstVirtual, allFunctions.at(firstVirtualIndex)); QCOMPARE(firstVirtuals, {allFunctions.at(firstVirtualIndex)});
} }
} }
QVERIFY(virtualityList.isEmpty()); QVERIFY(virtualityList.isEmpty());

4
src/plugins/cpptools/functionutils.h
View File

@@ -44,11 +44,11 @@ class CPPTOOLS_EXPORT FunctionUtils
public: public:
static bool isVirtualFunction(const CPlusPlus::Function *function, static bool isVirtualFunction(const CPlusPlus::Function *function,
const CPlusPlus::LookupContext &context, const CPlusPlus::LookupContext &context,
const CPlusPlus::Function **firstVirtual = nullptr); QList<const CPlusPlus::Function *> *firstVirtuals = nullptr);
static bool isPureVirtualFunction(const CPlusPlus::Function *function, static bool isPureVirtualFunction(const CPlusPlus::Function *function,
const CPlusPlus::LookupContext &context, const CPlusPlus::LookupContext &context,
const CPlusPlus::Function **firstVirtual = nullptr); QList<const CPlusPlus::Function *> *firstVirtuals = nullptr);
static QList<CPlusPlus::Function *> overrides(CPlusPlus::Function *function, static QList<CPlusPlus::Function *> overrides(CPlusPlus::Function *function,
CPlusPlus::Class *functionsClass, CPlusPlus::Class *functionsClass,