[clangd] Resolve __builtin_offsetof designator components precisely (#194407)

Building on the new TraverseOffsetOfNode hook in RecursiveASTVisitor and the OffsetOfNode DynTypedNode kind, teach SelectionTree, FindTarget, and the explicit-references collector to address each designator component individually. Cursor positions inside a nested designator (for example the 'B' in __builtin_offsetof(A, B.c)) now resolve to the corresponding field instead of always picking the innermost component. - SelectionTree: wrap each OffsetOfNode visit in traverseNode so it becomes a selectable node alongside its enclosing OffsetOfExpr. - FindTarget::allTargetDecls: resolve OffsetOfNode (Field kind) to its FieldDecl, and drop the OffsetOfExpr fallback so non-component selections do not guess a field target. - ExplicitReferenceCollector: emit one ReferenceLoc per component via a new VisitOffsetOfNode hook, replacing the manual component loop in refInStmt. Tests: - TargetDeclTest.OffsetOf: precise resolution for outer/inner/base/ identifier components, macro form, array index expressions, and non-component selections. - Hover.All and Hover.OffsetOfBuiltin: field cases for nested offsetof, and expression hover on the builtin token. - LocateSymbol.All: simple, outer, inner, macro, inherited-field, and builtin-token cases. - AllRefsInFoo: existing offsetof cases continue to pass via the new per-component VisitOffsetOfNode path. Context: https://github.com/llvm/llvm-project/pull/194122 https://github.com/llvm/llvm-project/pull/192953
2026-04-29 06:39:19 +02:00
parent 19a3d7b5db
commit bcc5d96ccc
5 changed files with 193 additions and 27 deletions
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -294,17 +294,6 @@ public:
            break;
          }
      }
-      void VisitOffsetOfExpr(const OffsetOfExpr *OOE) {
-        for (unsigned I = OOE->getNumComponents(); I != 0; --I) {
-          const OffsetOfNode &Component = OOE->getComponent(I - 1);
-          if (Component.getKind() == OffsetOfNode::Field) {
-            Outer.add(Component.getField(), Flags);
-            // We don't know which component was intended, we assume the
-            // innermost.
-            break;
-          }
-        }
-      }
      void VisitGotoStmt(const GotoStmt *Goto) {
        if (auto *LabelDecl = Goto->getLabel())
          Outer.add(LabelDecl, Flags);
@@ -563,6 +552,10 @@ allTargetDecls(const DynTypedNode &N, const HeuristicResolver *Resolver) {
    Finder.add(PL->getProtocol(), Flags);
  else if (const ConceptReference *CR = N.get<ConceptReference>())
    Finder.add(CR, Flags);
+  else if (const OffsetOfNode *OON = N.get<OffsetOfNode>()) {
+    if (OON->getKind() == OffsetOfNode::Field)
+      Finder.add(OON->getField(), Flags);
+  }
  return Finder.takeDecls();
 }

@@ -826,17 +819,6 @@ llvm::SmallVector<ReferenceLoc> refInStmt(const Stmt *S,
      }
    }

-    void VisitOffsetOfExpr(const OffsetOfExpr *OOE) {
-      for (unsigned I = 0, N = OOE->getNumComponents(); I < N; ++I) {
-        const OffsetOfNode &Component = OOE->getComponent(I);
-        if (Component.getKind() == OffsetOfNode::Field)
-          Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
-                                      Component.getEndLoc(),
-                                      /*IsDecl=*/false,
-                                      {Component.getField()}});
-      }
-    }
-
    void VisitGotoStmt(const GotoStmt *GS) {
      Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
                                  GS->getLabelLoc(),
@@ -1041,6 +1023,11 @@ public:
    return true;
  }

+  bool VisitOffsetOfNode(const OffsetOfNode *N) {
+    visitNode(DynTypedNode::create(*N));
+    return true;
+  }
+
 private:
  /// Obtain information about a reference directly defined in \p N. Does not
  /// recurse into child nodes, e.g. do not expect references for constructor
@@ -1095,6 +1082,14 @@ private:
                           CR->getConceptNameLoc(),
                           /*IsDecl=*/false,
                           {CR->getNamedConcept()}}};
+    if (const OffsetOfNode *OON = N.get<OffsetOfNode>()) {
+      if (OON->getKind() == OffsetOfNode::Field)
+        return {ReferenceLoc{NestedNameSpecifierLoc(),
+                             OON->getEndLoc(),
+                             /*IsDecl=*/false,
+                             {OON->getField()}}};
+      return {};
+    }

    // We do not have location information for other nodes (QualType, etc)
    return {};
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -669,6 +669,9 @@ public:
  bool TraverseConceptReference(ConceptReference *X) {
    return traverseNode(X, [&] { return Base::TraverseConceptReference(X); });
  }
