From 8d6c10b2416c38e93e0ff53f677c31ba711ad0ae Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Fri, 11 Dec 2015 11:45:05 +0100 Subject: [PATCH] Clang: Flatten code completion chunks Avoid the sub vector for performance reason and use an flag for every optional argument because there can be no recursion. Change-Id: Iae1eaa1f164e4129e30358a1719582e5231f0385 Reviewed-by: Nikolai Kosjar --- .../clangbackendipc/codecompletionchunk.cpp | 51 +++++---------- .../clangbackendipc/codecompletionchunk.h | 16 ++--- .../clangcompletionchunkstotextconverter.cpp | 64 +++++++++++++------ .../clangcompletionchunkstotextconverter.h | 6 +- .../codecompletionchunkconverter.cpp | 20 ++---- .../ipcsource/codecompletionchunkconverter.h | 1 - .../unittest/codecompletionsextractortest.cpp | 9 ++- .../completionchunkstotextconvertertest.cpp | 31 ++++++--- 8 files changed, 101 insertions(+), 97 deletions(-) diff --git a/src/libs/clangbackendipc/codecompletionchunk.cpp b/src/libs/clangbackendipc/codecompletionchunk.cpp index 55869534906..474b58690e3 100644 --- a/src/libs/clangbackendipc/codecompletionchunk.cpp +++ b/src/libs/clangbackendipc/codecompletionchunk.cpp @@ -37,17 +37,12 @@ namespace ClangBackEnd { -CodeCompletionChunk::CodeCompletionChunk() - : kind_(Invalid) -{ -} - CodeCompletionChunk::CodeCompletionChunk(CodeCompletionChunk::Kind kind, const Utf8String &text, - const CodeCompletionChunks &optionalChunks) + bool isOptional) : text_(text), - optionalChunks_(optionalChunks), - kind_(kind) + kind_(kind), + isOptional_(isOptional) { } @@ -61,23 +56,21 @@ const Utf8String &CodeCompletionChunk::text() const return text_; } -const CodeCompletionChunks &CodeCompletionChunk::optionalChunks() const +bool CodeCompletionChunk::isOptional() const { - return optionalChunks_; + return isOptional_; } -quint32 &CodeCompletionChunk::kindAsInt() +quint8 &CodeCompletionChunk::kindAsInt() { - return reinterpret_cast(kind_); + return reinterpret_cast(kind_); } QDataStream &operator<<(QDataStream &out, const CodeCompletionChunk &chunk) { - out << chunk.kind_; + out << quint8(chunk.kind_); out << chunk.text_; - - if (chunk.kind() == CodeCompletionChunk::Optional) - out << chunk.optionalChunks_; + out << chunk.isOptional_; return out; } @@ -86,9 +79,7 @@ QDataStream &operator>>(QDataStream &in, CodeCompletionChunk &chunk) { in >> chunk.kindAsInt(); in >> chunk.text_; - - if (chunk.kind_ == CodeCompletionChunk::Optional) - in >> chunk.optionalChunks_; + in >> chunk.isOptional_; return in; } @@ -97,7 +88,7 @@ bool operator==(const CodeCompletionChunk &first, const CodeCompletionChunk &sec { return first.kind() == second.kind() && first.text() == second.text() - && first.optionalChunks() == second.optionalChunks(); + && first.isOptional() == second.isOptional(); } static const char *completionChunkKindToString(CodeCompletionChunk::Kind kind) @@ -136,8 +127,8 @@ QDebug operator<<(QDebug debug, const CodeCompletionChunk &chunk) debug.nospace() << completionChunkKindToString(chunk.kind()) << ", "; debug.nospace() << chunk.text(); - if (chunk.kind() == CodeCompletionChunk::Optional) - debug.nospace() << ", " << chunk.optionalChunks(); + if (chunk.isOptional()) + debug.nospace() << ", optional"; debug.nospace() << ")"; @@ -150,20 +141,8 @@ void PrintTo(const CodeCompletionChunk &chunk, ::std::ostream* os) *os << completionChunkKindToString(chunk.kind()) << ", "; *os << chunk.text().constData(); - if (chunk.kind() == CodeCompletionChunk::Optional) { - const auto optionalChunks = chunk.optionalChunks(); - *os << ", {"; - - for (auto optionalChunkPosition = optionalChunks.cbegin(); - optionalChunkPosition != optionalChunks.cend(); - ++optionalChunkPosition) { - PrintTo(*optionalChunkPosition, os); - if (std::next(optionalChunkPosition) != optionalChunks.cend()) - *os << ", "; - } - - *os << "}"; - } + if (chunk.isOptional()) + *os << ", optional"; *os << "}"; } diff --git a/src/libs/clangbackendipc/codecompletionchunk.h b/src/libs/clangbackendipc/codecompletionchunk.h index 72df089f289..d4f016fec4c 100644 --- a/src/libs/clangbackendipc/codecompletionchunk.h +++ b/src/libs/clangbackendipc/codecompletionchunk.h @@ -49,7 +49,7 @@ class CMBIPC_EXPORT CodeCompletionChunk friend CMBIPC_EXPORT bool operator==(const CodeCompletionChunk &first, const CodeCompletionChunk &second); public: - enum Kind : quint32 { + enum Kind : quint8 { Optional, TypedText, Text, @@ -71,25 +71,25 @@ public: Equal, HorizontalSpace, VerticalSpace, - Invalid = 255}; + Invalid = 255 + }; - CodeCompletionChunk(); + CodeCompletionChunk() = default; CodeCompletionChunk(Kind kind, const Utf8String &text, - const CodeCompletionChunks &optionalChunks = CodeCompletionChunks()); + bool isOptional = false); Kind kind() const; const Utf8String &text() const; - const CodeCompletionChunks &optionalChunks() const; + bool isOptional() const; private: - quint32 &kindAsInt(); + quint8 &kindAsInt(); private: Utf8String text_; - CodeCompletionChunks optionalChunks_; Kind kind_ = Invalid; - + bool isOptional_ = false; }; CMBIPC_EXPORT QDataStream &operator<<(QDataStream &out, const CodeCompletionChunk &chunk); diff --git a/src/plugins/clangcodemodel/clangcompletionchunkstotextconverter.cpp b/src/plugins/clangcodemodel/clangcompletionchunkstotextconverter.cpp index 43e6c86f76e..46a9c946905 100644 --- a/src/plugins/clangcodemodel/clangcompletionchunkstotextconverter.cpp +++ b/src/plugins/clangcodemodel/clangcompletionchunkstotextconverter.cpp @@ -36,7 +36,8 @@ namespace ClangCodeModel { namespace Internal { -void CompletionChunksToTextConverter::parseChunks(const ClangBackEnd::CodeCompletionChunks &codeCompletionChunks) +void CompletionChunksToTextConverter::parseChunks( + const ClangBackEnd::CodeCompletionChunks &codeCompletionChunks) { m_text.clear(); m_placeholderPositions.clear(); @@ -49,7 +50,7 @@ void CompletionChunksToTextConverter::parseChunks(const ClangBackEnd::CodeComple m_codeCompletionChunks.cend(), [this] (const ClangBackEnd::CodeCompletionChunk &chunk) { - parse(chunk); + parseDependendOnTheOptionalState(chunk); m_previousCodeCompletionChunk = chunk; }); } @@ -74,7 +75,8 @@ void CompletionChunksToTextConverter::setAddSpaces(bool addSpaces) m_addSpaces = addSpaces; } -void CompletionChunksToTextConverter::setAddExtraVerticalSpaceBetweenBraces(bool addExtraVerticalSpaceBetweenBraces) +void CompletionChunksToTextConverter::setAddExtraVerticalSpaceBetweenBraces( + bool addExtraVerticalSpaceBetweenBraces) { m_addExtraVerticalSpaceBetweenBraces = addExtraVerticalSpaceBetweenBraces; } @@ -104,7 +106,8 @@ bool CompletionChunksToTextConverter::hasPlaceholderPositions() const return m_placeholderPositions.size() > 0; } -QString CompletionChunksToTextConverter::convertToFunctionSignature(const ClangBackEnd::CodeCompletionChunks &codeCompletionChunks) +QString CompletionChunksToTextConverter::convertToFunctionSignature( + const ClangBackEnd::CodeCompletionChunks &codeCompletionChunks) { CompletionChunksToTextConverter converter; converter.setAddPlaceHolderText(true); @@ -116,7 +119,8 @@ QString CompletionChunksToTextConverter::convertToFunctionSignature(const ClangB return converter.text(); } -QString CompletionChunksToTextConverter::convertToName(const ClangBackEnd::CodeCompletionChunks &codeCompletionChunks) +QString CompletionChunksToTextConverter::convertToName( + const ClangBackEnd::CodeCompletionChunks &codeCompletionChunks) { CompletionChunksToTextConverter converter; @@ -125,7 +129,8 @@ QString CompletionChunksToTextConverter::convertToName(const ClangBackEnd::CodeC return converter.text(); } -QString CompletionChunksToTextConverter::convertToToolTip(const ClangBackEnd::CodeCompletionChunks &codeCompletionChunks) +QString CompletionChunksToTextConverter::convertToToolTip( + const ClangBackEnd::CodeCompletionChunks &codeCompletionChunks) { CompletionChunksToTextConverter converter; converter.setAddPlaceHolderText(true); @@ -140,13 +145,13 @@ QString CompletionChunksToTextConverter::convertToToolTip(const ClangBackEnd::Co return converter.text(); } -void CompletionChunksToTextConverter::parse(const ClangBackEnd::CodeCompletionChunk &codeCompletionChunk) +void CompletionChunksToTextConverter::parse( + const ClangBackEnd::CodeCompletionChunk &codeCompletionChunk) { using ClangBackEnd::CodeCompletionChunk; switch (codeCompletionChunk.kind()) { case CodeCompletionChunk::ResultType: parseResultType(codeCompletionChunk.text()); break; - case CodeCompletionChunk::Optional: parseOptional(codeCompletionChunk); break; case CodeCompletionChunk::Placeholder: parsePlaceHolder(codeCompletionChunk); break; case CodeCompletionChunk::LeftParen: parseLeftParen(codeCompletionChunk); break; case CodeCompletionChunk::LeftBrace: parseLeftBrace(codeCompletionChunk); break; @@ -154,6 +159,15 @@ void CompletionChunksToTextConverter::parse(const ClangBackEnd::CodeCompletionCh } } +void CompletionChunksToTextConverter::parseDependendOnTheOptionalState( + const ClangBackEnd::CodeCompletionChunk &codeCompletionChunk) +{ + wrapInCursiveTagIfOptional(codeCompletionChunk); + + if (isNotOptionalOrAddOptionals(codeCompletionChunk)) + parse(codeCompletionChunk); +} + void CompletionChunksToTextConverter::parseResultType(const Utf8String &resultTypeText) { if (m_addResultType) @@ -170,20 +184,21 @@ void CompletionChunksToTextConverter::parseText(const Utf8String &text) m_text += text.toString(); } -void CompletionChunksToTextConverter::parseOptional(const ClangBackEnd::CodeCompletionChunk &optionalCodeCompletionChunk) +void CompletionChunksToTextConverter::wrapInCursiveTagIfOptional( + const ClangBackEnd::CodeCompletionChunk &codeCompletionChunk) { if (m_addOptional) { - if (m_addHtmlTags) - m_text += QStringLiteral(""); - - m_text += convertToFunctionSignature(optionalCodeCompletionChunk.optionalChunks()); - - if (m_addHtmlTags) - m_text += QStringLiteral(""); + if (m_addHtmlTags) { + if (!m_previousCodeCompletionChunk.isOptional() && codeCompletionChunk.isOptional()) + m_text += QStringLiteral(""); + else if (m_previousCodeCompletionChunk.isOptional() && !codeCompletionChunk.isOptional()) + m_text += QStringLiteral(""); + } } } -void CompletionChunksToTextConverter::parsePlaceHolder(const ClangBackEnd::CodeCompletionChunk &codeCompletionChunk) +void CompletionChunksToTextConverter::parsePlaceHolder( + const ClangBackEnd::CodeCompletionChunk &codeCompletionChunk) { if (m_addPlaceHolderText) m_text += codeCompletionChunk.text().toString(); @@ -192,7 +207,8 @@ void CompletionChunksToTextConverter::parsePlaceHolder(const ClangBackEnd::CodeC m_placeholderPositions.push_back(m_text.size()); } -void CompletionChunksToTextConverter::parseLeftParen(const ClangBackEnd::CodeCompletionChunk &codeCompletionChunk) +void CompletionChunksToTextConverter::parseLeftParen( + const ClangBackEnd::CodeCompletionChunk &codeCompletionChunk) { if (canAddSpace()) m_text += QChar(QChar::Space); @@ -200,7 +216,8 @@ void CompletionChunksToTextConverter::parseLeftParen(const ClangBackEnd::CodeCom m_text += codeCompletionChunk.text().toString(); } -void CompletionChunksToTextConverter::parseLeftBrace(const ClangBackEnd::CodeCompletionChunk &codeCompletionChunk) +void CompletionChunksToTextConverter::parseLeftBrace( + const ClangBackEnd::CodeCompletionChunk &codeCompletionChunk) { if (canAddSpace()) m_text += QChar(QChar::Space); @@ -214,7 +231,8 @@ void CompletionChunksToTextConverter::addExtraVerticalSpaceBetweenBraces() addExtraVerticalSpaceBetweenBraces(m_codeCompletionChunks.begin()); } -void CompletionChunksToTextConverter::addExtraVerticalSpaceBetweenBraces(const ClangBackEnd::CodeCompletionChunks::iterator &begin) +void CompletionChunksToTextConverter::addExtraVerticalSpaceBetweenBraces( + const ClangBackEnd::CodeCompletionChunks::iterator &begin) { using ClangBackEnd::CodeCompletionChunk; @@ -263,6 +281,12 @@ bool CompletionChunksToTextConverter::canAddSpace() const && m_previousCodeCompletionChunk.kind() != ClangBackEnd::CodeCompletionChunk::RightAngle; } +bool CompletionChunksToTextConverter::isNotOptionalOrAddOptionals( + const ClangBackEnd::CodeCompletionChunk &codeCompletionChunk) const +{ + return !codeCompletionChunk.isOptional() || m_addOptional; +} + } // namespace Internal } // namespace ClangCodeModel diff --git a/src/plugins/clangcodemodel/clangcompletionchunkstotextconverter.h b/src/plugins/clangcodemodel/clangcompletionchunkstotextconverter.h index f80ab1a383e..c08aa0f5b32 100644 --- a/src/plugins/clangcodemodel/clangcompletionchunkstotextconverter.h +++ b/src/plugins/clangcodemodel/clangcompletionchunkstotextconverter.h @@ -63,10 +63,11 @@ public: static QString convertToName(const ClangBackEnd::CodeCompletionChunks &codeCompletionChunks); static QString convertToToolTip(const ClangBackEnd::CodeCompletionChunks &codeCompletionChunks); private: - void parse(const ClangBackEnd::CodeCompletionChunk & codeCompletionChunk); + void parse(const ClangBackEnd::CodeCompletionChunk &codeCompletionChunk); + void parseDependendOnTheOptionalState(const ClangBackEnd::CodeCompletionChunk &codeCompletionChunk); void parseResultType(const Utf8String &text); void parseText(const Utf8String &text); - void parseOptional(const ClangBackEnd::CodeCompletionChunk &optionalCodeCompletionChunk); + void wrapInCursiveTagIfOptional(const ClangBackEnd::CodeCompletionChunk &codeCompletionChunk); void parsePlaceHolder(const ClangBackEnd::CodeCompletionChunk &codeCompletionChunk); void parseLeftParen(const ClangBackEnd::CodeCompletionChunk &codeCompletionChunk); void parseLeftBrace(const ClangBackEnd::CodeCompletionChunk &codeCompletionChunk); @@ -74,6 +75,7 @@ private: void addExtraVerticalSpaceBetweenBraces(const ClangBackEnd::CodeCompletionChunks::iterator &); bool canAddSpace() const; + bool isNotOptionalOrAddOptionals(const ClangBackEnd::CodeCompletionChunk &codeCompletionChunk) const; private: std::vector m_placeholderPositions; diff --git a/src/tools/clangbackend/ipcsource/codecompletionchunkconverter.cpp b/src/tools/clangbackend/ipcsource/codecompletionchunkconverter.cpp index 03adf6e13c2..7a9d9b659ac 100644 --- a/src/tools/clangbackend/ipcsource/codecompletionchunkconverter.cpp +++ b/src/tools/clangbackend/ipcsource/codecompletionchunkconverter.cpp @@ -42,13 +42,12 @@ void CodeCompletionChunkConverter::extractCompletionChunks(CXCompletionString co for (uint chunkIndex = 0; chunkIndex < completionChunkCount; ++chunkIndex) { const CodeCompletionChunk::Kind kind = chunkKind(completionString, chunkIndex); - if (kind == CodeCompletionChunk::Optional) - chunks.append(CodeCompletionChunk(kind, - chunkText(completionString, chunkIndex), - optionalChunks(completionString, chunkIndex))); - else + if (kind == CodeCompletionChunk::Optional) { + extractOptionalCompletionChunks(clang_getCompletionChunkCompletionString(completionString, chunkIndex)); + } else { chunks.append(CodeCompletionChunk(kind, chunkText(completionString, chunkIndex))); + } } } @@ -62,7 +61,7 @@ void CodeCompletionChunkConverter::extractOptionalCompletionChunks(CXCompletionS if (kind == CodeCompletionChunk::Optional) extractOptionalCompletionChunks(clang_getCompletionChunkCompletionString(completionString, chunkIndex)); else - chunks.append(CodeCompletionChunk(kind, chunkText(completionString, chunkIndex))); + chunks.append(CodeCompletionChunk(kind, chunkText(completionString, chunkIndex), true)); } } @@ -85,14 +84,5 @@ Utf8String CodeCompletionChunkConverter::chunkText(CXCompletionString completion return ClangString(clang_getCompletionChunkText(completionString, chunkIndex)); } -CodeCompletionChunks CodeCompletionChunkConverter::optionalChunks(CXCompletionString completionString, uint chunkIndex) -{ - CodeCompletionChunkConverter converter; - - converter.extractOptionalCompletionChunks(clang_getCompletionChunkCompletionString(completionString, chunkIndex)); - - return converter.chunks; -} - } // namespace ClangBackEnd diff --git a/src/tools/clangbackend/ipcsource/codecompletionchunkconverter.h b/src/tools/clangbackend/ipcsource/codecompletionchunkconverter.h index 6907690e81e..2aed074eb46 100644 --- a/src/tools/clangbackend/ipcsource/codecompletionchunkconverter.h +++ b/src/tools/clangbackend/ipcsource/codecompletionchunkconverter.h @@ -47,7 +47,6 @@ public: static Utf8String chunkText(CXCompletionString completionString, uint chunkIndex); private: - CodeCompletionChunks optionalChunks(CXCompletionString completionString, uint chunkIndex); static CodeCompletionChunk::Kind chunkKind(CXCompletionString completionString, uint chunkIndex); void extractCompletionChunks(CXCompletionString completionString); void extractOptionalCompletionChunks(CXCompletionString completionString); diff --git a/tests/unit/unittest/codecompletionsextractortest.cpp b/tests/unit/unittest/codecompletionsextractortest.cpp index d9f070a3baf..f4777343bd7 100644 --- a/tests/unit/unittest/codecompletionsextractortest.cpp +++ b/tests/unit/unittest/codecompletionsextractortest.cpp @@ -630,11 +630,10 @@ TEST_F(CodeCompletionsExtractor, CompletionChunksFunctionWithOptionalChunks) {CodeCompletionChunk::TypedText, Utf8StringLiteral("FunctionWithOptional")}, {CodeCompletionChunk::LeftParen, Utf8StringLiteral("(")}, {CodeCompletionChunk::Placeholder, Utf8StringLiteral("int x")}, - {CodeCompletionChunk::Optional, Utf8String(), CodeCompletionChunks({ - {CodeCompletionChunk::Comma, Utf8StringLiteral(", ")}, - {CodeCompletionChunk::Placeholder, Utf8StringLiteral("char y")}, - {CodeCompletionChunk::Comma, Utf8StringLiteral(", ")}, - {CodeCompletionChunk::Placeholder, Utf8StringLiteral("int z")}})}, + {CodeCompletionChunk::Comma, Utf8StringLiteral(", "), true}, + {CodeCompletionChunk::Placeholder, Utf8StringLiteral("char y"), true}, + {CodeCompletionChunk::Comma, Utf8StringLiteral(", "), true}, + {CodeCompletionChunk::Placeholder, Utf8StringLiteral("int z"), true}, {CodeCompletionChunk::RightParen, Utf8StringLiteral(")")}}))); } diff --git a/tests/unit/unittest/completionchunkstotextconvertertest.cpp b/tests/unit/unittest/completionchunkstotextconvertertest.cpp index 032124dfc52..b9c1dd9a5a8 100644 --- a/tests/unit/unittest/completionchunkstotextconvertertest.cpp +++ b/tests/unit/unittest/completionchunkstotextconvertertest.cpp @@ -80,10 +80,12 @@ protected: CodeCompletionChunk elseName{CodeCompletionChunk::TypedText, Utf8StringLiteral("else")}; CodeCompletionChunk ifName{CodeCompletionChunk::TypedText, Utf8StringLiteral("if")}; CodeCompletionChunk horizontalSpace{CodeCompletionChunk::HorizontalSpace, Utf8StringLiteral(" ")}; - CodeCompletionChunk optional{CodeCompletionChunk::Optional, Utf8String(), {comma, functionArgumentY, comma, functionArgumentZ}}; CodeCompletionChunk enableIfT{CodeCompletionChunk::TypedText, Utf8StringLiteral("enable_if_t")}; CodeCompletionChunk enableIfTCondition{CodeCompletionChunk::Placeholder, Utf8StringLiteral("_Cond")}; - CodeCompletionChunk enableIfTType{CodeCompletionChunk::Placeholder, Utf8StringLiteral("_Tp")}; + CodeCompletionChunk optionalEnableIfTType{CodeCompletionChunk::Placeholder, Utf8StringLiteral("_Tp"), true}; + CodeCompletionChunk optionalComma{CodeCompletionChunk::Comma, Utf8StringLiteral(", "), true}; + CodeCompletionChunk optionalFunctionArgumentY{CodeCompletionChunk::Placeholder, Utf8StringLiteral("int y"), true}; + CodeCompletionChunk optionalFunctionArgumentZ{CodeCompletionChunk::Placeholder, Utf8StringLiteral("int z"), true}; }; TEST_F(CompletionChunksToTextConverter, ParseIsClearingText) @@ -119,7 +121,15 @@ TEST_F(CompletionChunksToTextConverter, ConvertFunctionWithParameters) TEST_F(CompletionChunksToTextConverter, ConvertFunctionWithOptionalParameter) { - CodeCompletionChunks completionChunks({integerResultType, functionName, leftParen, functionArgumentX, optional,rightParen}); + CodeCompletionChunks completionChunks({integerResultType, + functionName, + leftParen, + functionArgumentX, + optionalComma, + optionalFunctionArgumentY, + optionalComma, + optionalFunctionArgumentZ, + rightParen}); ASSERT_THAT(Converter::convertToToolTip(completionChunks), QStringLiteral("int Function (char x, int y, int z)")); @@ -157,12 +167,12 @@ TEST_F(CompletionChunksToTextConverter, Enumeration) TEST_F(CompletionChunksToTextConverter, Switch) { CodeCompletionChunks completionChunks({switchName, - leftParen, - condition, - rightParen, - leftBrace, - verticalSpace, - rightBrace}); + leftParen, + condition, + rightParen, + leftBrace, + verticalSpace, + rightBrace}); setupConverterForKeywords(); converter.parseChunks(completionChunks); @@ -239,7 +249,8 @@ TEST_F(CompletionChunksToTextConverter, EnableIfT) CodeCompletionChunks completionChunks({enableIfT, leftAngle, enableIfTCondition, - CodeCompletionChunk(CodeCompletionChunk::Optional, Utf8String(), {comma, enableIfTType}), + optionalComma, + optionalEnableIfTType, rightAngle}); setupConverterForKeywords();