From a1f088376a79443f0738228e1b20f4642342197a Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Mon, 25 Oct 2021 18:02:38 +0200 Subject: [PATCH] ClangCodeModel: Mark output arguments also for lambdas ... with clangd. This required rewriting getAstPath(), because the previous implementation did not do the necessary backtracking and could therefore miss the AST branch containing the node fully matching the input range. Task-number: QTCREATORBUG-22381 Change-Id: Id5caf2a401b920c0e76f742bec97b5ca6977b4df Reviewed-by: David Schulz --- src/plugins/clangcodemodel/clangdclient.cpp | 114 ++++++++++++------ .../clangcodemodel/test/clangdtests.cpp | 10 +- .../test/data/highlighting/highlighting.cpp | 11 ++ 3 files changed, 97 insertions(+), 38 deletions(-) diff --git a/src/plugins/clangcodemodel/clangdclient.cpp b/src/plugins/clangcodemodel/clangdclient.cpp index cc192fc9c18..0c6b16e2bc9 100644 --- a/src/plugins/clangcodemodel/clangdclient.cpp +++ b/src/plugins/clangcodemodel/clangdclient.cpp @@ -359,49 +359,89 @@ public: } }; -static QList getAstPath(const AstNode &root, const Range &range) +class AstPathCollector { - QList path; - QList queue{root}; - bool isRoot = true; +public: + AstPathCollector(const AstNode &root, const Range &range) : m_root(root), m_range(range) {} - while (!queue.isEmpty()) { - AstNode curNode = queue.takeFirst(); - if (!isRoot && !curNode.hasRange()) - continue; - if (curNode.range() == range) - return path << curNode; - if (isRoot || curNode.range().contains(range)) { - path << curNode; - const auto children = curNode.children(); - if (!children) - break; - if (curNode.kind() == "Function" || curNode.role() == "expression") { - // Functions and expressions can contain implicit nodes that make the list unsorted. - // They cannot be ignored, as we need to consider them in certain contexts. - // Therefore, the binary search cannot be used here. - queue = *children; - } else { - queue.clear(); + QList collectPath() + { + if (!m_root.isValid()) + return {}; + visitNode(m_root, true); + return m_done ? m_path : m_longestSubPath; + } - // Class and struct nodes can contain implicit constructors, destructors and - // operators, which appear at the end of the list, but whose range is the same - // as the class name. Therefore, we must force them not to compare less to - // anything else. - static const auto leftOfRange = [](const AstNode &node, const Range &range) { - return node.range().isLeftOf(range) && !node.arcanaContains(" implicit "); - }; +private: + void visitNode(const AstNode &node, bool isRoot = false) + { + if (!isRoot && (!node.hasRange() || !node.range().contains(m_range))) + return; + m_path << node; - for (auto it = std::lower_bound(children->cbegin(), children->cend(), range, - leftOfRange); - it != children->cend() && !range.isLeftOf(it->range()); ++it) { - queue << *it; - } + class PathDropper { + public: + PathDropper(AstPathCollector &collector) : m_collector(collector) {}; + ~PathDropper() { + if (m_collector.m_done) + return; + if (m_collector.m_path.size() > m_collector.m_longestSubPath.size()) + m_collector.m_longestSubPath = m_collector.m_path; + m_collector.m_path.removeLast(); + } + private: + AstPathCollector &m_collector; + } pathDropper(*this); + + // Still traverse the children, because they could have the same range. + if (node.range() == m_range) + m_done = true; + + const auto children = node.children(); + if (!children) + return; + + QList childrenToCheck; + if (node.kind() == "Function" || node.role() == "expression") { + // Functions and expressions can contain implicit nodes that make the list unsorted. + // They cannot be ignored, as we need to consider them in certain contexts. + // Therefore, the binary search cannot be used here. + childrenToCheck = *children; + } else { + for (auto it = std::lower_bound(children->cbegin(), children->cend(), m_range, + leftOfRange); + it != children->cend() && !m_range.isLeftOf(it->range()); ++it) { + childrenToCheck << *it; } } - isRoot = false; + + const bool wasDone = m_done; + for (const AstNode &child : qAsConst(childrenToCheck)) { + visitNode(child); + if (m_done && !wasDone) + break; + } } - return path; + + static bool leftOfRange(const AstNode &node, const Range &range) + { + // Class and struct nodes can contain implicit constructors, destructors and + // operators, which appear at the end of the list, but whose range is the same + // as the class name. Therefore, we must force them not to compare less to + // anything else. + return node.range().isLeftOf(range) && !node.arcanaContains(" implicit "); + }; + + const AstNode &m_root; + const Range &m_range; + QList m_path; + QList m_longestSubPath; + bool m_done = false; +}; + +static QList getAstPath(const AstNode &root, const Range &range) +{ + return AstPathCollector(root, range).collectPath(); } static QList getAstPath(const AstNode &root, const Position &pos) @@ -2334,7 +2374,7 @@ static void semanticHighlighter(QFutureInterface &future, return false; for (auto it = path.rbegin() + 1; it != path.rend(); ++it) { if (it->kind() == "Call" || it->kind() == "CXXConstruct" - || it->kind() == "MemberInitializer") { + || it->kind() == "MemberInitializer" || it->kind() == "CXXOperatorCall") { return true; } if (it->kind().endsWith("Cast") && it->hasConstType()) diff --git a/src/plugins/clangcodemodel/test/clangdtests.cpp b/src/plugins/clangcodemodel/test/clangdtests.cpp index e31cc4d6f55..82e64fad787 100644 --- a/src/plugins/clangcodemodel/test/clangdtests.cpp +++ b/src/plugins/clangcodemodel/test/clangdtests.cpp @@ -1040,7 +1040,7 @@ void ClangdTestHighlighting::test_data() QTest::newRow("typedef as underlying type in enum declaration") << 424 << 21 << 424 << 39 << QList{C_TYPE} << 0; QTest::newRow("argument to user-defined subscript operator") << 434 << 12 << 434 << 17 - << QList{C_PARAMETER} << 0; + << QList{C_PARAMETER, C_OUTPUT_ARGUMENT} << 0; QTest::newRow("partial class template specialization") << 553 << 25 << 553 << 28 << QList{C_TYPE, C_DECLARATION} << 0; QTest::newRow("using declaration for function") << 556 << 10 << 556 << 13 @@ -1237,6 +1237,14 @@ void ClangdTestHighlighting::test_data() << QList{C_LOCAL, C_OUTPUT_ARGUMENT} << 0; QTest::newRow("override attribute") << 186 << 28 << 186 << 36 << QList{C_KEYWORD} << 0; QTest::newRow("final attribute") << 187 << 33 << 187 << 38 << QList{C_KEYWORD} << 0; + QTest::newRow("non-const argument to named lambda") << 827 << 10 << 827 << 13 + << QList{C_LOCAL, C_OUTPUT_ARGUMENT} << 0; + QTest::newRow("const argument to named lambda") << 828 << 10 << 828 << 13 + << QList{C_LOCAL} << 0; + QTest::newRow("non-const argument to unnamed lambda") << 829 << 18 << 829 << 21 + << QList{C_LOCAL, C_OUTPUT_ARGUMENT} << 0; + QTest::newRow("const argument to unnamed lambda") << 830 << 16 << 830 << 19 + << QList{C_LOCAL} << 0; } void ClangdTestHighlighting::test() diff --git a/src/plugins/clangcodemodel/test/data/highlighting/highlighting.cpp b/src/plugins/clangcodemodel/test/data/highlighting/highlighting.cpp index d9b08689f12..6e0ed1c8ec8 100644 --- a/src/plugins/clangcodemodel/test/data/highlighting/highlighting.cpp +++ b/src/plugins/clangcodemodel/test/data/highlighting/highlighting.cpp @@ -818,3 +818,14 @@ void staticMemberFuncTest() { int i; s.staticFunc(i); } + +void lambdaArgTest() +{ + const auto foo1 = [](int &) {}; + const auto foo2 = [](int) {}; + int val; + foo1(val); + foo2(val); + [](int &) {}(val); + [](int) {}(val); +}