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 <[email protected]> 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> _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
