[clang] [clang][RAV] Visit components of __builtin_offsetof designators (PR #194122)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Apr 25 00:00:34 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Jakob Linke (schuay)
<details>
<summary>Changes</summary>
`RecursiveASTVisitor` previously only traversed the type operand of an `OffsetOfExpr,` ignoring the field/identifier/base/array-index components of the designator. This meant tools built on RAV (clang-tidy, clangd, indexers, ...) silently missed every field reference inside `__builtin_offsetof(T, a.b.c)`.
Add a `TraverseOffsetOfNode / VisitOffsetOfNode` pair following the same pattern used for `ConceptReference`, `ObjCProtocolLoc`, and friends. The `DEF_TRAVERSE_STMT` for `OffsetOfExpr` now invokes `TraverseOffsetOfNode` for each component; array index expressions continue to be reached via the existing children() traversal. Default visitation is a no-op, so the change is opt-in for consumers and behavior-preserving otherwise.
Also expose `OffsetOfNode` as a `DynTypedNode` kind via `ASTTypeTraits` so that downstream machinery (`SelectionTree`, parent maps, matchers) can reference these nodes uniformly.
The same gap exists for Designator inside `DesignatedInitExpr` and can follow this recipe in a follow-up.
Related: https://github.com/llvm/llvm-project/pull/192953. The current PR enables precise component resolves in clangd (support to be added in a followup).
---
Full diff: https://github.com/llvm/llvm-project/pull/194122.diff
8 Files Affected:
- (modified) clang/include/clang/AST/ASTFwd.h (+1)
- (modified) clang/include/clang/AST/ASTTypeTraits.h (+6)
- (modified) clang/include/clang/AST/DynamicRecursiveASTVisitor.h (+5)
- (modified) clang/include/clang/AST/RecursiveASTVisitor.h (+22-4)
- (modified) clang/lib/AST/ASTTypeTraits.cpp (+3)
- (modified) clang/lib/AST/DynamicRecursiveASTVisitor.cpp (+9)
- (modified) clang/unittests/Tooling/CMakeLists.txt (+1)
- (added) clang/unittests/Tooling/RecursiveASTVisitorTests/OffsetOfExpr.cpp (+155)
``````````diff
diff --git a/clang/include/clang/AST/ASTFwd.h b/clang/include/clang/AST/ASTFwd.h
index 8823663386ea1..440ae954ebe46 100644
--- a/clang/include/clang/AST/ASTFwd.h
+++ b/clang/include/clang/AST/ASTFwd.h
@@ -35,6 +35,7 @@ class Attr;
#include "clang/Basic/AttrList.inc"
class ObjCProtocolLoc;
class ConceptReference;
+class OffsetOfNode;
} // end namespace clang
diff --git a/clang/include/clang/AST/ASTTypeTraits.h b/clang/include/clang/AST/ASTTypeTraits.h
index 6f40705fb7436..e0925551ef1a2 100644
--- a/clang/include/clang/AST/ASTTypeTraits.h
+++ b/clang/include/clang/AST/ASTTypeTraits.h
@@ -164,6 +164,7 @@ class ASTNodeKind {
#include "clang/Basic/AttrList.inc"
NKI_ObjCProtocolLoc,
NKI_ConceptReference,
+ NKI_OffsetOfNode,
NKI_NumberOfKinds
};
@@ -224,6 +225,7 @@ KIND_TO_KIND_ID(Attr)
KIND_TO_KIND_ID(ObjCProtocolLoc)
KIND_TO_KIND_ID(CXXBaseSpecifier)
KIND_TO_KIND_ID(ConceptReference)
+KIND_TO_KIND_ID(OffsetOfNode)
#define DECL(DERIVED, BASE) KIND_TO_KIND_ID(DERIVED##Decl)
#include "clang/AST/DeclNodes.inc"
#define STMT(DERIVED, BASE) KIND_TO_KIND_ID(DERIVED)
@@ -589,6 +591,10 @@ template <>
struct DynTypedNode::BaseConverter<ConceptReference, void>
: public PtrConverter<ConceptReference> {};
+template <>
+struct DynTypedNode::BaseConverter<OffsetOfNode, void>
+ : public PtrConverter<OffsetOfNode> {};
+
// The only operation we allow on unsupported types is \c get.
// This allows to conveniently use \c DynTypedNode when having an arbitrary
// AST node that is not supported, but prevents misuse - a user cannot create
diff --git a/clang/include/clang/AST/DynamicRecursiveASTVisitor.h b/clang/include/clang/AST/DynamicRecursiveASTVisitor.h
index 7b5bdca318348..67b87171719e8 100644
--- a/clang/include/clang/AST/DynamicRecursiveASTVisitor.h
+++ b/clang/include/clang/AST/DynamicRecursiveASTVisitor.h
@@ -213,6 +213,11 @@ template <bool IsConst> class DynamicRecursiveASTVisitorBase {
return true;
}
+ /// Recursively visit a single component of an __builtin_offsetof
+ /// designator (a field, identifier, base-class, or array-index node).
+ virtual bool TraverseOffsetOfNode(const OffsetOfNode *Node);
+ virtual bool VisitOffsetOfNode(const OffsetOfNode *Node) { return true; }
+
/// Visit a node.
virtual bool VisitAttr(MaybeConst<Attr> *A) { return true; }
virtual bool VisitDecl(MaybeConst<Decl> *D) { return true; }
diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h
index 27f31efdb1f31..8b1a7bae94d46 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -323,6 +323,16 @@ template <typename Derived> class RecursiveASTVisitor {
// Visit concept reference.
bool VisitConceptReference(ConceptReference *CR) { return true; }
+
+ /// Recursively visit a single component of an __builtin_offsetof
+ /// designator (a field, identifier, base-class, or array-index node).
+ ///
+ /// \returns false if the visitation was terminated early, true otherwise.
+ bool TraverseOffsetOfNode(const OffsetOfNode *Node);
+
+ /// Visit a single component of an __builtin_offsetof designator.
+ bool VisitOffsetOfNode(const OffsetOfNode *Node) { return true; }
+
// ---- Methods on Attrs ----
// Visit an attribute.
@@ -2729,6 +2739,13 @@ bool RecursiveASTVisitor<Derived>::TraverseConceptReference(
return true;
}
+template <typename Derived>
+bool RecursiveASTVisitor<Derived>::TraverseOffsetOfNode(
+ const OffsetOfNode *Node) {
+ TRY_TO(VisitOffsetOfNode(Node));
+ return true;
+}
+
// If shouldVisitImplicitCode() returns false, this method traverses only the
// syntactic form of InitListExpr.
// If shouldVisitImplicitCode() return true, this method is called once for
@@ -2800,11 +2817,12 @@ DEF_TRAVERSE_STMT(CXXNewExpr, {
})
DEF_TRAVERSE_STMT(OffsetOfExpr, {
- // The child-iterator will pick up the expression representing
- // the field.
- // FIXME: for code like offsetof(Foo, a.b.c), should we get
- // making a MemberExpr callbacks for Foo.a, Foo.a.b, and Foo.a.b.c?
TRY_TO(TraverseTypeLoc(S->getTypeSourceInfo()->getTypeLoc()));
+ // Visit each designator component (e.g. the `a`, `b`, `c` in
+ // offsetof(Foo, a.b.c)). Array index expressions are reached through the
+ // child-iterator, which DEF_TRAVERSE_STMT walks automatically.
+ for (unsigned I = 0, E = S->getNumComponents(); I != E; ++I)
+ TRY_TO(TraverseOffsetOfNode(&S->getComponent(I)));
})
DEF_TRAVERSE_STMT(UnaryExprOrTypeTraitExpr, {
diff --git a/clang/lib/AST/ASTTypeTraits.cpp b/clang/lib/AST/ASTTypeTraits.cpp
index 84eb77730b1cb..c717fd77d6ba6 100644
--- a/clang/lib/AST/ASTTypeTraits.cpp
+++ b/clang/lib/AST/ASTTypeTraits.cpp
@@ -56,6 +56,7 @@ const ASTNodeKind::KindInfo ASTNodeKind::AllKindInfo[] = {
#include "clang/Basic/AttrList.inc"
{NKI_None, "ObjCProtocolLoc"},
{NKI_None, "ConceptReference"},
+ {NKI_None, "OffsetOfNode"},
};
bool ASTNodeKind::isBaseOf(ASTNodeKind Other) const {
@@ -279,5 +280,7 @@ SourceRange DynTypedNode::getSourceRange(bool IncludeQualifier) const {
return P->getSourceRange();
if (const ConceptReference *C = get<ConceptReference>())
return C->getSourceRange();
+ if (const OffsetOfNode *O = get<OffsetOfNode>())
+ return O->getSourceRange();
return SourceRange();
}
diff --git a/clang/lib/AST/DynamicRecursiveASTVisitor.cpp b/clang/lib/AST/DynamicRecursiveASTVisitor.cpp
index 6d7925b437b04..dde18aba4fcf7 100644
--- a/clang/lib/AST/DynamicRecursiveASTVisitor.cpp
+++ b/clang/lib/AST/DynamicRecursiveASTVisitor.cpp
@@ -163,6 +163,13 @@ template <bool Const> struct Impl : RecursiveASTVisitor<Impl<Const>> {
return Visitor.TraverseConceptReference(CR);
}
+ bool TraverseOffsetOfNode(const OffsetOfNode *Node) {
+ return Visitor.TraverseOffsetOfNode(Node);
+ }
+ bool VisitOffsetOfNode(const OffsetOfNode *Node) {
+ return Visitor.VisitOffsetOfNode(Node);
+ }
+
bool TraverseCXXBaseSpecifier(const CXXBaseSpecifier &Base) {
return Visitor.TraverseCXXBaseSpecifier(Base);
}
@@ -311,6 +318,8 @@ FORWARD_TO_BASE(TraverseConceptReference, ConceptReference, *)
FORWARD_TO_BASE(TraverseConceptNestedRequirement,
concepts::NestedRequirement, *)
+FORWARD_TO_BASE_EXACT(TraverseOffsetOfNode, const OffsetOfNode *)
+
FORWARD_TO_BASE_EXACT(TraverseCXXBaseSpecifier, const CXXBaseSpecifier &)
FORWARD_TO_BASE_EXACT(TraverseDeclarationNameInfo, DeclarationNameInfo)
FORWARD_TO_BASE_EXACT(TraverseTemplateArgument, const TemplateArgument &)
diff --git a/clang/unittests/Tooling/CMakeLists.txt b/clang/unittests/Tooling/CMakeLists.txt
index 8c8b22250cd83..4de300fa86385 100644
--- a/clang/unittests/Tooling/CMakeLists.txt
+++ b/clang/unittests/Tooling/CMakeLists.txt
@@ -42,6 +42,7 @@ add_clang_unittest(ToolingTests
RecursiveASTVisitorTests/LambdaTemplateParams.cpp
RecursiveASTVisitorTests/MemberPointerTypeLoc.cpp
RecursiveASTVisitorTests/NestedNameSpecifiers.cpp
+ RecursiveASTVisitorTests/OffsetOfExpr.cpp
RecursiveASTVisitorTests/ParenExpr.cpp
RecursiveASTVisitorTests/TemplateArgumentLocTraverser.cpp
RecursiveASTVisitorTests/TraversalScope.cpp
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/OffsetOfExpr.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/OffsetOfExpr.cpp
new file mode 100644
index 0000000000000..742989303c43c
--- /dev/null
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/OffsetOfExpr.cpp
@@ -0,0 +1,155 @@
+//===- unittest/Tooling/RecursiveASTVisitorTests/OffsetOfExpr.cpp ---------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "TestVisitor.h"
+#include "clang/AST/Expr.h"
+
+using namespace clang;
+
+namespace {
+
+struct OffsetOfNodeRecorder : TestVisitor {
+ struct Component {
+ OffsetOfNode::Kind Kind;
+ // Field name for Field/Identifier kinds; empty otherwise.
+ std::string Name;
+ // For Array kind: the slot in OffsetOfExpr's trailing index-expression
+ // array, NOT the literal subscript value. Slots are assigned in source
+ // order (so e.g. `a[2].b[7].c` produces Array nodes referring to slots
+ // 0 and 1, whose Expr*s evaluate to 2 and 7 respectively).
+ unsigned ArrayExprSlot = ~0u;
+ // Whether the pointer received in TraverseOffsetOfNode equals the one
+ // that the enclosing OffsetOfExpr exposes via getComponent().
+ bool PointerStable = false;
+ };
+
+ bool TraverseOffsetOfNode(const OffsetOfNode *Node) override {
+ ++Traversed;
+ return TestVisitor::TraverseOffsetOfNode(Node);
+ }
+
+ bool VisitOffsetOfNode(const OffsetOfNode *Node) override {
+ Component C;
+ C.Kind = Node->getKind();
+ if (C.Kind == OffsetOfNode::Field)
+ C.Name = Node->getField()->getNameAsString();
+ else if (C.Kind == OffsetOfNode::Identifier)
+ C.Name = Node->getFieldName()->getName().str();
+ else if (C.Kind == OffsetOfNode::Array)
+ C.ArrayExprSlot = Node->getArrayExprIndex();
+ C.PointerStable = StableAddresses.count(Node) != 0;
+ Visited.push_back(std::move(C));
+ return true;
+ }
+
+ bool VisitOffsetOfExpr(OffsetOfExpr *E) override {
+ for (unsigned I = 0, N = E->getNumComponents(); I != N; ++I)
+ StableAddresses.insert(&E->getComponent(I));
+ return true;
+ }
+
+ llvm::DenseSet<const OffsetOfNode *> StableAddresses;
+ std::vector<Component> Visited;
+ int Traversed = 0;
+};
+
+TEST(RecursiveASTVisitor, OffsetOfFlatField) {
+ OffsetOfNodeRecorder Recorder;
+ EXPECT_TRUE(Recorder.runOver(
+ "struct Foo { int bar; };\n"
+ "unsigned long x = __builtin_offsetof(struct Foo, bar);\n",
+ OffsetOfNodeRecorder::Lang_C));
+ ASSERT_EQ(1u, Recorder.Visited.size());
+ EXPECT_EQ(1, Recorder.Traversed);
+ EXPECT_EQ(OffsetOfNode::Field, Recorder.Visited[0].Kind);
+ EXPECT_EQ("bar", Recorder.Visited[0].Name);
+ EXPECT_TRUE(Recorder.Visited[0].PointerStable);
+}
+
+TEST(RecursiveASTVisitor, OffsetOfNestedFields) {
+ OffsetOfNodeRecorder Recorder;
+ EXPECT_TRUE(Recorder.runOver(
+ "struct Inner { int c; };\n"
+ "struct Mid { struct Inner b; };\n"
+ "struct Outer { struct Mid a; };\n"
+ "unsigned long x = __builtin_offsetof(struct Outer, a.b.c);\n",
+ OffsetOfNodeRecorder::Lang_C));
+ ASSERT_EQ(3u, Recorder.Visited.size());
+ EXPECT_EQ(3, Recorder.Traversed);
+ EXPECT_EQ(OffsetOfNode::Field, Recorder.Visited[0].Kind);
+ EXPECT_EQ("a", Recorder.Visited[0].Name);
+ EXPECT_EQ(OffsetOfNode::Field, Recorder.Visited[1].Kind);
+ EXPECT_EQ("b", Recorder.Visited[1].Name);
+ EXPECT_EQ(OffsetOfNode::Field, Recorder.Visited[2].Kind);
+ EXPECT_EQ("c", Recorder.Visited[2].Name);
+ for (const auto &C : Recorder.Visited)
+ EXPECT_TRUE(C.PointerStable);
+}
+
+TEST(RecursiveASTVisitor, OffsetOfArrayAndField) {
+ OffsetOfNodeRecorder Recorder;
+ // Two array subscripts mean two Array OffsetOfNodes; their getArrayExprIndex
+ // values should be 0 and 1 in source order, indexing into OffsetOfExpr's
+ // trailing Expr* array (which holds the literals `2` and `7`).
+ EXPECT_TRUE(Recorder.runOver(
+ "struct Inner { int c; };\n"
+ "struct Mid { struct Inner b[10]; };\n"
+ "struct Outer { struct Mid a[5]; };\n"
+ "unsigned long x = __builtin_offsetof(struct Outer, a[2].b[7].c);\n",
+ OffsetOfNodeRecorder::Lang_C));
+ ASSERT_EQ(5u, Recorder.Visited.size());
+ EXPECT_EQ(5, Recorder.Traversed);
+ EXPECT_EQ(OffsetOfNode::Field, Recorder.Visited[0].Kind);
+ EXPECT_EQ("a", Recorder.Visited[0].Name);
+ EXPECT_EQ(OffsetOfNode::Array, Recorder.Visited[1].Kind);
+ EXPECT_EQ(0u, Recorder.Visited[1].ArrayExprSlot);
+ EXPECT_EQ(OffsetOfNode::Field, Recorder.Visited[2].Kind);
+ EXPECT_EQ("b", Recorder.Visited[2].Name);
+ EXPECT_EQ(OffsetOfNode::Array, Recorder.Visited[3].Kind);
+ EXPECT_EQ(1u, Recorder.Visited[3].ArrayExprSlot);
+ EXPECT_EQ(OffsetOfNode::Field, Recorder.Visited[4].Kind);
+ EXPECT_EQ("c", Recorder.Visited[4].Name);
+}
+
+TEST(RecursiveASTVisitor, OffsetOfDependentIdentifier) {
+ OffsetOfNodeRecorder Recorder;
+ EXPECT_TRUE(Recorder.runOver(
+ "template <typename T>\n"
+ "unsigned long off() { return __builtin_offsetof(T, x.y); }\n",
+ OffsetOfNodeRecorder::Lang_CXX11));
+ // Inside a dependent type the components are recorded as Identifier nodes
+ // because the field decls are not yet known. We expect both `x` and `y`.
+ ASSERT_EQ(2u, Recorder.Visited.size());
+ EXPECT_EQ(OffsetOfNode::Identifier, Recorder.Visited[0].Kind);
+ EXPECT_EQ("x", Recorder.Visited[0].Name);
+ EXPECT_EQ(OffsetOfNode::Identifier, Recorder.Visited[1].Kind);
+ EXPECT_EQ("y", Recorder.Visited[1].Name);
+}
+
+// Verifies that overriding only Visit (the typical use case) suffices: the
+// default TraverseOffsetOfNode in (Dynamic)RecursiveASTVisitor must dispatch
+// to VisitOffsetOfNode.
+struct VisitOnly : TestVisitor {
+ int Visits = 0;
+ bool VisitOffsetOfNode(const OffsetOfNode *Node) override {
+ ++Visits;
+ return true;
+ }
+};
+
+TEST(RecursiveASTVisitor, OffsetOfDefaultTraverseDispatchesToVisit) {
+ VisitOnly Recorder;
+ EXPECT_TRUE(Recorder.runOver(
+ "struct Inner { int c; };\n"
+ "struct Outer { struct Inner a; };\n"
+ "unsigned long x = __builtin_offsetof(struct Outer, a.c);\n",
+ VisitOnly::Lang_C));
+ EXPECT_EQ(2, Recorder.Visits);
+}
+
+} // namespace
``````````
</details>
https://github.com/llvm/llvm-project/pull/194122
More information about the cfe-commits
mailing list