[PATCH] D116630: [clangd] Handle declarators more consistently in Selection.

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

Thanks, this looks better!



================
Comment at: clang-tools-extra/clangd/Selection.cpp:756
+        claimRange(
+            SourceRange(FTL.getParensRange().getBegin(), FTL.getEndLoc()),
+            Result);
----------------
nit: FTL.getLParenLoc().


================
Comment at: clang-tools-extra/clangd/Selection.cpp:757
     if (const auto *TL = N.get<TypeLoc>()) {
-      // e.g. EltType Foo[OuterSize][InnerSize];
-      //      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ArrayTypeLoc (Outer)
----------------
I'd prefer to keep this comment, while we have some high-level and abstract comment in the new patch, I think it is still useful to give us some details (otherwise, I'd probably forget everything when coming back to read the code a few months later).


I also want to add the example `int (*Fun(OuterType))(InnerType);` from D116618 here, I understand it might be too verbose (personally, I find it useful to understand this part of code), up to you.


================
Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:209
           )cpp",
-          "CXXConstructExpr",
+          "VarDecl",
       },
----------------
Maybe a FIXME for "CXXConstructExpr is better", but I don't come up with a good fix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116630/new/

https://reviews.llvm.org/D116630