+  bool TraverseOffsetOfNode(const OffsetOfNode *N) {
+    return traverseNode(N, [&] { return Base::TraverseOffsetOfNode(N); });
+  }
  // Stmt is the same, but this form allows the data recursion optimization.
  bool dataTraverseStmtPre(Stmt *X) {
    if (!X || isImplicit(X))
@@ -920,6 +923,15 @@ private:
    if (N.get<ExprWithCleanups>())
      return;

+    if (const auto *OON = N.get<OffsetOfNode>()) {
+      if (OON->getKind() == OffsetOfNode::Array) {
+        // Leave the array index expression to its own child nodes.
+        claimRange(OON->getBeginLoc(), Result);
+        claimRange(OON->getEndLoc(), Result);
+        return;
+      }
+    }
+
    // Declarators nest "inside out", with parent types inside child ones.
    // Instead of claiming the whole range (clobbering parent tokens), carefully
    // claim the tokens owned by this node and non-declarator children.
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -347,14 +347,83 @@ TEST_F(TargetDeclTest, OffsetOf) {
    struct Foo { int bar; };
    int x = __builtin_offsetof(Foo, [[bar]]);
  )cpp";
-  EXPECT_DECLS("OffsetOfExpr", "int bar");
+  EXPECT_DECLS("OffsetOfNode", "int bar");

-  // For a nested designator, allTargetDecls reports the innermost field.
  Code = R"cpp(
-    struct A { struct { int c; } b; };
-    int x = __builtin_offsetof(A, [[b.c]]);
+    struct Inner { int c; };
+    struct Outer { Inner b; };
+    int x = __builtin_offsetof(Outer, [[b]].c);
  )cpp";
-  EXPECT_DECLS("OffsetOfExpr", "int c");
+  EXPECT_DECLS("OffsetOfNode", "Inner b");
+
+  Code = R"cpp(
+    struct Inner { int c; };
+    struct Outer { Inner b; };
+    int x = __builtin_offsetof(Outer, b.[[c]]);
+  )cpp";
+  EXPECT_DECLS("OffsetOfNode", "int c");
+
+  // Selection that spans multiple components doesn't match any single
+  // OffsetOfNode.
+  Code = R"cpp(
+    struct Inner { int c; };
+    struct Outer { Inner b; };
+    int x = __builtin_offsetof(Outer, [[b.c]]);
+  )cpp";
+  EXPECT_THAT(assertNodeAndPrintDecls("OffsetOfExpr"), ::testing::IsEmpty());
+
+  Code = R"cpp(
+    struct Inner { int c; };
+    struct Outer { Inner b; };
+    int x = __builtin_offsetof(Outer, [[b.]]c);
+  )cpp";
+  EXPECT_THAT(assertNodeAndPrintDecls("OffsetOfExpr"), ::testing::IsEmpty());
+
+  Code = R"cpp(
+    struct Inner { int c; };
+    struct Outer { Inner b; };
+    int x = __builtin_offsetof(Outer, b[[.c]]);
+  )cpp";
+  EXPECT_DECLS("OffsetOfNode", "int c");
+
+  Code = R"cpp(
+    struct Foo { int bar; };
+    int x = __builtin_[[offsetof]](Foo, bar);
+  )cpp";
+  EXPECT_THAT(assertNodeAndPrintDecls("OffsetOfExpr"), ::testing::IsEmpty());
+
+  // Base-class component: the implicit Base node has no source range, so
+  // the cursor on `x` resolves precisely to the inherited field.
+  Code = R"cpp(
+    struct B { int x; };
+    struct D : B {};
+    int o = __builtin_offsetof(D, [[x]]);
+  )cpp";
+  EXPECT_DECLS("OffsetOfNode", "int x");
+
+  // Dependent-type identifier component: no resolvable decl, but must not
+  // crash and must not produce spurious targets.
+  Code = R"cpp(
+    template <typename T> int f() { return __builtin_offsetof(T, [[x]]); }
+  )cpp";
+  EXPECT_THAT(assertNodeAndPrintDecls("OffsetOfNode"), ::testing::IsEmpty());
+
+  // C-style offsetof macro form resolves the same as __builtin_offsetof.
+  Code = R"cpp(
+    #define offsetof(t, m) __builtin_offsetof(t, m)
+    struct Foo { int bar; };
+    int x = offsetof(Foo, [[bar]]);
+  )cpp";
+  EXPECT_DECLS("OffsetOfNode", "int bar");
+
+  // Array-index expression in an offsetof designator should still resolve as
+  // the expression, not as the enclosing OffsetOfNode.
+  Code = R"cpp(
+    struct Foo { int bar[4]; };
+    int i;
+    int x = __builtin_offsetof(Foo, bar[ [[i]] ]);
+  )cpp";
+  EXPECT_DECLS("DeclRefExpr", "int i");
 }

 TEST_F(TargetDeclTest, NestedNameSpecifier) {
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1780,6 +1780,18 @@ TEST(Hover, NoHover) {
  }
 }

