[PATCH] D104376: [clangd] Correct SelectionTree behavior around anonymous field access.

[Sam McCall via Phabricator via cfe-commits]

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

struct A { struct { int b; }; };
A().^b;

This should be considered a reference to b, but currently it's
considered a reference to the anonymous struct field.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104376

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


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -420,6 +420,18 @@
   //   $def is the definition location (if absent, symbol has no definition)
   //   unnamed range becomes both $decl and $def.
   const char *Tests[] = {
+      R"cpp(
+        struct X {
+          union {
+            int [[a]];
+            float b;
+          };
+        };
+        int test(X &x) {
+          return x.^a;
+        }
+      )cpp",
+
       R"cpp(// Local variable
         int main() {
           int [[bonjour]];
Index: clang-tools-extra/clangd/Selection.cpp
===================================================================
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -57,6 +57,27 @@
     SelectionUsedRecovery.record(0, LanguageLabel); // unused.
 }
 
+SourceRange getSourceRange(const DynTypedNode &N) {
+  // MemberExprs to implicitly access anonymous fields should not claim any
+  // tokens for themselves. Given:
+  //   struct A { struct { int b; }; };
+  // The clang AST reports the following nodes for an access to b:
+  //   A().b;
+  //   [----] MemberExpr, base = A().<anonymous>, member = b
+  //   [----] MemberExpr: base = A(), member = <anonymous>
+  //   [-]    CXXConstructExpr
+  // For our purposes, we don't want the second MemberExpr to own any tokens,
+  // so we reduce its range to match the CXXConstructExpr.
+  // (It's not clear that changing the clang AST would be correct in general).
+  if (const auto *ME = N.get<MemberExpr>()) {
+    if (!ME->getMemberDecl()->getDeclName())
+      return ME->getBase()
+                 ? getSourceRange(DynTypedNode::create(*ME->getBase()))
+                 : SourceRange();
+  }
+  return N.getSourceRange();
+}
+
 // An IntervalSet maintains a set of disjoint subranges of an array.
 //
 // Initially, it contains the entire array.
@@ -608,7 +629,7 @@
   // 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();
+    SourceRange S = getSourceRange(N);
     if (auto *TL = N.get<TypeLoc>()) {
       // FIXME: TypeLoc::getBeginLoc()/getEndLoc() are pretty fragile
       // heuristics. We should consider only pruning critical TypeLoc nodes, to
@@ -665,7 +686,7 @@
   void pop() {
     Node &N = *Stack.top();
     dlog("{1}pop: {0}", printNodeToString(N.ASTNode, PrintPolicy), indent(-1));
-    claimRange(N.ASTNode.getSourceRange(), N.Selected);
+    claimRange(getSourceRange(N.ASTNode), N.Selected);
     if (N.Selected == NoTokens)
       N.Selected = SelectionTree::Unselected;
     if (N.Selected || !N.Children.empty()) {
@@ -868,13 +889,13 @@
 
 const SelectionTree::Node &SelectionTree::Node::ignoreImplicit() const {
   if (Children.size() == 1 &&
-      Children.front()->ASTNode.getSourceRange() == ASTNode.getSourceRange())
+      getSourceRange(Children.front()->ASTNode) == getSourceRange(ASTNode))
     return Children.front()->ignoreImplicit();
   return *this;
 }
 
 const SelectionTree::Node &SelectionTree::Node::outerImplicit() const {
-  if (Parent && Parent->ASTNode.getSourceRange() == ASTNode.getSourceRange())
+  if (Parent && getSourceRange(Parent->ASTNode) == getSourceRange(ASTNode))
     return Parent->outerImplicit();
   return *this;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D104376.352416.patch
Type: text/x-patch
Size: 3543 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210616/22c5be95/attachment.bin>