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

Clang: Move the majority of completion items sorting to ClangBackend

Browse Source 
With this change ClangCodeModel only needs to sort completions by prefix.

Also some other optimization have become possible and are implemented here:
1. Getting completions after '{' for constructor overloads by replacing
   it with '(' inside usaved file.
2. Checking for all overloads requires only previous item check because
   all Class completions are already sorted to go before all CXXConstructor
   completions. Since they are not mixed no extra search is required.

Change-Id: Ie0187ad96a20857a63c1d71ddec74606b803f572
Reviewed-by: Nikolai Kosjar <nikolai.kosjar@qt.io>
This commit is contained in:
Ivan Donchevskii
2018-09-27 15:50:12 +02:00
parent 931ec39f64
commit efc39304a1
10 changed files with 262 additions and 104 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
share/qtcreator/cplusplus/examples/tidy_example.cpp
View File

@@ -203,6 +203,8 @@ public:
virtual bool check(bool enable = true);
};
bool Derived2::
// performance-unnecessary-value-param
void use(Base b)
{

6
src/plugins/clangcodemodel/clangassistproposalmodel.cpp
View File

@@ -56,11 +56,7 @@ void ClangAssistProposalModel::sort(const QString &/*prefix*/)
return static_cast<int>(first->prefixMatch())
< static_cast<int>(second->prefixMatch());
}
if (first->requiresFixIts() != second->requiresFixIts())
return first->requiresFixIts() < second->requiresFixIts();
return (first->order() > 0
&& (first->order() < second->order()
|| (first->order() == second->order() && first->text() < second->text())));
return false;
};
// Keep the order for the items with the same priority and name.

104
src/plugins/clangcodemodel/clangcompletionassistprocessor.cpp
View File

