[clangd] Add metrics for selection tree and recovery expressions.

[sammccall]

clang-tools-extra/clangd/Selection.cpp

39

this deserves a comment about what we're tracking and why.
Like "measure the fraction of selections that were enabled by recovery AST"

40

just realized a slightly more elegant(?) way to measure this:

Metric SelectionUsedRecovery("selection_recovery", Distribution);
...
for (...) {
  if (...) {
    SelectionUsedRecovery.record(1); // used
    return;
  }
}
SelectionUsedRecovery.record(0); // not used

Now the count is the total number of selections, and the average is the fraction that used recovery.

48

FWIW, my original RecoveryExpr proposal included a StmtClass for the type of expr that was "supposed" to be here. (e.g. CallExpr for a failed overload resolution). It was dropped to keep the scope minimal.

I'm not sure if that exact idea generalizes to all the places that RecoveryExpr is used, but it would be great to somehow keep track of the semantic context where recovery was used. And I think it would make a great label here. Maybe this would be better as a dedicated enum, or just a string literal (e.g. "variable initializer").

Anyway, an idea for another time...

48

not clear quite what you mean by this - the *actual* type isn't a suitable label, as it's a large unbounded set.
The presence/absense might be interesting I guess, but I'm not sure about it: it's unclear what significance it has if the containing expression's type is know, if we're selecting something inside it. (More significant if the selected expression's type is known/unknown because a contained RecoveryExpr's type is known/unknown, but we can't track that)

I'm not sure this is a FIXME, it seems most likely we decide not to fix it.

[hokein]

address comments, and remove the fixme.

[sammccall]

clang-tools-extra/clangd/unittests/SelectionTests.cpp

454

delete commented code

clang-tools-extra/clangd/Selection.cpp

 //===--- Selection.cpp ----------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Selection.h"
 #include "SourceCode.h"
 #include "support/Logger.h"
+#include "support/Trace.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/TypeLoc.h"

 #include <string>
 
 namespace clang {
 namespace clangd {
 namespace {
 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) {
+  static constexpr trace::Metric SelectionUsedRecovery(
+      "selection_recovery", trace::Metric::Distribution);
+  const auto *Common = S.commonAncestor();
+  for (const auto *N = Common; N; N = N->Parent) {
+    if (N->ASTNode.get<RecoveryExpr>()) {
+      SelectionUsedRecovery.record(1); // used recovery ast.
+      return;
+    }
+  }
+  if (Common)
+    SelectionUsedRecovery.record(0); // unused.
+}
+
 // An IntervalSet maintains a set of disjoint subranges of an array.
 //
 // Initially, it contains the entire array.
 //           [-----------------------------------------------------------]
 //
 // When a range is erased(), it will typically split the array in two.
 //  Claim:                     [--------------------]
 //  after:   [----------------]                      [-------------------]
SelectionTree::SelectionTree(ASTContext &AST, const syntax::TokenBuffer &Tokens,
   PrintPolicy.TerseOutput = true;
   PrintPolicy.IncludeNewlines = false;
 
   dlog("Computing selection for {0}",
        SourceRange(SM.getComposedLoc(FID, Begin), SM.getComposedLoc(FID, End))
            .printToString(SM));
   Nodes = SelectionVisitor::collect(AST, Tokens, PrintPolicy, Begin, End, FID);
   Root = Nodes.empty() ? nullptr : &Nodes.front();
+  recordMetrics(*this);
   dlog("Built selection tree\n{0}", *this);
 }
 
 const Node *SelectionTree::commonAncestor() const {
   const Node *Ancestor = Root;
   while (Ancestor->Children.size() == 1 && !Ancestor->Selected)
     Ancestor = Ancestor->Children.front();
   // Returning nullptr here is a bit unprincipled, but it makes the API safer:

clang-tools-extra/clangd/unittests/SelectionTests.cpp

 //===-- SelectionTests.cpp - ----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 #include "Annotations.h"
 #include "Selection.h"
 #include "SourceCode.h"
 #include "TestTU.h"
+#include "support/TestTracer.h"
 #include "clang/AST/Decl.h"
 #include "llvm/Support/Casting.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace clangd {
 namespace {
Case Cases[] = {
         struct S1 { void f(); };
         struct S2 { S1 * operator->(); };
         void test(S2 s2) {
           s2[[-^>]]f();
         }
       )cpp", "DeclRefExpr"} // DeclRefExpr to the "operator->" method.
   };
   for (const Case &C : Cases) {
+    trace::TestTracer Tracer;
     Annotations Test(C.Code);
 
     TestTU TU;
     TU.Code = std::string(Test.code());
 
     // FIXME: Auto-completion in a template requires disabling delayed template
     // parsing.
     TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
     TU.ExtraArgs.push_back("-xobjective-c++");
 
     auto AST = TU.build();
     auto T = makeSelectionTree(C.Code, AST);
     EXPECT_EQ("TranslationUnitDecl", nodeKind(&T.root())) << C.Code;
 
     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());
     } else {
       // If there is an expected selection, common ancestor should exist
       // with the appropriate node type.
       EXPECT_EQ(C.CommonAncestorKind, nodeKind(T.commonAncestor()))
           << C.Code << "\n"
           << T;
       // Convert the reported common ancestor to a range and verify it.
       EXPECT_EQ(nodeRange(T.commonAncestor(), AST), Test.range())
           << C.Code << "\n"
           << T;
 
       // Check that common ancestor is reachable on exactly one path from root,
       // 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}));
     }
   }
 }
 
 // Regression test: this used to match the injected X, not the outer X.
 TEST(SelectionTest, InjectedClassName) {
   const char *Code = "struct ^X { int x; };";
   auto AST = TestTU::withCode(Annotations(Code).code()).build();
   auto T = makeSelectionTree(Code, AST);
   ASSERT_EQ("CXXRecordDecl", nodeKind(T.commonAncestor())) << T;
   auto *D = dyn_cast<CXXRecordDecl>(T.commonAncestor()->ASTNode.get<Decl>());
   EXPECT_FALSE(D->isInjectedClassName());
 }
 
+TEST(SelectionTree, Metrics) {
+  const char *Code = R"cpp(
+    // error-ok: testing behavior on recovery expression
+    int foo();
+    int foo(int, int);
+    int x = fo^o(42);
+  )cpp";
+  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}));
+}
+
 // FIXME: Doesn't select the binary operator node in
 //          #define FOO(X) X + 1
 //          int a, b = [[FOO(a)]];
 TEST(SelectionTest, Selected) {
   // Selection with ^marks^.
   // Partially selected nodes marked with a [[range]].
   // Completely selected nodes marked with a $C[[range]].
   const char *Cases[] = {