+TEST(Hover, OffsetOfBuiltin) {
+  Annotations T(R"cpp(
+    struct Foo { int x; };
+    int y = __builtin_o^ffsetof(Foo, x);
+  )cpp");
+  auto AST = TestTU::withCode(T.code()).build();
+  auto H = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+  ASSERT_TRUE(H);
+  EXPECT_EQ(H->Name, "expression");
+  EXPECT_EQ(H->Kind, index::SymbolKind::Unknown);
+}
+
 TEST(Hover, All) {
  struct {
    const char *const Code;
@@ -2007,6 +2019,50 @@ TEST(Hover, All) {
            HI.Type = "int";
            HI.Definition = "int x";
          }},
+      {
+          R"cpp(// Field, offsetof
+            struct Foo { int x; int y; };
+            int z = __builtin_offsetof(Foo, ^[[x]]);
+          )cpp",
+          [](HoverInfo &HI) {
+            HI.Name = "x";
+            HI.Kind = index::SymbolKind::Field;
+            HI.NamespaceScope = "";
+            HI.LocalScope = "Foo::";
+            HI.Type = "int";
+            HI.Definition = "int x";
+            HI.Value = "0";
+          }},
+      {
+          R"cpp(// Outer field, nested offsetof designator
+            struct Inner { int c; };
+            struct A { Inner B; };
+            int z = __builtin_offsetof(A, ^[[B]].c);
+          )cpp",
+          [](HoverInfo &HI) {
+            HI.Name = "B";
+            HI.Kind = index::SymbolKind::Field;
+            HI.NamespaceScope = "";
+            HI.LocalScope = "A::";
+            HI.Type = "Inner";
+            HI.Definition = "Inner B";
+            HI.Value = "0";
+          }},
+      {
+          R"cpp(// Inner field, nested offsetof designator
+            struct Inner { int c; };
+            struct A { Inner B; };
+            int z = __builtin_offsetof(A, B.^[[c]]);
+          )cpp",
+          [](HoverInfo &HI) {
+            HI.Name = "c";
+            HI.Kind = index::SymbolKind::Field;
+            HI.NamespaceScope = "";
+            HI.LocalScope = "Inner::";
+            HI.Type = "int";
+            HI.Definition = "int c";
+            HI.Value = "0";
+          }},
      {
          R"cpp(// Method call
            struct Foo { int x(); };
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -601,6 +601,40 @@ TEST(LocateSymbol, All) {
        }
      )cpp",

+      R"cpp(// Field in offsetof
+        struct Foo { int [[x]]; };
+        int y = __builtin_offsetof(Foo, ^x);
+      )cpp",
+
+      R"cpp(// Outer field in nested offsetof designator
+        struct Inner { int c; };
+        struct A { Inner [[B]]; };
+        int y = __builtin_offsetof(A, ^B.c);
+      )cpp",
+
+      R"cpp(// Inner field in nested offsetof designator
+        struct Inner { int [[c]]; };
+        struct A { Inner B; };
+        int y = __builtin_offsetof(A, B.^c);
+      )cpp",
+
+      R"cpp(// Field in offsetof macro form
+        #define offsetof(t, m) __builtin_offsetof(t, m)
+        struct Foo { int [[x]]; };
+        int y = offsetof(Foo, ^x);
+      )cpp",
+
+      R"cpp(// Inherited field in offsetof
+        struct B { int [[x]]; };
+        struct D : B {};
+        int y = __builtin_offsetof(D, ^x);
+      )cpp",
+
+      R"cpp(// Builtin offsetof name is not a field reference.
+        struct Foo { int x; };
+        int y = __builtin_o^ffsetof(Foo, x);
+      )cpp",
+
      R"cpp(// Method call
        struct Foo { int $decl[[x]](); };
        int main() {