[clang] [clang] Replace Sema::OffsetOfComponent with Designation (PR #199097)
Jakob Linke via cfe-commits
cfe-commits at lists.llvm.org
Sun Jun 28 02:48:44 PDT 2026
https://github.com/schuay updated https://github.com/llvm/llvm-project/pull/199097
>From ac61f03d765724277cb990c293809adf76ea855f Mon Sep 17 00:00:00 2001
From: Jakob Linke <jakob at linke.cx>
Date: Thu, 30 Apr 2026 18:50:59 +0200
Subject: [PATCH] [clang] Replace Sema::OffsetOfComponent with Designation
The parser previously fed __builtin_offsetof through a small ad-hoc
struct (Sema::OffsetOfComponent) that mirrors the same path data already
modeled by Designation/Designator. With the recent code-completion
work, the parser was building both shapes side by side.
Drop OffsetOfComponent. ActOnBuiltinOffsetOf, BuildBuiltinOffsetOf, and
TreeTransform::RebuildOffsetOfExpr now take a const Designation &.
- Parser: stops maintaining the parallel Comps vector and feeds the
Designation it already collected for completion straight to Sema.
- BuildBuiltinOffsetOf: iterates over the Designation.
- TreeTransform: rebuilds a Designation from each OffsetOfNode. The
AST node already encodes "leading-dot collapsed to name location"
via its Range constructor, so we mirror that by passing an empty
DotLoc whenever the new Designation is still empty.
- OffsetOfNode: tighten the identifier constructor and getFieldName
accessor to const IdentifierInfo*, matching Designator's accessor and
letting BuildBuiltinOffsetOf drop the lingering const_cast.
No functional change intended; existing offsetof tests cover dependent
types, dependent bases, anonymous unions, indirect fields, base-class
indirections, and the non-POD diagnostic source range.
---
clang/include/clang/AST/Expr.h | 4 +-
clang/include/clang/Sema/Designator.h | 13 ++++++
clang/include/clang/Sema/Sema.h | 14 +------
clang/lib/AST/Expr.cpp | 2 +-
clang/lib/Parse/ParseExpr.cpp | 33 ++++-----------
clang/lib/Sema/SemaExpr.cpp | 59 ++++++++++++++-------------
clang/lib/Sema/TreeTransform.h | 43 +++++++++----------
7 files changed, 76 insertions(+), 92 deletions(-)
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index b91bf4a5375fb..63d3725163dad 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -2465,7 +2465,7 @@ class OffsetOfNode {
Data(reinterpret_cast<uintptr_t>(Field) | OffsetOfNode::Field) {}
/// Create an offsetof node that refers to an identifier.
- OffsetOfNode(SourceLocation DotLoc, IdentifierInfo *Name,
+ OffsetOfNode(SourceLocation DotLoc, const IdentifierInfo *Name,
SourceLocation NameLoc)
: Range(DotLoc.isValid() ? DotLoc : NameLoc, NameLoc),
Data(reinterpret_cast<uintptr_t>(Name) | Identifier) {}
@@ -2492,7 +2492,7 @@ class OffsetOfNode {
/// For a field or identifier offsetof node, returns the name of
/// the field.
- IdentifierInfo *getFieldName() const;
+ const IdentifierInfo *getFieldName() const;
/// For a base class node, returns the base specifier.
CXXBaseSpecifier *getBase() const {
diff --git a/clang/include/clang/Sema/Designator.h b/clang/include/clang/Sema/Designator.h
index 244535978d4bf..94617c7e8f635 100644
--- a/clang/include/clang/Sema/Designator.h
+++ b/clang/include/clang/Sema/Designator.h
@@ -135,6 +135,19 @@ class Designator {
return FieldInfo.FieldLoc;
}
+ /// Returns the start location of this designator. A leading field designator
+ /// has no '.', so its location collapses to the field-name location.
+ SourceLocation getBeginLoc() const {
+ if (isFieldDesignator())
+ return getDotLoc().isValid() ? getDotLoc() : getFieldLoc();
+ return getLBracketLoc();
+ }
+
+ /// Returns the end location of this designator.
+ SourceLocation getEndLoc() const {
+ return isFieldDesignator() ? getFieldLoc() : getRBracketLoc();
+ }
+
//===--------------------------------------------------------------------===//
// ArrayDesignatorInfo:
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index e71794b2d92c9..c8954f95a49ae 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -7639,25 +7639,15 @@ class Sema final : public SemaBase {
ExprResult ActOnStmtExprResult(ExprResult E);
void ActOnStmtExprError();
- // __builtin_offsetof(type, identifier(.identifier|[expr])*)
- struct OffsetOfComponent {
- SourceLocation LocStart, LocEnd;
- bool isBrackets; // true if [expr], false if .ident
- union {
- IdentifierInfo *IdentInfo;
- Expr *E;
- } U;
- };
-
/// __builtin_offsetof(type, a.b[123][456].c)
ExprResult BuildBuiltinOffsetOf(SourceLocation BuiltinLoc,
TypeSourceInfo *TInfo,
- ArrayRef<OffsetOfComponent> Components,
+ const Designation &Desig,
SourceLocation RParenLoc);
ExprResult ActOnBuiltinOffsetOf(Scope *S, SourceLocation BuiltinLoc,
SourceLocation TypeLoc,
ParsedType ParsedArgTy,
- ArrayRef<OffsetOfComponent> Components,
+ const Designation &Desig,
SourceLocation RParenLoc);
// __builtin_choose_expr(constExpr, expr1, expr2)
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 90747be4208e1..a80e81893c8af 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -1690,7 +1690,7 @@ OffsetOfExpr::OffsetOfExpr(const ASTContext &C, QualType type,
setDependence(computeDependence(this));
}
-IdentifierInfo *OffsetOfNode::getFieldName() const {
+const IdentifierInfo *OffsetOfNode::getFieldName() const {
assert(getKind() == Field || getKind() == Identifier);
if (getKind() == Field)
return getField()->getIdentifier();
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index e38481f05da63..467a7c34779a4 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -2425,16 +2425,6 @@ ExprResult Parser::ParseBuiltinPrimaryExpression() {
return ExprError();
}
- // Keep track of the various subcomponents we see.
- // FIXME: Comps and D below carry the same designator chain in two
- // different shapes. ActOnBuiltinOffsetOf should be taught to accept a
- // Designation directly so this duplication can go away.
- SmallVector<Sema::OffsetOfComponent, 4> Comps;
-
- Comps.push_back(Sema::OffsetOfComponent());
- Comps.back().isBrackets = false;
- Comps.back().U.IdentInfo = Tok.getIdentifierInfo();
- Comps.back().LocStart = Comps.back().LocEnd = Tok.getLocation();
D.AddDesignator(Designator::CreateFieldDesignator(
Tok.getIdentifierInfo(), SourceLocation(), Tok.getLocation()));
ConsumeToken();
@@ -2443,9 +2433,7 @@ ExprResult Parser::ParseBuiltinPrimaryExpression() {
while (true) {
if (Tok.is(tok::period)) {
// offsetof-member-designator: offsetof-member-designator '.' identifier
- Comps.push_back(Sema::OffsetOfComponent());
- Comps.back().isBrackets = false;
- Comps.back().LocStart = ConsumeToken();
+ SourceLocation DotLoc = ConsumeToken();
if (Tok.is(tok::code_completion)) {
TriggerCompletion(D);
@@ -2456,33 +2444,26 @@ ExprResult Parser::ParseBuiltinPrimaryExpression() {
SkipUntil(tok::r_paren, StopAtSemi);
return ExprError();
}
- Comps.back().U.IdentInfo = Tok.getIdentifierInfo();
- Comps.back().LocEnd = Tok.getLocation();
D.AddDesignator(Designator::CreateFieldDesignator(
- Tok.getIdentifierInfo(), Comps.back().LocStart, Tok.getLocation()));
+ Tok.getIdentifierInfo(), DotLoc, Tok.getLocation()));
ConsumeToken();
} else if (Tok.is(tok::l_square)) {
if (CheckProhibitedCXX11Attribute())
return ExprError();
// offsetof-member-designator: offsetof-member-design '[' expression ']'
- Comps.push_back(Sema::OffsetOfComponent());
- Comps.back().isBrackets = true;
BalancedDelimiterTracker ST(*this, tok::l_square);
ST.consumeOpen();
- Comps.back().LocStart = ST.getOpenLocation();
Res = ParseExpression();
if (Res.isInvalid()) {
SkipUntil(tok::r_paren, StopAtSemi);
return Res;
}
- Comps.back().U.E = Res.get();
ST.consumeClose();
- Comps.back().LocEnd = ST.getCloseLocation();
Designator ArrayD =
- Designator::CreateArrayDesignator(Res.get(), Comps.back().LocStart);
- ArrayD.setRBracketLoc(Comps.back().LocEnd);
+ Designator::CreateArrayDesignator(Res.get(), ST.getOpenLocation());
+ ArrayD.setRBracketLoc(ST.getCloseLocation());
D.AddDesignator(ArrayD);
} else {
// A code-completion token here (e.g. cursor right after `]`) is past
@@ -2500,9 +2481,9 @@ ExprResult Parser::ParseBuiltinPrimaryExpression() {
Res = ExprError();
} else {
PT.consumeClose();
- Res = Actions.ActOnBuiltinOffsetOf(getCurScope(), StartLoc, TypeLoc,
- Ty.get(), Comps,
- PT.getCloseLocation());
+ Res =
+ Actions.ActOnBuiltinOffsetOf(getCurScope(), StartLoc, TypeLoc,
+ Ty.get(), D, PT.getCloseLocation());
}
break;
}
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 521a8516ac179..8d98d2bc57f19 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -16614,7 +16614,7 @@ ExprResult Sema::ActOnStmtExprResult(ExprResult ER) {
ExprResult Sema::BuildBuiltinOffsetOf(SourceLocation BuiltinLoc,
TypeSourceInfo *TInfo,
- ArrayRef<OffsetOfComponent> Components,
+ const Designation &Desig,
SourceLocation RParenLoc) {
QualType ArgTy = TInfo->getType();
bool Dependent = ArgTy->isDependentType();
@@ -16637,20 +16637,22 @@ ExprResult Sema::BuildBuiltinOffsetOf(SourceLocation BuiltinLoc,
bool DidWarnAboutNonPOD = false;
QualType CurrentType = ArgTy;
SmallVector<OffsetOfNode, 4> Comps;
- SmallVector<Expr*, 4> Exprs;
- for (const OffsetOfComponent &OC : Components) {
- if (OC.isBrackets) {
+ SmallVector<Expr *, 4> Exprs;
+ for (unsigned I = 0, N = Desig.getNumDesignators(); I != N; ++I) {
+ const Designator &D = Desig.getDesignator(I);
+ assert(!D.isArrayRangeDesignator());
+ if (D.isArrayDesignator()) {
// Offset of an array sub-field. TODO: Should we allow vector elements?
if (!CurrentType->isDependentType()) {
const ArrayType *AT = Context.getAsArrayType(CurrentType);
if(!AT)
- return ExprError(Diag(OC.LocEnd, diag::err_offsetof_array_type)
+ return ExprError(Diag(D.getEndLoc(), diag::err_offsetof_array_type)
<< CurrentType);
CurrentType = AT->getElementType();
} else
CurrentType = Context.DependentTy;
- ExprResult IdxRval = DefaultLvalueConversion(static_cast<Expr*>(OC.U.E));
+ ExprResult IdxRval = DefaultLvalueConversion(D.getArrayIndex());
if (IdxRval.isInvalid())
return ExprError();
Expr *Idx = IdxRval.get();
@@ -16664,29 +16666,33 @@ ExprResult Sema::BuildBuiltinOffsetOf(SourceLocation BuiltinLoc,
<< Idx->getSourceRange());
// Record this array index.
- Comps.push_back(OffsetOfNode(OC.LocStart, Exprs.size(), OC.LocEnd));
+ Comps.push_back(
+ OffsetOfNode(D.getBeginLoc(), Exprs.size(), D.getEndLoc()));
Exprs.push_back(Idx);
continue;
}
+ assert(D.isFieldDesignator());
+ const IdentifierInfo *Name = D.getFieldDecl();
+
// Offset of a field.
if (CurrentType->isDependentType()) {
// We have the offset of a field, but we can't look into the dependent
// type. Just record the identifier of the field.
- Comps.push_back(OffsetOfNode(OC.LocStart, OC.U.IdentInfo, OC.LocEnd));
+ Comps.push_back(OffsetOfNode(D.getBeginLoc(), Name, D.getEndLoc()));
CurrentType = Context.DependentTy;
continue;
}
// We need to have a complete type to look into.
- if (RequireCompleteType(OC.LocStart, CurrentType,
+ if (RequireCompleteType(D.getBeginLoc(), CurrentType,
diag::err_offsetof_incomplete_type))
return ExprError();
// Look for the designated field.
auto *RD = CurrentType->getAsRecordDecl();
if (!RD)
- return ExprError(Diag(OC.LocEnd, diag::err_offsetof_record_type)
+ return ExprError(Diag(D.getEndLoc(), diag::err_offsetof_record_type)
<< CurrentType);
// C++ [lib.support.types]p5:
@@ -16704,13 +16710,14 @@ ExprResult Sema::BuildBuiltinOffsetOf(SourceLocation BuiltinLoc,
if (!IsSafe && !DidWarnAboutNonPOD && !isUnevaluatedContext()) {
Diag(BuiltinLoc, DiagID)
- << SourceRange(Components[0].LocStart, OC.LocEnd) << CurrentType;
+ << SourceRange(Desig.getDesignator(0).getBeginLoc(), D.getEndLoc())
+ << CurrentType;
DidWarnAboutNonPOD = true;
}
}
// Look for the field.
- LookupResult R(*this, OC.U.IdentInfo, OC.LocStart, LookupMemberName);
+ LookupResult R(*this, Name, D.getBeginLoc(), LookupMemberName);
LookupQualifiedName(R, RD);
FieldDecl *MemberDecl = R.getAsSingle<FieldDecl>();
IndirectFieldDecl *IndirectMemberDecl = nullptr;
@@ -16725,7 +16732,7 @@ ExprResult Sema::BuildBuiltinOffsetOf(SourceLocation BuiltinLoc,
// In that case we would already have emitted a diagnostic
if (!R.isAmbiguous())
Diag(BuiltinLoc, diag::err_no_member)
- << OC.U.IdentInfo << RD << SourceRange(OC.LocStart, OC.LocEnd);
+ << Name << RD << SourceRange(D.getBeginLoc(), D.getEndLoc());
return ExprError();
}
@@ -16734,9 +16741,8 @@ ExprResult Sema::BuildBuiltinOffsetOf(SourceLocation BuiltinLoc,
//
// We diagnose this as an error.
if (MemberDecl->isBitField()) {
- Diag(OC.LocEnd, diag::err_offsetof_bitfield)
- << MemberDecl->getDeclName()
- << SourceRange(BuiltinLoc, RParenLoc);
+ Diag(D.getEndLoc(), diag::err_offsetof_bitfield)
+ << MemberDecl->getDeclName() << SourceRange(BuiltinLoc, RParenLoc);
Diag(MemberDecl->getLocation(), diag::note_bitfield_decl);
return ExprError();
}
@@ -16748,12 +16754,11 @@ ExprResult Sema::BuildBuiltinOffsetOf(SourceLocation BuiltinLoc,
// If the member was found in a base class, introduce OffsetOfNodes for
// the base class indirections.
CXXBasePaths Paths;
- if (IsDerivedFrom(OC.LocStart, CurrentType,
+ if (IsDerivedFrom(D.getBeginLoc(), CurrentType,
Context.getCanonicalTagType(Parent), Paths)) {
if (Paths.getDetectedVirtual()) {
- Diag(OC.LocEnd, diag::err_offsetof_field_of_virtual_base)
- << MemberDecl->getDeclName()
- << SourceRange(BuiltinLoc, RParenLoc);
+ Diag(D.getEndLoc(), diag::err_offsetof_field_of_virtual_base)
+ << MemberDecl->getDeclName() << SourceRange(BuiltinLoc, RParenLoc);
return ExprError();
}
@@ -16765,11 +16770,11 @@ ExprResult Sema::BuildBuiltinOffsetOf(SourceLocation BuiltinLoc,
if (IndirectMemberDecl) {
for (auto *FI : IndirectMemberDecl->chain()) {
assert(isa<FieldDecl>(FI));
- Comps.push_back(OffsetOfNode(OC.LocStart,
- cast<FieldDecl>(FI), OC.LocEnd));
+ Comps.push_back(
+ OffsetOfNode(D.getBeginLoc(), cast<FieldDecl>(FI), D.getEndLoc()));
}
} else
- Comps.push_back(OffsetOfNode(OC.LocStart, MemberDecl, OC.LocEnd));
+ Comps.push_back(OffsetOfNode(D.getBeginLoc(), MemberDecl, D.getEndLoc()));
CurrentType = MemberDecl->getType().getNonReferenceType();
}
@@ -16778,11 +16783,10 @@ ExprResult Sema::BuildBuiltinOffsetOf(SourceLocation BuiltinLoc,
Comps, Exprs, RParenLoc);
}
-ExprResult Sema::ActOnBuiltinOffsetOf(Scope *S,
- SourceLocation BuiltinLoc,
+ExprResult Sema::ActOnBuiltinOffsetOf(Scope *S, SourceLocation BuiltinLoc,
SourceLocation TypeLoc,
ParsedType ParsedArgTy,
- ArrayRef<OffsetOfComponent> Components,
+ const Designation &Desig,
SourceLocation RParenLoc) {
TypeSourceInfo *ArgTInfo;
@@ -16793,10 +16797,9 @@ ExprResult Sema::ActOnBuiltinOffsetOf(Scope *S,
if (!ArgTInfo)
ArgTInfo = Context.getTrivialTypeSourceInfo(ArgTy, TypeLoc);
- return BuildBuiltinOffsetOf(BuiltinLoc, ArgTInfo, Components, RParenLoc);
+ return BuildBuiltinOffsetOf(BuiltinLoc, ArgTInfo, Desig, RParenLoc);
}
-
ExprResult Sema::ActOnChooseExpr(SourceLocation BuiltinLoc,
Expr *CondExpr,
Expr *LHSExpr, Expr *RHSExpr,
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 9c0479fe8aa4f..ed1237f3483a7 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -2827,11 +2827,9 @@ class TreeTransform {
/// By default, performs semantic analysis to build the new expression.
/// Subclasses may override this routine to provide different behavior.
ExprResult RebuildOffsetOfExpr(SourceLocation OperatorLoc,
- TypeSourceInfo *Type,
- ArrayRef<Sema::OffsetOfComponent> Components,
+ TypeSourceInfo *Type, const Designation &Desig,
SourceLocation RParenLoc) {
- return getSema().BuildBuiltinOffsetOf(OperatorLoc, Type, Components,
- RParenLoc);
+ return getSema().BuildBuiltinOffsetOf(OperatorLoc, Type, Desig, RParenLoc);
}
/// Build a new sizeof, alignof or vec_step expression with a
@@ -13385,21 +13383,16 @@ TreeTransform<Derived>::TransformOffsetOfExpr(OffsetOfExpr *E) {
if (!Type)
return ExprError();
- // Transform all of the components into components similar to what the
- // parser uses.
+ // Transform all of the components into a Designation similar to what the
+ // parser builds.
// FIXME: It would be slightly more efficient in the non-dependent case to
// just map FieldDecls, rather than requiring the rebuilder to look for
// the fields again. However, __builtin_offsetof is rare enough in
// template code that we don't care.
bool ExprChanged = false;
- typedef Sema::OffsetOfComponent Component;
- SmallVector<Component, 4> Components;
+ Designation Desig;
for (unsigned I = 0, N = E->getNumComponents(); I != N; ++I) {
const OffsetOfNode &ON = E->getComponent(I);
- Component Comp;
- Comp.isBrackets = true;
- Comp.LocStart = ON.getSourceRange().getBegin();
- Comp.LocEnd = ON.getSourceRange().getEnd();
switch (ON.getKind()) {
case OffsetOfNode::Array: {
Expr *FromIndex = E->getIndexExpr(ON.getArrayExprIndex());
@@ -13408,26 +13401,30 @@ TreeTransform<Derived>::TransformOffsetOfExpr(OffsetOfExpr *E) {
return ExprError();
ExprChanged = ExprChanged || Index.get() != FromIndex;
- Comp.isBrackets = true;
- Comp.U.E = Index.get();
+ Designator AD =
+ Designator::CreateArrayDesignator(Index.get(), ON.getBeginLoc());
+ AD.setRBracketLoc(ON.getEndLoc());
+ Desig.AddDesignator(AD);
break;
}
case OffsetOfNode::Field:
- case OffsetOfNode::Identifier:
- Comp.isBrackets = false;
- Comp.U.IdentInfo = ON.getFieldName();
- if (!Comp.U.IdentInfo)
+ case OffsetOfNode::Identifier: {
+ const IdentifierInfo *Name = ON.getFieldName();
+ if (!Name)
continue;
-
+ // The leading designator has no '.'; subsequent ones do.
+ SourceLocation DotLoc =
+ Desig.empty() ? SourceLocation() : ON.getBeginLoc();
+ Desig.AddDesignator(
+ Designator::CreateFieldDesignator(Name, DotLoc, ON.getEndLoc()));
break;
+ }
case OffsetOfNode::Base:
// Will be recomputed during the rebuild.
continue;
}
-
- Components.push_back(Comp);
}
// If nothing changed, retain the existing expression.
@@ -13437,8 +13434,8 @@ TreeTransform<Derived>::TransformOffsetOfExpr(OffsetOfExpr *E) {
return E;
// Build a new offsetof expression.
- return getDerived().RebuildOffsetOfExpr(E->getOperatorLoc(), Type,
- Components, E->getRParenLoc());
+ return getDerived().RebuildOffsetOfExpr(E->getOperatorLoc(), Type, Desig,
+ E->getRParenLoc());
}
template<typename Derived>
More information about the cfe-commits
mailing list