From 1df0677e6ac65e18da54b1dd5c391bf17a4c2737 Mon Sep 17 00:00:00 2001 From: Haojian Wu Date: Mon, 7 Dec 2020 10:52:04 +0100 Subject: [PATCH] [clangd] Add language metrics for recovery AST usage. Differential Revision: https://reviews.llvm.org/D92157 --- clang-tools-extra/clangd/Selection.cpp | 19 +++++++++++-------- .../clangd/unittests/SelectionTests.cpp | 16 +++++++++------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/clang-tools-extra/clangd/Selection.cpp b/clang-tools-extra/clangd/Selection.cpp index fbd72be320a79..9f84b4729182e 100644 --- a/clang-tools-extra/clangd/Selection.cpp +++ b/clang-tools-extra/clangd/Selection.cpp @@ -38,21 +38,24 @@ using Node = SelectionTree::Node; using ast_type_traits::DynTypedNode; // Measure the fraction of selections that were enabled by recovery AST. -void recordMetrics(const SelectionTree &S) { +void recordMetrics(const SelectionTree &S, const LangOptions &Lang) { + if (!trace::enabled()) + return; + const char *LanguageLabel = Lang.CPlusPlus ? "C++" : Lang.ObjC ? "ObjC" : "C"; static constexpr trace::Metric SelectionUsedRecovery( - "selection_recovery", trace::Metric::Distribution); - static constexpr trace::Metric RecoveryType("selection_recovery_type", - trace::Metric::Distribution); + "selection_recovery", trace::Metric::Distribution, "language"); + static constexpr trace::Metric RecoveryType( + "selection_recovery_type", trace::Metric::Distribution, "language"); const auto *Common = S.commonAncestor(); for (const auto *N = Common; N; N = N->Parent) { if (const auto *RE = N->ASTNode.get()) { - SelectionUsedRecovery.record(1); // used recovery ast. - RecoveryType.record(RE->isTypeDependent() ? 0 : 1); + SelectionUsedRecovery.record(1, LanguageLabel); // used recovery ast. + RecoveryType.record(RE->isTypeDependent() ? 0 : 1, LanguageLabel); return; } } if (Common) - SelectionUsedRecovery.record(0); // unused. + SelectionUsedRecovery.record(0, LanguageLabel); // unused. } // An IntervalSet maintains a set of disjoint subranges of an array. @@ -834,7 +837,7 @@ SelectionTree::SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens, .printToString(SM)); Nodes = SelectionVisitor::collect(AST, Tokens, PrintPolicy, Begin, End, FID); Root = Nodes.empty() ? nullptr : &Nodes.front(); - recordMetrics(*this); + recordMetrics(*this, AST.getLangOpts()); dlog("Built selection tree\n{0}", *this); } diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp b/clang-tools-extra/clangd/unittests/SelectionTests.cpp index 55b05ebe11ab5..efe06383b04aa 100644 --- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp +++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp @@ -18,6 +18,7 @@ namespace clang { namespace clangd { namespace { +using ::testing::ElementsAreArray; using ::testing::UnorderedElementsAreArray; // Create a selection tree corresponding to a point or pair of points. @@ -452,7 +453,8 @@ TEST(SelectionTest, CommonAncestor) { if (Test.ranges().empty()) { // If no [[range]] is marked in the example, there should be no selection. EXPECT_FALSE(T.commonAncestor()) << C.Code << "\n" << T; - EXPECT_THAT(Tracer.takeMetric("selection_recovery"), testing::IsEmpty()); + EXPECT_THAT(Tracer.takeMetric("selection_recovery", "C++"), + testing::IsEmpty()); } else { // If there is an expected selection, common ancestor should exist // with the appropriate node type. @@ -468,8 +470,8 @@ TEST(SelectionTest, CommonAncestor) { // and no nodes outside it are selected. EXPECT_TRUE(verifyCommonAncestor(T.root(), T.commonAncestor(), C.Code)) << C.Code; - EXPECT_THAT(Tracer.takeMetric("selection_recovery"), - testing::ElementsAreArray({0})); + EXPECT_THAT(Tracer.takeMetric("selection_recovery", "C++"), + ElementsAreArray({0})); } } } @@ -494,10 +496,10 @@ TEST(SelectionTree, Metrics) { auto AST = TestTU::withCode(Annotations(Code).code()).build(); trace::TestTracer Tracer; auto T = makeSelectionTree(Code, AST); - EXPECT_THAT(Tracer.takeMetric("selection_recovery"), - testing::ElementsAreArray({1})); - EXPECT_THAT(Tracer.takeMetric("selection_recovery_type"), - testing::ElementsAreArray({1})); + EXPECT_THAT(Tracer.takeMetric("selection_recovery", "C++"), + ElementsAreArray({1})); + EXPECT_THAT(Tracer.takeMetric("selection_recovery_type", "C++"), + ElementsAreArray({1})); } // FIXME: Doesn't select the binary operator node in