[PATCH] D116618: [clangd] Use the localSourceRange as the default claimed range for typelocs.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
hokein requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

It also fixes a bug where SelectionTree doesn't select the right AST node on
`int (*Fun(OuterT^ype))(InnerType);`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116618

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -231,7 +231,7 @@
           R"cpp(
             [[void (^*S)(int)]] = nullptr;
           )cpp",
-          "FunctionProtoTypeLoc",
+          "PointerTypeLoc",
       },
       {
           R"cpp(
@@ -243,7 +243,7 @@
           R"cpp(
             [[void ^(*S)(int)]] = nullptr;
           )cpp",
-          "FunctionProtoTypeLoc",
+          "ParenTypeLoc",
       },
       {
           R"cpp(
@@ -326,6 +326,8 @@
       {"const int x = 1, y = 2; [[i^nt]] array[x][10][y];", "BuiltinTypeLoc"},
       {"void func(int x) { int v_array[^[[x]]][10]; }", "DeclRefExpr"},
 
+      {"int (*getFunc([[do^uble]]))(int);", "BuiltinTypeLoc"},
+
       // FIXME: the AST has no location info for qualifiers.
       {"const [[a^uto]] x = 42;", "AutoTypeLoc"},
       {"[[co^nst auto x = 42]];", "VarDecl"},
Index: clang-tools-extra/clangd/Selection.cpp
===================================================================
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -754,19 +754,40 @@
   // We override this if we want to claim fewer tokens (e.g. there are gaps).
   void claimTokensFor(const DynTypedNode &N, SelectionTree::Selection &Result) {
     if (const auto *TL = N.get<TypeLoc>()) {
-      // e.g. EltType Foo[OuterSize][InnerSize];
+      // We need the DecltypeTypeLoc hack in getSourceRange.
+      if (TL->getAs<DecltypeTypeLoc>())
+        return claimRange(getSourceRange(N), Result);
+
+      // E.g. a function which returns a function pointer.
+      // int (*Fun(OuterType))(InnerType);
+      // ~~~~~~~~~XXXXXXXXXXX~~~~~~~~~~~~ FunctionProtoTypeLoc
+      // ~~~~~X~~~~~~~~~~~~~~~~~~~~~~~~~~ |- PointerTypeLoc: int (*)(InnerType)
+      // ~~~~XXXXXXXXXXXXXXXXX~~~~~~~~~~~ |  `- ParenTypeLoc: int (InnerType)
+      // ~~~~~~~~~~~~~~~~~~~~~XXXXXXXXXXX |     `-FunctionProtoTypeLoc: int (InnerType)
+      //           ~~~~~~~~~              `- ParmVarDecl: OuterType
+      // The ParenTypeLoc must not claim its local source range (marked as XXX
+      // above), or it clobbers the outer ParmVarDecl.
+      if ( auto PTL = TL->getAs<ParenTypeLoc>()) {
+        PTL.getLocalSourceRange().dump(SM);
+        // Only claim the parentheses tokens, other tokens are handled by its
+        // children or are expected unclaimed.
+        claimRange(PTL.getLParenLoc(), Result);
+        claimRange(PTL.getRParenLoc(), Result);
+        return;
+      }
+
+      // We use LocalSourceRange as a default claimed range for TypeLocs -- it
+      // is better than getSourceRange (which returns a range covering itself
+      // and its children), because we want to claim fewer tokens (multiple
+      // discontiguous ranges) for some TypeLocs.
+      // E.g. EltType Foo[OuterSize][InnerSize];
       //      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ArrayTypeLoc (Outer)
       //      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ |-ArrayTypeLoc (Inner)
       //      ~~~~~~~                           | |-RecordType
       //                             ~~~~~~~~~  | `-Expr (InnerSize)
       //                  ~~~~~~~~~             `-Expr (OuterSize)
       // Inner ATL must not claim its whole SourceRange, or it clobbers Outer.
-      if (TL->getAs<ArrayTypeLoc>()) {
-        claimRange(TL->getLocalSourceRange(), Result);
-        return;
-      }
-      // FIXME: maybe LocalSourceRange is a better default for TypeLocs.
-      // It doesn't seem to be usable for FunctionTypeLocs.
+      return claimRange(TL->getLocalSourceRange(), Result);
     }
     claimRange(getSourceRange(N), Result);
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D116618.397375.patch
Type: text/x-patch
Size: 3792 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220104/16dae7db/attachment.bin>