[clangd] Include expression in DecltypeTypeLoc sourcerange while building SelectionTree

[kadircet]

Summary

Currently AST only contains the location for decltype keyword,
therefore we were skipping expressions inside decltype while building selection
tree.

This patch extends source range in such cases to contain the expression as well.
A proper fix would require changes to Sema and DecltypeTypeLoc to contain these
location information.

Fixes https://github.com/clangd/clangd/issues/250.

[merge_guards_bot]

Unit tests: pass. 61742 tests passed, 0 failed and 780 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[sammccall]

Thanks!

clang-tools-extra/clangd/Selection.cpp

530

Alternately, we could just always return false if it's a DecltypeTypeLoc.

531

I think this comment could be a little clearer on the three ranges, and that the AST one is just incorrect.

e.g.

// DeclTypeTypeLoc::getSourceRange() is incomplete, which would lead to failing
// to descend into the child expression.
// decltype(2+2);
// XXXXXXXXXXXXX <-- correct range
// XXXXXXXX      <-- range reported by getSourceRange()
// XXXXXXXXXXXX  <-- range with this hack

534

nit: parenthesis

[kadircet]

clang-tools-extra/clangd/Selection.cpp

530

i would rather keep it that way to not regress performance, even if it should be negligible most of the time.

clang-tools-extra/clangd/Selection.cpp

#endif
   // Here the first ("leftmost") sibling claims the tokens it wants, and the
   // other sibling gets what's left. So selecting "int" only includes the left
   // VarDecl in the selection tree.
 
   // An optimization for a common case: nodes outside macro expansions that
   // don't intersect the selection may be recursively skipped.
   bool canSafelySkipNode(const DynTypedNode &N) {
     SourceRange S = N.getSourceRange();
+    if (auto *TL = N.get<TypeLoc>()) {
+      // DeclTypeTypeLoc::getSourceRange() is incomplete, which would lead to
+      // failing
+      // to descend into the child expression.
+      // decltype(2+2);
+      // ~~~~~~~~~~~~~ <-- correct range
+      // ~~~~~~~~      <-- range reported by getSourceRange()
+      // ~~~~~~~~~~~~  <-- range with this hack(i.e, missing closing paren)
+      // FIXME: Alter DecltypeTypeLoc to contain parentheses locations and get
+      // rid of this patch.
+      if (auto DT = TL->getAs<DecltypeTypeLoc>())
+        S.setEnd(DT.getUnderlyingExpr()->getEndLoc());
+    }
     if (!SelChecker.mayHit(S)) {
       dlog("{1}skip: {0}", printNodeToString(N, PrintPolicy), indent());
       dlog("{1}skipped range = {0}", S.printToString(SM), indent(1));
       return true;
     }
     return false;
   }
 

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

Case Cases[] = {
       // For now we treat it as one, and the UserDefinedLiteral as a leaf.
       {
           R"cpp(
             struct Foo{};
             Foo operator""_ud(unsigned long long);
             Foo x = [[^12_ud]];
           )cpp",
           "UserDefinedLiteral"},
+      {
+          R"cpp(
+        int a;
+        decltype([[^a]] + a) b;
+        )cpp",
+          "DeclRefExpr"},
   };
   for (const Case &C : Cases) {
     Annotations Test(C.Code);
 
     TestTU TU;
     TU.Code = Test.code();
 
     // FIXME: Auto-completion in a template requires disabling delayed template