@@ -51,8 +51,9 @@
#include <clangsupport/filecontainer.h>
#include <utils/algorithm.h>
#include <utils/textutils.h>
#include <utils/mimetypes/mimedatabase.h>
#include <utils/optional.h>
#include <utils/textutils.h>
#include <utils/qtcassert.h>
#include <QDirIterator>
@@ -76,20 +77,18 @@ static void addAssistProposalItem(QList<AssistProposalItemInterface *> &items,
item->appendCodeCompletion(codeCompletion);
}
// Add the next CXXMethod or CXXConstructor which is the overload for another existing item.
static void addFunctionOverloadAssistProposalItem(QList<AssistProposalItemInterface *> &items,
AssistProposalItemInterface *sameItem,
const ClangCompletionAssistInterface *interface,
const CodeCompletion &codeCompletion,
const QString &name)
{
ClangBackEnd::CodeCompletionChunk resultType = codeCompletion.chunks.first();
auto *item = static_cast<ClangAssistProposalItem *>(sameItem);
item->setHasOverloadsWithParameters(true);
if (resultType.kind != ClangBackEnd::CodeCompletionChunk::ResultType) {
// It's the constructor.
if (codeCompletion.completionKind == CodeCompletion::ConstructorCompletionKind) {
// It's the constructor, currently constructor definitions do not lead here.
// CLANG-UPGRADE-CHECK: Can we get here with constructor definition?
if (!item->firstCodeCompletion().hasParameters)
item->removeFirstCodeCompletion();
item->appendCodeCompletion(codeCompletion);
return;
}
@@ -98,15 +97,27 @@ static void addFunctionOverloadAssistProposalItem(QList<AssistProposalItemInterf
cursor.setPosition(interface->position());
cursor.movePosition(QTextCursor::StartOfWord);
const ClangBackEnd::CodeCompletionChunk resultType = codeCompletion.chunks.first();
if (::Utils::Text::matchPreviousWord(*interface->textEditorWidget(),
cursor,
resultType.text.toString())) {
// Function definition completion - do not merge completions together.
addAssistProposalItem(items, codeCompletion, name);
} else {
item->appendCodeCompletion(codeCompletion);
}
}
// Check if they are both CXXMethod or CXXConstructor.
static bool isTheSameFunctionOverload(const CodeCompletion &completion,
const QString &name,
ClangAssistProposalItem *lastItem)
{
return completion.hasParameters
&& completion.completionKind == lastItem->firstCodeCompletion().completionKind
&& lastItem->text() == name;
}
static QList<AssistProposalItemInterface *> toAssistProposalItems(
const CodeCompletions &completions,
const ClangCompletionAssistInterface *interface)
@@ -117,8 +128,9 @@ static QList<AssistProposalItemInterface *> toAssistProposalItems(
QList<AssistProposalItemInterface *> items;
items.reserve(completions.size());
for (const CodeCompletion &codeCompletion : completions) {
if (codeCompletion.text.isEmpty()) // TODO: Make isValid()?
continue;
if (codeCompletion.text.isEmpty())
continue; // It's an OverloadCandidate which has text but no typedText.
if (signalCompletion && codeCompletion.completionKind != CodeCompletion::SignalCompletionKind)
continue;
if (slotCompletion && codeCompletion.completionKind != CodeCompletion::SlotCompletionKind)
@@ -128,34 +140,16 @@ static QList<AssistProposalItemInterface *> toAssistProposalItems(
? CompletionChunksToTextConverter::convertToName(codeCompletion.chunks)
: codeCompletion.text.toString();
if (codeCompletion.completionKind == CodeCompletion::ConstructorCompletionKind
|| codeCompletion.completionKind == CodeCompletion::ClassCompletionKind) {
auto samePreviousConstructor
= std::find_if(items.begin(),
items.end(),
[&](const AssistProposalItemInterface *item) {
return item->text() == name
&& static_cast<const ClangAssistProposalItem *>(item)->firstCodeCompletion()
.completionKind == codeCompletion.completionKind;
});
if (samePreviousConstructor == items.end()) {
addAssistProposalItem(items, codeCompletion, name);
} else if (codeCompletion.completionKind == CodeCompletion::ConstructorCompletionKind){
addFunctionOverloadAssistProposalItem(items, *samePreviousConstructor, interface,
codeCompletion, name);
}
continue;
}
if (!items.empty() && items.last()->text() == name) {
if ((codeCompletion.completionKind == CodeCompletion::FunctionCompletionKind
|| codeCompletion.completionKind == CodeCompletion::FunctionDefinitionCompletionKind)
&& codeCompletion.hasParameters) {
if (items.empty()) {
addAssistProposalItem(items, codeCompletion, name);
} else {
auto *lastItem = static_cast<ClangAssistProposalItem *>(items.last());
if (isTheSameFunctionOverload(codeCompletion, name, lastItem)) {
addFunctionOverloadAssistProposalItem(items, items.back(), interface,
codeCompletion, name);
} else {
addAssistProposalItem(items, codeCompletion, name);
}
} else {
addAssistProposalItem(items, codeCompletion, name);
}
}
@@ -187,48 +181,14 @@ IAssistProposal *ClangCompletionAssistProcessor::perform(const AssistInterface *
return startCompletionHelper(); // == 0 if results are calculated asynchronously
}
static CodeCompletions filterFunctionSignatures(const CodeCompletions &completions)
{
return ::Utils::filtered(completions, [](const CodeCompletion &completion) {
return completion.completionKind == CodeCompletion::FunctionOverloadCompletionKind;
});
}
static CodeCompletions filterConstructorSignatures(Utf8String textBefore,
const CodeCompletions &completions)
{
const int prevStatementEnd = textBefore.lastIndexOf(";");
if (prevStatementEnd != -1)
textBefore = textBefore.mid(prevStatementEnd + 1);
return ::Utils::filtered(completions, [&textBefore](const CodeCompletion &completion) {
if (completion.completionKind != CodeCompletion::ConstructorCompletionKind)
return false;
const Utf8String type = completion.chunks.at(0).text;
return textBefore.indexOf(type) != -1;
});
}
void ClangCompletionAssistProcessor::handleAvailableCompletions(
const CodeCompletions &completions)
void ClangCompletionAssistProcessor::handleAvailableCompletions(const CodeCompletions &completions)
{
QTC_CHECK(m_completions.isEmpty());
if (m_sentRequestType == FunctionHintCompletion){
CodeCompletions functionSignatures;
if (m_completionOperator == T_LPAREN) {
functionSignatures = filterFunctionSignatures(completions);
} else {
const QTextBlock block = m_interface->textDocument()->findBlock(
m_interface->position());
const QString textBefore = block.text().left(
m_interface->position() - block.position());
functionSignatures = filterConstructorSignatures(textBefore, completions);
}
if (!functionSignatures.isEmpty()) {
setAsyncProposalAvailable(createFunctionHintProposal(functionSignatures));
if (m_sentRequestType == FunctionHintCompletion) {
const CodeCompletion &firstCompletion = completions.front();
if (firstCompletion.completionKind == CodeCompletion::FunctionOverloadCompletionKind) {
setAsyncProposalAvailable(createFunctionHintProposal(completions));
return;
}
// else: Proceed with a normal completion in case:

51
src/tools/clangbackend/source/codecompleter.cpp
View File

@@ -40,6 +40,8 @@
#include "unsavedfile.h"
#include "unsavedfiles.h"
#include <utils/qtcassert.h>
#include <clang-c/Index.h>
namespace ClangBackEnd {
@@ -47,13 +49,14 @@ namespace ClangBackEnd {
namespace {
CodeCompletions toCodeCompletions(const TranslationUnit &translationUnit,
const ClangCodeCompleteResults &results)
const ClangCodeCompleteResults &results,
bool onlyFunctionOverloads)
{
if (results.isNull())
return CodeCompletions();
CodeCompletionsExtractor extractor(translationUnit.cxTranslationUnit(), results.data());
CodeCompletions codeCompletions = extractor.extractAll();
CodeCompletions codeCompletions = extractor.extractAll(onlyFunctionOverloads);
return codeCompletions;
}
@@ -83,33 +86,49 @@ CodeCompleter::CodeCompleter(const TranslationUnit &translationUnit,
{
}
static void replaceWithOpeningParen(UnsavedFile &file, uint line, uint column)
{
bool ok;
const uint pos = file.toUtf8Position(line, column, &ok);
QTC_ASSERT(ok, return;);
file.replaceAt(pos, 1, Utf8String("(", 1));
}
CodeCompletions CodeCompleter::complete(uint line, uint column,
int funcNameStartLine,
int funcNameStartColumn)
{
if (funcNameStartLine >= 0) {
UnsavedFile &file = unsavedFiles.unsavedFile(translationUnit.filePath());
// Replace '{' by '(' to get proper FunctionOverloadCompletionKind for constructor.
if (file.hasCharacterAt(line, column - 1, '{'))
replaceWithOpeningParen(file, line, column - 1);
}
// Check if we have a smart pointer completion and get proper constructor signatures in results.
// Results are empty when it's not a smart pointer or this completion failed.
ClangCodeCompleteResults results = completeSmartPointerCreation(line,
column,
funcNameStartLine,
funcNameStartColumn);
ClangCodeCompleteResults clangCompletions = completeSmartPointerCreation(line,
column,
funcNameStartLine,
funcNameStartColumn);
if (results.isNull() || results.isEmpty())
results = completeHelper(line, column);
// Default completion.
if (clangCompletions.isNull() || clangCompletions.isEmpty())
clangCompletions = completeHelper(line, column);
filterUnknownContextResults(results, unsavedFile(), line, column);
filterUnknownContextResults(clangCompletions, unsavedFile(), line, column);
return toCodeCompletions(translationUnit, results);
return toCodeCompletions(translationUnit, clangCompletions, funcNameStartLine >= 0);
}
// For given "make_unique<T>" / "make_shared<T>" / "QSharedPointer<T>::create" return "new T("
// Otherwize return empty QString
static QString tweakName(const QString &oldName)
static QString tweakName(const Utf8String &oldName)
{
QString fullName = oldName.trimmed();
if (!fullName.contains('>'))
if (!oldName.contains('>'))
return QString();
QString fullName = QString(oldName).trimmed();
if (!fullName.endsWith('>')) {
// This is the class<type>::method case - remove ::method part
if (!fullName.endsWith("create") || !fullName.contains("QSharedPointer"))
@@ -138,10 +157,12 @@ ClangCodeCompleteResults CodeCompleter::completeSmartPointerCreation(uint line,
bool ok;
const uint startPos = file.toUtf8Position(funcNameStartLine, funcNameStartColumn, &ok);
QTC_ASSERT(ok, return ClangCodeCompleteResults(););
const uint endPos = file.toUtf8Position(line, column - 1, &ok);
QTC_ASSERT(ok, return ClangCodeCompleteResults(););
Utf8String content = file.fileContent();
const QString oldName = content.mid(startPos, endPos - startPos);
const Utf8String content = file.fileContent();
const Utf8String oldName = content.mid(startPos, endPos - startPos);
const QString updatedName = tweakName(oldName);
if (updatedName.isEmpty())
return ClangCodeCompleteResults();

72
src/tools/clangbackend/source/codecompletionsextractor.cpp
View File

@@ -27,9 +27,13 @@
#include "clangbackend_global.h"
#include "clangstring.h"
#include "clangtranslationunit.h"
#include "codecompletionchunkconverter.h"
#include "sourcelocation.h"
#include "sourcerange.h"
#include <utils/algorithm.h>
#include <QDebug>
namespace ClangBackEnd {
@@ -85,7 +89,7 @@ bool CodeCompletionsExtractor::peek(const Utf8String &name)
return false;
}
CodeCompletions CodeCompletionsExtractor::extractAll()
CodeCompletions CodeCompletionsExtractor::extractAll(bool onlyFunctionOverloads)
{
CodeCompletions codeCompletions;
codeCompletions.reserve(int(cxCodeCompleteResults->NumResults));
@@ -93,9 +97,75 @@ CodeCompletions CodeCompletionsExtractor::extractAll()
while (next())
codeCompletions.append(currentCodeCompletion_);
handleCompletions(codeCompletions, onlyFunctionOverloads);
return codeCompletions;
}
static CodeCompletions filterFunctionOverloads(const CodeCompletions &completions)
{
return ::Utils::filtered(completions, [](const CodeCompletion &completion) {
return completion.completionKind == CodeCompletion::FunctionOverloadCompletionKind;
});
}
static ::Utils::optional<bool> classBeforeCXXConstructor(const CodeCompletion &first,
const CodeCompletion &second)
{
// Put ClassCompletionKind elements before ConstructorCompletionKind elements
// when they have the same name.
if (first.completionKind == CodeCompletion::ClassCompletionKind
&& second.completionKind == CodeCompletion::ConstructorCompletionKind
&& first.text == second.text) {
return true;
}
if (first.completionKind == CodeCompletion::ConstructorCompletionKind
&& second.completionKind == CodeCompletion::ClassCompletionKind
&& first.text == second.text) {
return false;
}
return ::Utils::optional<bool>();
}
static void sortCodeCompletions(CodeCompletions &codeCompletions)
{
auto currentItemsCompare = [](const CodeCompletion &first,
const CodeCompletion &second) {
// Items without fix-its come first.
if (first.requiredFixIts.empty() != second.requiredFixIts.empty())
return first.requiredFixIts.empty() > second.requiredFixIts.empty();
const ::Utils::optional<bool> classBeforeConstructorWithTheSameName
= classBeforeCXXConstructor(first, second);
if (classBeforeConstructorWithTheSameName)
return classBeforeConstructorWithTheSameName.value();
return (first.priority > 0
&& (first.priority < second.priority
|| (first.priority == second.priority && first.text < second.text)));
};
// Keep the order for the items with the same priority and name.
std::stable_sort(codeCompletions.begin(), codeCompletions.end(), currentItemsCompare);
}
void CodeCompletionsExtractor::handleCompletions(CodeCompletions &codeCompletions,
bool onlyFunctionOverloads)
{
if (onlyFunctionOverloads) {
const CodeCompletions overloadCompletions = filterFunctionOverloads(codeCompletions);
// If filtered completions are empty the assumption we need function overloads is wrong
// therefore we do not use filtered results in that case.
if (!overloadCompletions.isEmpty())
codeCompletions = overloadCompletions;
}
sortCodeCompletions(codeCompletions);
}
void CodeCompletionsExtractor::extractCompletionKind()
{
switch (currentCxCodeCompleteResult.CursorKind) {

4
src/tools/clangbackend/source/codecompletionsextractor.h
View File

@@ -50,7 +50,7 @@ public:
bool next();
bool peek(const Utf8String &name);
CodeCompletions extractAll();
CodeCompletions extractAll(bool onlyFunctionOverloads);
const CodeCompletion &currentCodeCompletion() const;
@@ -73,6 +73,8 @@ private:
void decreasePriorityForQObjectInternals();
void decreasePriorityForOperators();
void handleCompletions(CodeCompletions &codeCompletions, bool onlyFunctionOverloads);
bool hasText(const Utf8String &text, CXCompletionString cxCompletionString) const;
private:

99
tests/unit/unittest/codecompleter-test.cpp
View File

@@ -43,6 +43,7 @@ using ::testing::Not;
using ::testing::PrintToString;
using ClangBackEnd::CodeCompletion;
using ClangBackEnd::CodeCompletionChunk;
using ClangBackEnd::CodeCompleter;
namespace {
@@ -65,6 +66,29 @@ MATCHER_P2(IsCodeCompletion, text, completionKind,
return true;
}
MATCHER_P(IsOverloadCompletion, text,
std::string(negation ? "isn't" : "is") + " overload completion with text " + PrintToString(text))
{
Utf8String overloadName;
for (auto &chunk : arg.chunks) {
if (chunk.kind == CodeCompletionChunk::Text) {
overloadName = chunk.text;
break;
}
}
if (overloadName != text) {
*result_listener << "text is " + PrintToString(overloadName) + " and not " + PrintToString(text);
return false;
}
if (arg.completionKind != CodeCompletion::FunctionOverloadCompletionKind) {
*result_listener << "kind is " + PrintToString(arg.completionKind) + " and not " + PrintToString(CodeCompletion::FunctionOverloadCompletionKind);
return false;
}
return true;
}
MATCHER(HasFixIts, "")
{
return !arg.requiredFixIts.empty();
@@ -195,6 +219,12 @@ protected:
readFileContent("/complete_smartpointer.cpp"),
true
};
ClangBackEnd::FileContainer completionsOrder{
Utf8StringLiteral(TESTDATA_DIR"/completions_order.cpp"),
{includePathArgument},
readFileContent("/completions_order.cpp"),
true
};
};
using CodeCompleterSlowTest = CodeCompleter;
@@ -306,27 +336,24 @@ TEST_F(CodeCompleterSlowTest, UniquePointerCompletion)
{
auto myCompleter = setupCompleter(smartPointerCompletion);
ASSERT_THAT(myCompleter.complete(55, 54, 55, 32),
Contains(IsCodeCompletion(Utf8StringLiteral("Bar"),
CodeCompletion::ConstructorCompletionKind)));
ASSERT_THAT(myCompleter.complete(59, 54, 59, 32),
Contains(IsOverloadCompletion(Utf8StringLiteral("Bar"))));
}
TEST_F(CodeCompleterSlowTest, SharedPointerCompletion)
{
auto myCompleter = setupCompleter(smartPointerCompletion);
ASSERT_THAT(myCompleter.complete(56, 55, 56, 33),
Contains(IsCodeCompletion(Utf8StringLiteral("Bar"),
CodeCompletion::ConstructorCompletionKind)));
ASSERT_THAT(myCompleter.complete(60, 55, 60, 33),
Contains(IsOverloadCompletion(Utf8StringLiteral("Bar"))));
}
TEST_F(CodeCompleterSlowTest, QSharedPointerCompletion)
{
auto myCompleter = setupCompleter(smartPointerCompletion);
ASSERT_THAT(myCompleter.complete(57, 60, 57, 32),
Contains(IsCodeCompletion(Utf8StringLiteral("Bar"),
CodeCompletion::ConstructorCompletionKind)));
ASSERT_THAT(myCompleter.complete(61, 60, 61, 32),
Contains(IsOverloadCompletion(Utf8StringLiteral("Bar"))));
}
TEST_F(CodeCompleterSlowTest, FunctionInUnsavedIncludedHeader)
@@ -499,6 +526,60 @@ TEST_F(CodeCompleterSlowTest, GlobalCompletionAfterForwardDeclaredClassPointer)
ASSERT_TRUE(!completions.isEmpty());
}
TEST_F(CodeCompleterSlowTest, ConstructorCompletionExists)
{
auto myCompleter = setupCompleter(completionsOrder);
const ClangBackEnd::CodeCompletions completions = myCompleter.complete(8, 1);
int constructorIndex = Utils::indexOf(completions, [](const CodeCompletion &codeCompletion) {
return codeCompletion.text == "Constructor" && codeCompletion.completionKind == CodeCompletion::ConstructorCompletionKind;
});
ASSERT_THAT(constructorIndex != -1, true);
}
TEST_F(CodeCompleterSlowTest, ClassConstructorCompletionsOrder)
{
auto myCompleter = setupCompleter(completionsOrder);
const ClangBackEnd::CodeCompletions completions = myCompleter.complete(8, 1);
int classIndex = Utils::indexOf(completions, [](const CodeCompletion &codeCompletion) {
return codeCompletion.text == "Constructor" && codeCompletion.completionKind == CodeCompletion::ClassCompletionKind;
});
int constructorIndex = Utils::indexOf(completions, [](const CodeCompletion &codeCompletion) {
return codeCompletion.text == "Constructor" && codeCompletion.completionKind == CodeCompletion::ConstructorCompletionKind;
});
ASSERT_THAT(classIndex < constructorIndex, true);
}
TEST_F(CodeCompleterSlowTest, SharedPointerCompletionsOrder)
{
auto myCompleter = setupCompleter(smartPointerCompletion);
const ClangBackEnd::CodeCompletions completions = myCompleter.complete(62, 11);
int resetIndex = Utils::indexOf(completions, [](const CodeCompletion &codeCompletion) {
return codeCompletion.text == "reset";
});
int barDestructorIndex = Utils::indexOf(completions, [](const CodeCompletion &codeCompletion) {
return codeCompletion.text == "~Bar";
});
ASSERT_THAT(barDestructorIndex < resetIndex, true);
}
TEST_F(CodeCompleterSlowTest, ConstructorHasOverloadCompletions)
{
auto myCompleter = setupCompleter(completionsOrder);
const ClangBackEnd::CodeCompletions completions = myCompleter.complete(8, 1);
int constructorsCount = Utils::count(completions, [](const CodeCompletion &codeCompletion) {
return codeCompletion.text == "Constructor" && codeCompletion.completionKind == CodeCompletion::ConstructorCompletionKind;
});
ASSERT_THAT(constructorsCount, 2);
}
ClangBackEnd::CodeCompleter CodeCompleter::setupCompleter(
const ClangBackEnd::FileContainer &fileContainer)
{

12
tests/unit/unittest/codecompletionsextractor-test.cpp
View File

@@ -794,6 +794,18 @@ TEST_F(CodeCompletionsExtractorSlowTest, OverloadCandidate)
})));
}
TEST_F(CodeCompletionsExtractorSlowTest, ExtractAll)
{
ClangCodeCompleteResults completeResults(getResults(constructorDocument, 25));
::CodeCompletionsExtractor extractor(
constructorDocument.translationUnit().cxTranslationUnit(),
completeResults.data());
auto codeCompletions = extractor.extractAll(false);
ASSERT_THAT(codeCompletions.empty(), false);
}
ClangCodeCompleteResults CodeCompletionsExtractor::getResults(const Document &document,
uint line,
uint column,

7
tests/unit/unittest/data/complete_smartpointer.cpp
View File

@@ -27,7 +27,11 @@ template<class Type, class... Args>
class unique_ptr {};
template<class Type, class... Args>
class shared_ptr {};
class shared_ptr {
public:
void reset();
Type *operator->();
};
template<class Type, class... Args>
unique_ptr<Type> make_unique(Args&&... args);
@@ -55,4 +59,5 @@ void f2()
std::unique_ptr<Bar> bar = std::make_unique<Bar>();
std::shared_ptr<Bar> bar2 = std::make_shared<Bar>();
QSharedPointer<Bar> bar3 = QSharedPointer<Bar>::create();
bar2->
}

9
tests/unit/unittest/data/completions_order.cpp Normal file
View File

@@ -0,0 +1,9 @@
class Constructor {
public:
Constructor() = default;
Constructor(int) {}
};
void testConstructor() {
}