[clang-tools-extra] bb41f85 - [clangd] Correct SelectionTree behavior around anonymous field access.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2021-06-30T17:34:48+02:00
New Revision: bb41f8569138f9f87baf7f4b4e26b3cdcdfd42c6

URL: https://github.com/llvm/llvm-project/commit/bb41f8569138f9f87baf7f4b4e26b3cdcdfd42c6
DIFF: https://github.com/llvm/llvm-project/commit/bb41f8569138f9f87baf7f4b4e26b3cdcdfd42c6.diff

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

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

Differential Revision: https://reviews.llvm.org/D104376

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/Selection.cpp b/clang-tools-extra/clangd/Selection.cpp
index 017f4b22861b5..ad41dec9f20f8 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -57,6 +57,27 @@ void recordMetrics(const SelectionTree &S, const LangOptions &Lang) {
     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 @@ class SelectionVisitor : public RecursiveASTVisitor<SelectionVisitor> {
   // 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 @@ class SelectionVisitor : public RecursiveASTVisitor<SelectionVisitor> {
   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 DeclContext &SelectionTree::Node::getDeclContext() const {
 
 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;
 }

diff  --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 8c37532507d45..166e0674afea6 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -420,6 +420,18 @@ TEST(LocateSymbol, All) {
   //   $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]];