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 <nikolai.kosjar@theqtcompany.com>

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<quint32&>(kind_);
+    return reinterpret_cast<quint8&>(kind_);
 }
 
 QDataStream &operator<<(QDataStream &out, const CodeCompletionChunk &chunk)
 {
-    out << chunk.kind_;
+    out << quint8(chunk.kind_);
     out << chunk.text_;
-
+    out << chunk.isOptional_;
-    if (chunk.kind() == CodeCompletionChunk::Optional)
-        out << chunk.optionalChunks_;
 
     return out;
 }
@@ -86,9 +79,7 @@ QDataStream &operator>>(QDataStream &in, CodeCompletionChunk &chunk)
 {
     in >> chunk.kindAsInt();
     in >> chunk.text_;
-
+    in >> chunk.isOptional_;
-    if (chunk.kind_ == CodeCompletionChunk::Optional)
-        in >> chunk.optionalChunks_;
 
     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)
+    if (chunk.isOptional())
-        debug.nospace() << ", " << chunk.optionalChunks();
+        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) {
+    if (chunk.isOptional())
-        const auto optionalChunks = chunk.optionalChunks();
+        *os << ", optional";
-        *os << ", {";
-
-        for (auto optionalChunkPosition = optionalChunks.cbegin();
-             optionalChunkPosition != optionalChunks.cend();
-             ++optionalChunkPosition) {
-            PrintTo(*optionalChunkPosition, os);
-            if (std::next(optionalChunkPosition) != optionalChunks.cend())
-                *os << ", ";
-        }
-
-        *os << "}";
-    }
 
     *os << "}";
 }

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);

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)
+        if (m_addHtmlTags) {
+            if (!m_previousCodeCompletionChunk.isOptional() && codeCompletionChunk.isOptional())
                 m_text += QStringLiteral("<i>");
-
+            else if (m_previousCodeCompletionChunk.isOptional() && !codeCompletionChunk.isOptional())
-        m_text += convertToFunctionSignature(optionalCodeCompletionChunk.optionalChunks());
-
-        if (m_addHtmlTags)
                 m_text += QStringLiteral("</i>");
         }
     }
+}
 
-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
 

src/plugins/clangcodemodel/clangcompletionchunkstotextconverter.h

@@ -64,9 +64,10 @@ public:
     static QString convertToToolTip(const ClangBackEnd::CodeCompletionChunks &codeCompletionChunks);
 private:
     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<int> m_placeholderPositions;

src/tools/clangbackend/ipcsource/codecompletionchunkconverter.cpp

@@ -42,15 +42,14 @@ void CodeCompletionChunkConverter::extractCompletionChunks(CXCompletionString co
     for (uint chunkIndex = 0; chunkIndex < completionChunkCount; ++chunkIndex) {
         const CodeCompletionChunk::Kind kind = chunkKind(completionString, chunkIndex);
 
-        if (kind == CodeCompletionChunk::Optional)
+        if (kind == CodeCompletionChunk::Optional) {
-            chunks.append(CodeCompletionChunk(kind,
+            extractOptionalCompletionChunks(clang_getCompletionChunkCompletionString(completionString, chunkIndex));
-                                              chunkText(completionString, chunkIndex),
+        } else {
-                                              optionalChunks(completionString, chunkIndex)));
-        else
             chunks.append(CodeCompletionChunk(kind,
                                               chunkText(completionString, chunkIndex)));
         }
     }
+}
 
 void CodeCompletionChunkConverter::extractOptionalCompletionChunks(CXCompletionString completionString)
 {
@@ -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
 

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);

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(", "), true},
-                                                                          {CodeCompletionChunk::Comma, Utf8StringLiteral(", ")},
+                                                                     {CodeCompletionChunk::Placeholder, Utf8StringLiteral("char y"), true},
-                                                                          {CodeCompletionChunk::Placeholder, Utf8StringLiteral("char y")},
+                                                                     {CodeCompletionChunk::Comma, Utf8StringLiteral(", "), true},
-                                                                          {CodeCompletionChunk::Comma, Utf8StringLiteral(", ")},
+                                                                     {CodeCompletionChunk::Placeholder, Utf8StringLiteral("int z"), true},
-                                                                          {CodeCompletionChunk::Placeholder, Utf8StringLiteral("int z")}})},
                                                                      {CodeCompletionChunk::RightParen, Utf8StringLiteral(")")}})));
 }
 

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<i>, int y, int z</i>)"));
@@ -239,7 +249,8 @@ TEST_F(CompletionChunksToTextConverter, EnableIfT)
     CodeCompletionChunks completionChunks({enableIfT,
                                            leftAngle,
                                            enableIfTCondition,
-                                           CodeCompletionChunk(CodeCompletionChunk::Optional, Utf8String(), {comma, enableIfTType}),
+                                           optionalComma,
+                                           optionalEnableIfTType,
                                            rightAngle});
     setupConverterForKeywords();