[clangd] consider ~^foo() to target the destructor, not the type

[sammccall]

Summary

This behavior was once deliberate, but i've yet to find someone who likes it.
The reference behavior is unchanged: the foo within ~foo is still considered
a reference to the type. This means rename etc still works.

fixes https://github.com/clangd/clangd/issues/179

[sammccall]

format

[nridge]

Thanks for fixing!

clang-tools-extra/clangd/Selection.cpp

881

Doesn't N.get<MemberExpr> already return a MemberExpr*?

884

remove log

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

469

The FIXME looks like it pertains to the next line where the behaviour is unchanged -- should it be preserved?

[sammccall]

Ugh, sorry for the sloppiness, think I was tired.

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

469

I'm pretty sure the reason for the FIXME was to consistently handle type names within constructors/destructors as referencing the type - with this patch we're (hopefully) consistent in the other direction instead.

(I think I probably wrote this comment, I wish I'd included more context)

clang-tools-extra/clangd/Selection.cpp

void pop() {
     }
     Stack.pop();
   }
 
   // Returns the range of tokens that this node will claim directly, and
   // is not available to the node's children.
   // Usually empty, but sometimes children cover tokens but shouldn't own them.
   SourceRange earlySourceRange(const DynTypedNode &N) {
-    if (const Decl *D = N.get<Decl>()) {
+    if (const Decl *VD = N.get<VarDecl>()) {
       // We want the name in the var-decl to be claimed by the decl itself and
       // not by any children. Ususally, we don't need this, because source
       // ranges of children are not overlapped with their parent's.
       // An exception is lambda captured var decl, where AutoTypeLoc is
       // overlapped with the name loc.
       //    auto fun = [bar = foo]() { ... }
       //                ~~~~~~~~~   VarDecl
       //                ~~~         |- AutoTypeLoc
-      if (const auto *DD = llvm::dyn_cast<VarDecl>(D))
-        return DD->getLocation();
+      return VD->getLocation();
+    }
+
+    // When referring to a destructor ~Foo(), attribute Foo to the destructor
+    // rather than the TypeLoc nested inside it.
+    // We still traverse the TypeLoc, because it may contain other targeted
+    // things like the T in ~Foo<T>().
+    if (const auto *CDD = N.get<CXXDestructorDecl>())
+      return CDD->getNameInfo().getNamedTypeInfo()->getTypeLoc().getBeginLoc();
+    if (const auto *ME = N.get<MemberExpr>()) {
+      auto NameInfo = ME->getMemberNameInfo();
+      if (NameInfo.getName().getNameKind() ==
+          DeclarationName::CXXDestructorName)
+        return NameInfo.getNamedTypeInfo()->getTypeLoc().getBeginLoc();
     }
 
     return SourceRange();
   }
 
   // Claim tokens for N, after processing its children.
   // By default this claims all unclaimed tokens in getSourceRange().
   // We override this if we want to claim fewer tokens (e.g. there are gaps).

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

Case Cases[] = {
             -(int)foo;
             @end
             int test(I *f) { return 42 + [[f.^foo]]; }
           )cpp",
           "ObjCPropertyRefExpr"},
       {"struct foo { [[int has^h<:32:>]]; };", "FieldDecl"},
       {"struct foo { [[op^erator int()]]; };", "CXXConversionDecl"},
       {"struct foo { [[^~foo()]]; };", "CXXDestructorDecl"},
-      // FIXME: The following to should be class itself instead.
+      {"struct foo { [[~^foo()]]; };", "CXXDestructorDecl"},
+      {"template <class T> struct foo { ~foo<[[^T]]>(){} };",
+       "TemplateTypeParmTypeLoc"},
+      {"struct foo {}; void bar(foo *f) { [[f->~^foo]](); }", "MemberExpr"},
       {"struct foo { [[fo^o(){}]] };", "CXXConstructorDecl"},
 
       {R"cpp(
         struct S1 { void f(); };
         struct S2 { S1 * operator->(); };
         void test(S2 s2) {
           s2[[-^>]]f();
         }