From 11f6ae4a143c2669a443ebcee693b71c310cf564 Mon Sep 17 00:00:00 2001 From: Leandro Melo Date: Wed, 15 Aug 2012 12:50:01 +0200 Subject: [PATCH] C++: Completion for templates as base classes This fixes a variety of issues regarding class completion when templates are used as base classes. The test cases show examples. Task-number: QTCREATORBUG-4357 Change-Id: I764d5ce817a78e1b19336e5beab758ca9e10f34b Reviewed-by: Roberto Raggi --- src/libs/cplusplus/LookupContext.cpp | 135 ++++++++++++++-- src/libs/cplusplus/LookupContext.h | 3 +- src/plugins/cpptools/cppcompletion_test.cpp | 169 ++++++++++++++++++++ src/plugins/cpptools/cpptoolsplugin.h | 2 + 4 files changed, 295 insertions(+), 14 deletions(-) diff --git a/src/libs/cplusplus/LookupContext.cpp b/src/libs/cplusplus/LookupContext.cpp index fc5e54c26a6..dae8eb6a023 100644 --- a/src/libs/cplusplus/LookupContext.cpp +++ b/src/libs/cplusplus/LookupContext.cpp @@ -32,6 +32,7 @@ #include "ResolveExpression.h" #include "Overview.h" #include "DeprecatedGenTemplateInstance.h" +#include "CppRewriter.h" #include #include @@ -382,9 +383,6 @@ QList ClassOrNamespace::enums() const QList ClassOrNamespace::symbols() const { - if (_templateId && ! _usings.isEmpty()) - return _usings.first()->symbols(); // ask to the base implementation - const_cast(this)->flush(); return _symbols; } @@ -608,7 +606,7 @@ ClassOrNamespace *ClassOrNamespace::lookupType_helper(const Name *name, return 0; } -ClassOrNamespace *ClassOrNamespace::nestedType(const Name *name, ClassOrNamespace *origin) const +ClassOrNamespace *ClassOrNamespace::nestedType(const Name *name, ClassOrNamespace *origin) { Q_ASSERT(name != 0); Q_ASSERT(name->isNameId() || name->isTemplateNameId()); @@ -616,21 +614,131 @@ ClassOrNamespace *ClassOrNamespace::nestedType(const Name *name, ClassOrNamespac const_cast(this)->flush(); Table::const_iterator it = _classOrNamespaces.find(name); - if (it == _classOrNamespaces.end()) return 0; - ClassOrNamespace *c = it->second; + ClassOrNamespace *reference = it->second; - if (const TemplateNameId *templId = name->asTemplateNameId()) { - ClassOrNamespace *i = _factory->allocClassOrNamespace(c); - i->_templateId = templId; - i->_instantiationOrigin = origin; - i->_usings.append(c); - return i; + // The reference binding might still be missing some of its base classes in the case they + // are templates. We need to collect them now. First, we track the bases which are already + // part of the binding so we can identify the missings ones later. + + QSet knownBases; + foreach (ClassOrNamespace *con, reference->usings()) { + foreach (Symbol *s, con->symbols()) { + if (Class *c = s->asClass()) { + knownBases.insert(c->name()); + break; + } + } } - return c; + Class *referenceClass = 0; + QList missingBases; + foreach (Symbol *s, reference->symbols()) { + if (Class *clazz = s->asClass()) { + for (unsigned i = 0; i < clazz->baseClassCount(); ++i) { + BaseClass *baseClass = clazz->baseClassAt(i); + if (baseClass->name() && !knownBases.contains(baseClass->name())) + missingBases.append(baseClass->name()); + } + referenceClass = clazz; + break; + } + } + + if (!referenceClass) + return reference; + + // If we are dealling with a template type, more work is required, since we need to + // construct all instantiation data. + if (const TemplateNameId *templId = name->asTemplateNameId()) { + ClassOrNamespace *instantiation = _factory->allocClassOrNamespace(reference); + instantiation->_templateId = templId; + instantiation->_instantiationOrigin = origin; + + // The instantiation should have all symbols, enums, and usings from the reference. + instantiation->_symbols.append(reference->symbols()); + instantiation->_enums.append(reference->enums()); + instantiation->_usings.append(reference->usings()); + + // It gets a bit complicated if the reference is actually a class template because we + // now must worry about dependent names in base classes. + if (Template *templ = referenceClass->enclosingTemplate()) { + QHash templParams; + for (unsigned i = 0; i < templ->templateParameterCount(); ++i) + templParams.insert(templ->templateParameterAt(i)->name(), i); + + foreach (const Name *baseName, missingBases) { + ClassOrNamespace *baseBinding = 0; + + if (const Identifier *nameId = baseName->asNameId()) { + // This is the simple case in which a template parameter is itself a base. + // Ex.: template class A : public T {}; + if (templParams.contains(nameId)) { + const FullySpecifiedType &fullType = + templId->templateArgumentAt(templParams.value(nameId)); + if (NamedType *namedType = fullType.type()->asNamedType()) + baseBinding = lookupType(namedType->name()); + } + } else { + SubstitutionMap map; + for (unsigned i = 0; + i < templ->templateParameterCount() && i < templId->templateArgumentCount(); + ++i) { + map.bind(templ->templateParameterAt(i)->name(), + templId->templateArgumentAt(i)); + } + SubstitutionEnvironment env; + env.enter(&map); + + baseName = rewriteName(baseName, &env, _control.data()); + + if (const TemplateNameId *baseTemplId = baseName->asTemplateNameId()) { + // Another template that uses the dependent name. + // Ex.: template class A : public B {}; + if (baseTemplId->identifier() != templId->identifier()) + baseBinding = nestedType(baseName, origin); + } else if (const QualifiedNameId *qBaseName = baseName->asQualifiedNameId()) { + // Qualified names in general. + // Ex.: template class A : public B::Type {}; + ClassOrNamespace *binding = this; + if (const Name *qualification = qBaseName->base()) + binding = lookupType(qualification); + baseName = qBaseName->name(); + + if (binding) + baseBinding = binding->lookupType(baseName); + } + } + + if (baseBinding) + instantiation->addUsing(baseBinding); + } + } + + return instantiation; + } + + // Find the missing bases for regular (non-template) types. + // Ex.: class A : public B::Type {}; + foreach (const Name *baseName, missingBases) { + ClassOrNamespace *binding = this; + if (const QualifiedNameId *qBaseName = baseName->asQualifiedNameId()) { + if (const Name *qualification = qBaseName->base()) + binding = lookupType(qualification); + baseName = qBaseName->name(); + } + + if (binding) { + ClassOrNamespace * baseBinding = binding->lookupType(baseName); + if (baseBinding) + reference->addUsing(baseBinding); + } + } + + + return reference; } void ClassOrNamespace::flush() @@ -761,6 +869,7 @@ QSharedPointer CreateBindings::control() const ClassOrNamespace *CreateBindings::allocClassOrNamespace(ClassOrNamespace *parent) { ClassOrNamespace *e = new ClassOrNamespace(this, parent); + e->_control = control(); _entities.append(e); return e; } diff --git a/src/libs/cplusplus/LookupContext.h b/src/libs/cplusplus/LookupContext.h index d9c1da39eb3..a2de7f8cadf 100644 --- a/src/libs/cplusplus/LookupContext.h +++ b/src/libs/cplusplus/LookupContext.h @@ -90,7 +90,7 @@ private: ClassOrNamespace *lookupType_helper(const Name *name, QSet *processed, bool searchInEnclosingScope, ClassOrNamespace *origin); - ClassOrNamespace *nestedType(const Name *name, ClassOrNamespace *origin) const; + ClassOrNamespace *nestedType(const Name *name, ClassOrNamespace *origin); private: struct CompareName: std::binary_function { @@ -106,6 +106,7 @@ private: Table _classOrNamespaces; QList _enums; QList _todo; + QSharedPointer _control; // it's an instantiation. const TemplateNameId *_templateId; diff --git a/src/plugins/cpptools/cppcompletion_test.cpp b/src/plugins/cpptools/cppcompletion_test.cpp index 6d4a2641f05..7d9d9f3417f 100644 --- a/src/plugins/cpptools/cppcompletion_test.cpp +++ b/src/plugins/cpptools/cppcompletion_test.cpp @@ -196,3 +196,172 @@ void CppToolsPlugin::test_completion_template_1() QVERIFY(!completions.contains("f")); QVERIFY(!completions.contains("func")); } + +void CppToolsPlugin::test_completion_template_as_base() +{ + QFETCH(QByteArray, code); + QFETCH(QStringList, expectedCompletions); + + TestData data; + data.srcText = code; + setup(&data); + + Utils::ChangeSet change; + change.insert(data.pos, "c."); + QTextCursor cursor(data.doc); + change.apply(&cursor); + data.pos += 2; + + QStringList actualCompletions = getCompletions(data); + actualCompletions.sort(); + expectedCompletions.sort(); + + QCOMPARE(actualCompletions, expectedCompletions); +} + +void CppToolsPlugin::test_completion_template_as_base_data() +{ + QTest::addColumn("code"); + QTest::addColumn("expectedCompletions"); + + QByteArray code; + QStringList completions; + + code = "\n" + "class Data { int dataMember; };\n" + "template class Other : public T { int otherMember; };\n" + "\n" + "void func() {\n" + " Other c;\n" + " @\n" + " // padding so we get the scope right\n" + "}"; + completions.append("Data"); + completions.append("dataMember"); + completions.append("Other"); + completions.append("otherMember"); + QTest::newRow("case: base as template directly") << code << completions; + + + completions.clear(); + code = "\n" + "class Data { int dataMember; };\n" + "template class Other : public T { int otherMember; };\n" + "template class More : public Other { int moreMember; };\n" + "\n" + "void func() {\n" + " More c;\n" + " @\n" + " // padding so we get the scope right\n" + "}"; + completions.append("Data"); + completions.append("dataMember"); + completions.append("Other"); + completions.append("otherMember"); + completions.append("More"); + completions.append("moreMember"); + QTest::newRow("case: base as class template") << code << completions; + + + completions.clear(); + code = "\n" + "class Data { int dataMember; };\n" + "template class Other : public T { int otherMember; };\n" + "template class More : public ::Other { int moreMember; };\n" + "\n" + "void func() {\n" + " More c;\n" + " @\n" + " // padding so we get the scope right\n" + "}"; + completions.append("Data"); + completions.append("dataMember"); + completions.append("Other"); + completions.append("otherMember"); + completions.append("More"); + completions.append("moreMember"); + QTest::newRow("case: base as globally qualified class template") << code << completions; + + + completions.clear(); + code = "\n" + "class Data { int dataMember; };\n" + "namespace NS {\n" + "template class Other : public T { int otherMember; };\n" + "}\n" + "template class More : public NS::Other { int moreMember; };\n" + "\n" + "void func() {\n" + " More c;\n" + " @\n" + " // padding so we get the scope right\n" + "}"; + completions.append("Data"); + completions.append("dataMember"); + completions.append("Other"); + completions.append("otherMember"); + completions.append("More"); + completions.append("moreMember"); + QTest::newRow("case: base as namespace qualified class template") << code << completions; + + + completions.clear(); + code = "\n" + "class Data { int dataMember; };\n" + "namespace NS {\n" + "template class Delegate { typedef Data Type; };\n" + "}\n" + "template class Final : public NS::Delegate::Type { int finalMember; };\n" + "\n" + "void func() {\n" + " Final c;\n" + " @\n" + " // padding so we get the scope right\n" + "}"; + completions.append("Data"); + completions.append("dataMember"); + completions.append("Final"); + completions.append("finalMember"); + QTest::newRow("case: base as nested template name") << code << completions; + + + completions.clear(); + code = "\n" + "class Data { int dataMember; };\n" + "namespace NS {\n" + "template class Delegate { typedef Data Type; };\n" + "}\n" + "class Final : public NS::Delegate::Type { int finalMember; };\n" + "\n" + "void func() {\n" + " Final c;\n" + " @\n" + " // padding so we get the scope right\n" + "}"; + completions.append("Data"); + completions.append("dataMember"); + completions.append("Final"); + completions.append("finalMember"); + QTest::newRow("case: base as nested template name in non-template") << code << completions; + + completions.clear(); + code = "\n" + "class Data { int dataMember; };\n" + "namespace NS {\n" + "template class Other : public T { int otherMember; };\n" + "}\n" + "class Final : public NS::Other { int finalMember; };\n" + "\n" + "void func() {\n" + " Final c;\n" + " @\n" + " // padding so we get the scope right\n" + "}"; + completions.append("Data"); + completions.append("dataMember"); + completions.append("Final"); + completions.append("finalMember"); + completions.append("Other"); + completions.append("otherMember"); + QTest::newRow("case: base as template name in non-template") << code << completions; +} diff --git a/src/plugins/cpptools/cpptoolsplugin.h b/src/plugins/cpptools/cpptoolsplugin.h index 1c89af6fd34..5a07c0add3f 100644 --- a/src/plugins/cpptools/cpptoolsplugin.h +++ b/src/plugins/cpptools/cpptoolsplugin.h @@ -92,6 +92,8 @@ private slots: void test_completion_basic_1(); void test_completion_template_1(); + void test_completion_template_as_base(); + void test_completion_template_as_base_data(); #endif private: