[PATCH] D75106: [clangd] Fix early selection for non-vardecl declarators

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 25 01:46:28 PST 2020


sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/Selection.cpp:613
       // int (*[[s]])();
-      else if (auto *VD = llvm::dyn_cast<VarDecl>(D))
-        return VD->getLocation();
+      if (auto *DD = llvm::dyn_cast<DeclaratorDecl>(D))
+        return DD->getLocation();
----------------
I think we can combine these two cases.
The weird cases handled by FunctionDecl are actually a bit wrong at the moment (early claiming types).
 - conversion operator: we should probably be claiming `operator` only, which hapepns to be getLocation()
 - destructor: we should be claiming ~ only, which is also getLocation()
 - constructor: we shouldn't be claiming anything, need to blacklist
 - all the others seem fine to be handled by the DeclaratorDecl case

This changes more behaviour (and the changes are likely to be seen as regressions in some cases, though more consistent) so we could also consider just landing this as-is.


================
Comment at: clang-tools-extra/clangd/unittests/SelectionTests.cpp:374
+          R"cpp(
+            // We've got this nice trick, as annotation library eagerly selects
+            // the range and if we've got [32] below, there's no way to select
----------------
I'd suggest using digraphs instead, if it works:

`int hash<:32:>`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75106





More information about the cfe-commits mailing list