https://github.com/yronglin updated https://github.com/llvm/llvm-project/pull/219288
>From cd9fbd40edaaddd8d2c71060625b34ca06b4fd7d Mon Sep 17 00:00:00 2001 From: yronglin <[email protected]> Date: Thu, 27 Aug 2026 12:44:02 -0700 Subject: [PATCH 1/3] [clang][Sema] Separate aggregate default member initializer evaluation A default member initializer used by a constructor is a separate full-expression, while one used during aggregate initialization belongs to the full-expression containing the aggregate initialization. Split the two building paths so aggregate initialization rebuilds the initializer in the surrounding evaluation context. Signed-off-by: yronglin <[email protected]> --- clang/docs/ReleaseNotes.md | 4 + clang/include/clang/AST/ParentMap.h | 31 +++ clang/include/clang/Sema/Sema.h | 23 +- clang/lib/AST/ByteCode/Compiler.cpp | 64 +++++- clang/lib/AST/ParentMap.cpp | 17 ++ clang/lib/Analysis/CFG.cpp | 55 ++++- clang/lib/Analysis/ReachableCode.cpp | 67 ++---- clang/lib/Sema/SemaDeclCXX.cpp | 10 +- clang/lib/Sema/SemaExpr.cpp | 210 +++++++++++------- clang/lib/Sema/SemaInit.cpp | 28 +-- clang/lib/Sema/TreeTransform.h | 2 +- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 57 +++-- clang/test/AST/ByteCode/records.cpp | 23 +- clang/test/AST/ast-dump-default-init.cpp | 67 +++--- clang/test/AST/ast-dump-recovery.cpp | 2 +- .../Analysis/lifetime-extended-regions.cpp | 7 +- .../aggregate-default-member-initializers.cpp | 42 ++++ .../aggregate-default-member-initializers.cpp | 103 +++++++++ clang/test/SemaCXX/cxx2c-placeholder-vars.cpp | 8 +- clang/test/SemaCXX/warn-unreachable.cpp | 104 +++++++++ .../UncheckedOptionalAccessModelTest.cpp | 18 ++ 21 files changed, 708 insertions(+), 234 deletions(-) create mode 100644 clang/test/CodeGenCXX/aggregate-default-member-initializers.cpp create mode 100644 clang/test/SemaCXX/aggregate-default-member-initializers.cpp diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 35aae605d8476..50cfa0e75188b 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -468,6 +468,10 @@ features cannot lower the translation-unit ABI level; #### Bug Fixes to C++ Support +- Fixed the destruction timing of temporaries created by default member + initializers during aggregate initialization. Such an initializer is part of + the full-expression containing the aggregate initialization. (#GH85601) + - Fixed an issue where `__typeof__` incorrectly rejected cv-qualified function types. - Fixed a bug where top-level CV qualifiers (such as ``const``) were dropped from pointers modified by Microsoft pointer attributes (like ``__ptr32`` and ``__ptr64``) and WebAssembly's ``__funcref``. diff --git a/clang/include/clang/AST/ParentMap.h b/clang/include/clang/AST/ParentMap.h index 86e2f048a3445..5853a0be0a483 100644 --- a/clang/include/clang/AST/ParentMap.h +++ b/clang/include/clang/AST/ParentMap.h @@ -13,6 +13,8 @@ #ifndef LLVM_CLANG_AST_PARENTMAP_H #define LLVM_CLANG_AST_PARENTMAP_H +#include "llvm/Support/Casting.h" + namespace clang { class Stmt; class Expr; @@ -39,6 +41,25 @@ class ParentMap { Stmt *getParentIgnoreParenImpCasts(Stmt *) const; Stmt *getOuterParenParent(Stmt *) const; + template <typename... Ts> Stmt *getOuterMostAncestor(Stmt *S) const { + Stmt *Res = nullptr; + while (S) { + if (llvm::isa<Ts...>(S)) + Res = S; + S = getParent(S); + } + return Res; + } + + template <typename... Ts> Stmt *getInnerMostAncestor(Stmt *S) const { + while (S) { + if (llvm::isa<Ts...>(S)) + return S; + S = getParent(S); + } + return nullptr; + } + const Stmt *getParent(const Stmt* S) const { return getParent(const_cast<Stmt*>(S)); } @@ -51,6 +72,16 @@ class ParentMap { return getParentIgnoreParenCasts(const_cast<Stmt*>(S)); } + template <typename... Ts> + const Stmt *getOuterMostAncestor(const Stmt *S) const { + return getOuterMostAncestor<Ts...>(const_cast<Stmt *>(S)); + } + + template <typename... Ts> + const Stmt *getInnerMostAncestor(const Stmt *S) const { + return getInnerMostAncestor<Ts...>(const_cast<Stmt *>(S)); + } + bool hasParent(const Stmt *S) const { return getParent(S) != nullptr; } bool isConsumedExpr(Expr *E) const; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index dcf112fd8eaa4..10a764a403e37 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5543,6 +5543,10 @@ class Sema final : public SemaBase { ExprResult ConvertMemberDefaultInitExpression(FieldDecl *FD, Expr *InitExpr, SourceLocation InitLoc); + ExprResult ConvertMemberDefaultInitExpression(FieldDecl *FD, + const InitializedEntity &Entity, + Expr *InitExpr, + SourceLocation InitLoc); /// FinalizeVarWithDestructor - Prepare for calling destructor on the /// constructed variable. @@ -7712,7 +7716,24 @@ class Sema final : public SemaBase { /// Emit a warning for all pending noderef expressions that we recorded. void WarnOnPendingNoDerefs(ExpressionEvaluationContextRecord &Rec); - ExprResult BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field); +private: + /// Shared logic for building default member initializer which used in a + /// constructor or an aggregate initialization. + /// + /// + /// The caller enters that evaluation context and decides whether the result + /// is finished as a full-expression. \p NestedDefaultChecking and + /// \p NeedRebuild have to be sampled before entering it. + ExprResult BuildCXXDefaultInitInternal(SourceLocation Loc, FieldDecl *Field, + const InitializedEntity &Entity, + bool NestedDefaultChecking, + bool NeedRebuild); + +public: + ExprResult BuildCXXCtorDefaultInitExpr(SourceLocation Loc, FieldDecl *Field); + ExprResult + BuildCXXAggregateDefaultInitExpr(SourceLocation Loc, FieldDecl *Field, + const InitializedEntity &MemberEntity); /// Instantiate or parse a C++ default argument expression as necessary. /// Return true on error. diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index c182639ea07f8..6b02a57a0deda 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -50,6 +50,22 @@ static bool isSideEffectFree(const Expr *E) { return false; } +static bool containsDefaultInitExpr(const Expr *E) { + class Finder final : public ConstDynamicRecursiveASTVisitor { + public: + Finder() { ShouldVisitImplicitCode = true; } + + bool VisitCXXDefaultInitExpr(const CXXDefaultInitExpr *) override { + Found = true; + return true; + } + + bool Found = false; + } F; + F.TraverseStmt(E); + return F.Found; +} + /// Scope chain managing the variable lifetimes. template <class Emitter> class VariableScope { public: @@ -265,7 +281,9 @@ template <class Emitter> class InitStackScope final { public: InitStackScope(Compiler<Emitter> *Ctx, bool Active) : Ctx(Ctx), OldValue(Ctx->InitStackActive), Active(Active) { - Ctx->InitStackActive = Active; + // An explicit initializer nested in a default member initializer still + // needs the surrounding default initializer's `this` reconstruction. + Ctx->InitStackActive = OldValue || Active; if (Active) Ctx->InitStack.push_back(InitLink::DIE()); } @@ -3475,6 +3493,18 @@ bool Compiler<Emitter>::VisitExprWithCleanups(const ExprWithCleanups *E) { LocalScope<Emitter> ES(this, ScopeKind::FullExpression); const Expr *SubExpr = E->getSubExpr(); + if (DiscardResult && !SubExpr->isGLValue() && + !canClassify(SubExpr->getType()) && containsDefaultInitExpr(SubExpr)) { + UnsignedOrNone LocalIndex = + allocateLocal(SubExpr, QualType(), ScopeKind::FullExpression); + if (!LocalIndex) + return false; + InitLinkScope<Emitter> ILS(this, InitLink::Temp(*LocalIndex)); + if (!this->emitGetPtrLocal(*LocalIndex, E)) + return false; + return this->visitInitializerPop(SubExpr) && ES.destroyLocals(E); + } + return this->delegate(SubExpr) && ES.destroyLocals(E); } @@ -3541,8 +3571,12 @@ bool Compiler<Emitter>::VisitMaterializeTemporaryExpr( // Non-primitive values. if (!this->emitGetPtrGlobal(*GlobalIndex, E)) return false; + if (!this->emitStartInit(E)) + return false; if (!this->visitInitializer(Inner)) return false; + if (!this->emitEndInit(E)) + return false; if (IsStatic) { assert(TempDecl); return this->emitInitGlobalTempComp(TempDecl, E); @@ -3585,7 +3619,11 @@ bool Compiler<Emitter>::VisitMaterializeTemporaryExpr( if (!this->emitGetPtrLocal(*LocalIndex, E)) return false; - return this->visitInitializer(Inner); + if (!this->emitStartInit(E)) + return false; + if (!this->visitInitializer(Inner)) + return false; + return this->emitEndInit(E); } return false; } @@ -4888,6 +4926,22 @@ bool Compiler<Emitter>::VisitStmtExpr(const StmtExpr *E) { } template <class Emitter> bool Compiler<Emitter>::discard(const Expr *E) { + // A discarded composite prvalue still needs a result object when a default + // member initializer refers to previously initialized subobjects. Let an + // ExprWithCleanups establish its full-expression scope before allocating + // that object. + if (!isa<ExprWithCleanups>(E) && !E->isGLValue() && + !canClassify(E->getType()) && containsDefaultInitExpr(E)) { + UnsignedOrNone LocalIndex = + allocateLocal(E, QualType(), ScopeKind::FullExpression); + if (!LocalIndex) + return false; + InitLinkScope<Emitter> ILS(this, InitLink::Temp(*LocalIndex)); + if (!this->emitGetPtrLocal(*LocalIndex, E)) + return false; + return this->visitInitializerPop(E); + } + OptionScope<Emitter> Scope(this, /*NewDiscardResult=*/true, /*NewInitializing=*/false, /*ToLValue=*/false); return this->Visit(E); @@ -5451,8 +5505,14 @@ bool Compiler<Emitter>::visitExpr(const Expr *E, bool DestroyToplevelScope) { if (!this->emitGetPtrLocal(*LocalOffset, E)) return false; + // A const-qualified result object is writable while it is being + // initialized, just like an object evaluated through visitVarDecl(). + if (!this->emitStartInit(E)) + return false; if (!visitInitializer(E)) return false; + if (!this->emitEndInit(E)) + return false; // We are destroying the locals AFTER the Ret op. // The Ret op needs to copy the (alive) values, but the // destructors may still turn the entire expression invalid. diff --git a/clang/lib/AST/ParentMap.cpp b/clang/lib/AST/ParentMap.cpp index e62e71bf5a514..580613b2618fb 100644 --- a/clang/lib/AST/ParentMap.cpp +++ b/clang/lib/AST/ParentMap.cpp @@ -13,6 +13,7 @@ #include "clang/AST/ParentMap.h" #include "clang/AST/Decl.h" #include "clang/AST/Expr.h" +#include "clang/AST/ExprCXX.h" #include "clang/AST/StmtObjC.h" #include "llvm/ADT/DenseMap.h" @@ -103,6 +104,22 @@ static void BuildParentMap(MapTy& M, Stmt* S, BuildParentMap(M, SubStmt, OVMode); } break; + case Stmt::CXXDefaultArgExprClass: + if (auto *Arg = dyn_cast<CXXDefaultArgExpr>(S)) { + if (Arg->hasRewrittenInit()) { + M[Arg->getExpr()] = S; + BuildParentMap(M, Arg->getExpr(), OVMode); + } + } + break; + case Stmt::CXXDefaultInitExprClass: + if (auto *Init = dyn_cast<CXXDefaultInitExpr>(S)) { + if (Init->hasRewrittenInit()) { + M[Init->getExpr()] = S; + BuildParentMap(M, Init->getExpr(), OVMode); + } + } + break; default: for (Stmt *SubStmt : S->children()) { if (SubStmt) { diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index 5263114ebca28..f92e6f3dcbf47 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -581,6 +581,10 @@ class CFGBuilder { private: // Visitors to walk an AST and construct the CFG. + CFGBlock *VisitCXXDefaultArgExpr(CXXDefaultArgExpr *Default, + AddStmtChoice asc); + CFGBlock *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *Default, + AddStmtChoice asc); CFGBlock *VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc); CFGBlock *VisitAddrLabelExpr(AddrLabelExpr *A, AddStmtChoice asc); CFGBlock *VisitAttributedStmt(AttributedStmt *A, AddStmtChoice asc); @@ -2405,16 +2409,10 @@ CFGBlock *CFGBuilder::Visit(Stmt * S, AddStmtChoice asc, asc, ExternallyDestructed); case Stmt::CXXDefaultArgExprClass: + return VisitCXXDefaultArgExpr(cast<CXXDefaultArgExpr>(S), asc); + case Stmt::CXXDefaultInitExprClass: - // FIXME: The expression inside a CXXDefaultArgExpr is owned by the - // called function's declaration, not by the caller. If we simply add - // this expression to the CFG, we could end up with the same Expr - // appearing multiple times (PR13385). - // - // It's likewise possible for multiple CXXDefaultInitExprs for the same - // expression to be used in the same function (through aggregate - // initialization). - return VisitStmt(S, asc); + return VisitCXXDefaultInitExpr(cast<CXXDefaultInitExpr>(S), asc); case Stmt::CXXBindTemporaryExprClass: return VisitCXXBindTemporaryExpr(cast<CXXBindTemporaryExpr>(S), asc); @@ -2597,6 +2595,45 @@ CFGBlock *CFGBuilder::VisitCallExprChildren(CallExpr *C) { return VisitChildren(C); } +CFGBlock *CFGBuilder::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *Arg, + AddStmtChoice asc) { + if (Arg->hasRewrittenInit()) { + if (asc.alwaysAdd(*this, Arg)) { + autoCreateBlock(); + appendStmt(Block, Arg); + } + return VisitStmt(Arg->getExpr()->IgnoreParens(), asc); + } + + // We can't add the default argument if it's not rewritten because the + // expression inside a CXXDefaultArgExpr is owned by the called function's + // declaration, not by the caller. We could end up with the same expression + // appearing multiple times. + return VisitStmt(Arg, asc); +} + +CFGBlock *CFGBuilder::VisitCXXDefaultInitExpr(CXXDefaultInitExpr *Init, + AddStmtChoice asc) { + if (Init->hasRewrittenInit()) { + if (asc.alwaysAdd(*this, Init)) { + autoCreateBlock(); + appendStmt(Block, Init); + } + + // Unlike CXXDefaultArgExpr::getExpr, which strips off the top-level + // FullExpr and ConstantExpr, CXXDefaultInitExpr::getExpr does not do this, + // so the top level cannot be a ParenExpr. Use Visit rather than VisitStmt + // so that control flow inside the initializer (a conditional operator, for + // instance) is decomposed into blocks instead of being laid out linearly. + return Visit(Init->getExpr(), asc); + } + + // We can't add the default initializer if it's not rewritten because multiple + // CXXDefaultInitExprs can refer to the same subexpression in the same + // function (through aggregate initialization). + return VisitStmt(Init, asc); +} + CFGBlock *CFGBuilder::VisitInitListExpr(InitListExpr *ILE, AddStmtChoice asc) { if (asc.alwaysAdd(*this, ILE)) { autoCreateBlock(); diff --git a/clang/lib/Analysis/ReachableCode.cpp b/clang/lib/Analysis/ReachableCode.cpp index 4a9ab5d9f0f73..7d17a0cb4cfd4 100644 --- a/clang/lib/Analysis/ReachableCode.cpp +++ b/clang/lib/Analysis/ReachableCode.cpp @@ -25,6 +25,7 @@ #include "clang/Basic/SourceManager.h" #include "clang/Lex/Preprocessor.h" #include "llvm/ADT/BitVector.h" +#include <cstddef> #include <optional> using namespace clang; @@ -396,6 +397,7 @@ namespace { SmallVector<const CFGBlock *, 10> WorkList; Preprocessor &PP; ASTContext &C; + AnalysisDeclContext &AC; typedef SmallVector<std::pair<const CFGBlock *, const Stmt *>, 12> DeferredLocsTy; @@ -403,10 +405,10 @@ namespace { DeferredLocsTy DeferredLocs; public: - DeadCodeScan(llvm::BitVector &reachable, Preprocessor &PP, ASTContext &C) - : Visited(reachable.size()), - Reachable(reachable), - PP(PP), C(C) {} + DeadCodeScan(llvm::BitVector &reachable, Preprocessor &PP, + AnalysisDeclContext &AC) + : Visited(reachable.size()), Reachable(reachable), PP(PP), + C(AC.getASTContext()), AC(AC) {} void enqueue(const CFGBlock *block); unsigned scanBackwards(const CFGBlock *Start, @@ -453,47 +455,8 @@ bool DeadCodeScan::isDeadCodeRoot(const clang::CFGBlock *Block) { return isDeadRoot; } -// Check if the given `DeadStmt` is a coroutine statement and is a substmt of -// the coroutine statement. `Block` is the CFGBlock containing the `DeadStmt`. -static bool isInCoroutineStmt(const Stmt *DeadStmt, const CFGBlock *Block) { - // The coroutine statement, co_return, co_await, or co_yield. - const Stmt *CoroStmt = nullptr; - // Find the first coroutine statement after the DeadStmt in the block. - bool AfterDeadStmt = false; - for (const CFGElement &Elem : *Block) - if (std::optional<CFGStmt> CS = Elem.getAs<CFGStmt>()) { - const Stmt *S = CS->getStmt(); - if (S == DeadStmt) - AfterDeadStmt = true; - if (AfterDeadStmt && - // For simplicity, we only check simple coroutine statements. - (llvm::isa<CoreturnStmt>(S) || llvm::isa<CoroutineSuspendExpr>(S))) { - CoroStmt = S; - break; - } - } - if (!CoroStmt) - return false; - struct Checker : DynamicRecursiveASTVisitor { - const Stmt *DeadStmt; - bool CoroutineSubStmt = false; - Checker(const Stmt *S) : DeadStmt(S) { - // Statements captured in the CFG can be implicit. - ShouldVisitImplicitCode = true; - } - - bool VisitStmt(Stmt *S) override { - if (S == DeadStmt) - CoroutineSubStmt = true; - return true; - } - }; - Checker checker(DeadStmt); - checker.TraverseStmt(const_cast<Stmt *>(CoroStmt)); - return checker.CoroutineSubStmt; -} - -static bool isValidDeadStmt(const Stmt *S, const clang::CFGBlock *Block) { +static bool isValidDeadStmt(ParentMap &PM, const Stmt *S, + const clang::CFGBlock *) { if (S->getBeginLoc().isInvalid()) return false; if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(S)) @@ -501,21 +464,27 @@ static bool isValidDeadStmt(const Stmt *S, const clang::CFGBlock *Block) { // Coroutine statements are never considered dead statements, because removing // them may change the function semantic if it is the only coroutine statement // of the coroutine. - return !isInCoroutineStmt(S, Block); + return !PM.getInnerMostAncestor<CoreturnStmt, CoroutineSuspendExpr>(S); } const Stmt *DeadCodeScan::findDeadCode(const clang::CFGBlock *Block) { + auto &PM = AC.getParentMap(); + for (CFGBlock::const_iterator I = Block->begin(), E = Block->end(); I!=E; ++I) if (std::optional<CFGStmt> CS = I->getAs<CFGStmt>()) { const Stmt *S = CS->getStmt(); - if (isValidDeadStmt(S, Block)) + auto *RewrittenParent = + PM.getOuterMostAncestor<CXXDefaultArgExpr, CXXDefaultInitExpr>(S); + if (RewrittenParent) + S = RewrittenParent; + if (isValidDeadStmt(AC.getParentMap(), S, Block)) return S; } CFGTerminator T = Block->getTerminator(); if (T.isStmtBranch()) { const Stmt *S = T.getStmt(); - if (S && isValidDeadStmt(S, Block)) + if (S && isValidDeadStmt(AC.getParentMap(), S, Block)) return S; } @@ -761,7 +730,7 @@ void FindUnreachableCode(AnalysisDeclContext &AC, Preprocessor &PP, if (reachable[block->getBlockID()]) continue; - DeadCodeScan DS(reachable, PP, AC.getASTContext()); + DeadCodeScan DS(reachable, PP, AC); numReachable += DS.scanBackwards(block, CB); if (numReachable == cfg->getNumBlockIDs()) diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 4e90c496de342..f03d615bdce5b 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -4242,6 +4242,12 @@ ExprResult Sema::ConvertMemberDefaultInitExpression(FieldDecl *FD, SourceLocation InitLoc) { InitializedEntity Entity = InitializedEntity::InitializeMemberFromDefaultMemberInitializer(FD); + return ConvertMemberDefaultInitExpression(FD, Entity, InitExpr, InitLoc); +} + +ExprResult Sema::ConvertMemberDefaultInitExpression( + FieldDecl *FD, const InitializedEntity &Entity, Expr *InitExpr, + SourceLocation InitLoc) { InitializationKind Kind = FD->getInClassInitStyle() == ICIS_ListInit ? InitializationKind::CreateDirectList(InitExpr->getBeginLoc(), @@ -5342,7 +5348,7 @@ static bool CollectFieldInitializer(Sema &SemaRef, BaseAndFieldInfo &Info, if (Field->hasInClassInitializer() && !Info.isImplicitCopyOrMove()) { ExprResult DIE = - SemaRef.BuildCXXDefaultInitExpr(Info.Ctor->getLocation(), Field); + SemaRef.BuildCXXCtorDefaultInitExpr(Info.Ctor->getLocation(), Field); if (DIE.isInvalid()) return true; @@ -14129,7 +14135,7 @@ bool SpecialMemberExceptionSpecInfo::visitField(FieldDecl *FD) { // FIXME: We should have a single context note pointing at Loc, and // this location should be MD->getLocation() instead, since that's // the location where we actually use the default init expression. - E = S.BuildCXXDefaultInitExpr(Loc, FD).get(); + E = S.BuildCXXCtorDefaultInitExpr(Loc, FD).get(); if (E) ExceptSpec.CalledExpr(E); } else if (auto *RD = S.Context.getBaseElementType(FD->getType()) diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 2b524a956ecc4..31b308ad4b52b 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -5876,39 +5876,51 @@ static FieldDecl *FindFieldDeclInstantiationPattern(const ASTContext &Ctx, return cast<FieldDecl>(*Rng.begin()); } -ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) { - assert(Field->hasInClassInitializer()); - - CXXThisScopeRAII This(*this, Field->getParent(), Qualifiers()); - +ExprResult Sema::BuildCXXDefaultInitInternal(SourceLocation Loc, + FieldDecl *Field, + const InitializedEntity &Entity, + bool NestedDefaultChecking, + bool NeedRebuild) { auto *ParentRD = cast<CXXRecordDecl>(Field->getParent()); - std::optional<ExpressionEvaluationContextRecord::InitializationContext> - InitializationContext = - OutermostDeclarationWithDelayedImmediateInvocations(); - if (!InitializationContext.has_value()) - InitializationContext.emplace(Loc, Field, CurContext); - - Expr *Init = nullptr; - - bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer(); - bool NeedRebuild = needsRebuildOfDefaultArgOrInit(); - EnterExpressionEvaluationContext EvalContext( - *this, ExpressionEvaluationContext::PotentiallyEvaluated, Field); - - if (!Field->getInClassInitializer()) { + if (!Field->getInClassInitializer() && + isTemplateInstantiation(ParentRD->getTemplateSpecializationKind())) { // Maybe we haven't instantiated the in-class initializer. Go check the // pattern FieldDecl to see if it has one. - if (isTemplateInstantiation(ParentRD->getTemplateSpecializationKind())) { - FieldDecl *Pattern = - FindFieldDeclInstantiationPattern(getASTContext(), Field); - assert(Pattern && "We must have set the Pattern!"); - if (!Pattern->hasInClassInitializer() || - InstantiateInClassInitializer(Loc, Field, Pattern, - getTemplateInstantiationArgs(Field))) { - return ExprError(); - } - } + FieldDecl *Pattern = + FindFieldDeclInstantiationPattern(getASTContext(), Field); + assert(Pattern && "We must have set the Pattern!"); + if (!Pattern->hasInClassInitializer() || + InstantiateInClassInitializer(Loc, Field, Pattern, + getTemplateInstantiationArgs(Field))) + return ExprError(); + } + + Expr *InClassInit = Field->getInClassInitializer(); + if (!InClassInit) { + // DR1351: + // If the brace-or-equal-initializer of a non-static data member + // invokes a defaulted default constructor of its class or of an + // enclosing class in a potentially evaluated subexpression, the + // program is ill-formed. + // + // This resolution is unworkable: the exception specification of the + // default constructor can be needed in an unevaluated context, in + // particular, in the operand of a noexcept-expression, and we can be + // unable to compute an exception specification for an enclosed class. + // + // Any attempt to resolve the exception specification of a defaulted default + // constructor before the initializer is lexically complete will ultimately + // come here at which point we can diagnose it. + RecordDecl *OutermostClass = ParentRD->getOuterLexicalRecordContext(); + Diag(Loc, diag::err_default_member_initializer_not_yet_parsed) + << OutermostClass << Field; + Diag(Field->getEndLoc(), + diag::note_default_member_initializer_not_yet_parsed); + // Recover by marking the field invalid, unless we're in a SFINAE context. + if (!isSFINAEContext()) + Field->setInvalidDecl(); + return ExprError(); } // CWG2631 @@ -5927,27 +5939,27 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) { // expression is an ExprWithCleanups. Then make sure the normal lifetime // extension code recurses into the default initializer and does lifetime // extension when warranted. - bool ContainsAnyTemporaries = - isa_and_present<ExprWithCleanups>(Field->getInClassInitializer()); - if (Field->getInClassInitializer() && - !Field->getInClassInitializer()->containsErrors() && + bool ContainsAnyTemporaries = isa<ExprWithCleanups>(InClassInit); + Expr *Init = InClassInit; + if (!InClassInit->containsErrors() && (V.HasImmediateCalls || (NeedRebuild && ContainsAnyTemporaries))) { ExprEvalContexts.back().DelayedDefaultInitializationContext = {Loc, Field, CurContext}; ExprEvalContexts.back().IsCurrentlyCheckingDefaultArgumentOrInitializer = NestedDefaultChecking; // Pass down lifetime extending flag, and collect temporaries in - // CreateMaterializeTemporaryExpr when we rewrite the call argument. + // CreateMaterializeTemporaryExpr when we rewrite the initializer. currentEvaluationContext().InLifetimeExtendingContext = parentEvaluationContext().InLifetimeExtendingContext; + EnsureImmediateInvocationInDefaultArgs Immediate(*this); ExprResult Res; runWithSufficientStackSpace(Loc, [&] { - Res = Immediate.TransformInitializer(Field->getInClassInitializer(), + Res = Immediate.TransformInitializer(InClassInit, /*CXXDirectInit=*/false); }); if (!Res.isInvalid()) - Res = ConvertMemberDefaultInitExpression(Field, Res.get(), Loc); + Res = ConvertMemberDefaultInitExpression(Field, Entity, Res.get(), Loc); if (Res.isInvalid()) { Field->setInvalidDecl(); return ExprError(); @@ -5955,52 +5967,92 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) { Init = Res.get(); } - if (Field->getInClassInitializer()) { - Expr *E = Init ? Init : Field->getInClassInitializer(); - if (!NestedDefaultChecking) - runWithSufficientStackSpace(Loc, [&] { - MarkDeclarationsReferencedInExpr(E, /*SkipLocalVariables=*/false); - }); - if (isInLifetimeExtendingContext()) - DiscardCleanupsInEvaluationContext(); - // C++11 [class.base.init]p7: - // The initialization of each base and member constitutes a - // full-expression. - ExprResult Res = ActOnFinishFullExpr(E, /*DiscardedValue=*/false); - if (Res.isInvalid()) { - Field->setInvalidDecl(); - return ExprError(); - } - Init = Res.get(); + if (!NestedDefaultChecking) + runWithSufficientStackSpace(Loc, [&] { + MarkDeclarationsReferencedInExpr(Init, /*SkipLocalVariables=*/false); + }); + return Init; +} - return CXXDefaultInitExpr::Create(Context, InitializationContext->Loc, - Field, InitializationContext->Context, - Init); - } +ExprResult Sema::BuildCXXCtorDefaultInitExpr(SourceLocation Loc, + FieldDecl *Field) { + assert(Field->hasInClassInitializer()); - // DR1351: - // If the brace-or-equal-initializer of a non-static data member - // invokes a defaulted default constructor of its class or of an - // enclosing class in a potentially evaluated subexpression, the - // program is ill-formed. - // - // This resolution is unworkable: the exception specification of the - // default constructor can be needed in an unevaluated context, in - // particular, in the operand of a noexcept-expression, and we can be - // unable to compute an exception specification for an enclosed class. - // - // Any attempt to resolve the exception specification of a defaulted default - // constructor before the initializer is lexically complete will ultimately - // come here at which point we can diagnose it. - RecordDecl *OutermostClass = ParentRD->getOuterLexicalRecordContext(); - Diag(Loc, diag::err_default_member_initializer_not_yet_parsed) - << OutermostClass << Field; - Diag(Field->getEndLoc(), - diag::note_default_member_initializer_not_yet_parsed); - // Recover by marking the field invalid, unless we're in a SFINAE context. - if (!isSFINAEContext()) + bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer(); + bool NeedRebuild = needsRebuildOfDefaultArgOrInit(); + + // C++11 [class.base.init]p7: + // The initialization of each base and member constitutes a + // full-expression. + // So this initializer gets an evaluation context of its own, and is finished + // as a full-expression below. + EnterExpressionEvaluationContext EvalContext( + *this, ExpressionEvaluationContext::PotentiallyEvaluated, Field); + CXXThisScopeRAII This(*this, Field->getParent(), Qualifiers()); + + auto InitContext = OutermostDeclarationWithDelayedImmediateInvocations(); + if (!InitContext) + InitContext.emplace(Loc, Field, CurContext); + + ExprResult Init = BuildCXXDefaultInitInternal( + Loc, Field, + InitializedEntity::InitializeMemberFromDefaultMemberInitializer(Field), + NestedDefaultChecking, NeedRebuild); + if (Init.isInvalid()) + return ExprError(); + + if (isInLifetimeExtendingContext()) + DiscardCleanupsInEvaluationContext(); + Init = ActOnFinishFullExpr(Init.get(), /*DiscardedValue=*/false); + if (Init.isInvalid()) { Field->setInvalidDecl(); - return ExprError(); + return ExprError(); + } + + return CXXDefaultInitExpr::Create( + Context, InitContext->Loc, Field, InitContext->Context, + Init.get() == Field->getInClassInitializer() ? nullptr : Init.get()); +} + +ExprResult +Sema::BuildCXXAggregateDefaultInitExpr(SourceLocation Loc, FieldDecl *Field, + const InitializedEntity &MemberEntity) { + assert(Field->hasInClassInitializer()); + + bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer(); + + // Unlike a mem-initializer, this initializer is a subexpression of the + // full-expression containing the aggregate initialization. It is evaluated + // exactly as that full-expression is, so inherit the enclosing context kind + // rather than forcing a potentially evaluated one. + EnterExpressionEvaluationContext EvalContext( + *this, currentEvaluationContext().Context, Field); + CXXThisScopeRAII This(*this, Field->getParent(), Qualifiers()); + + auto InitContext = OutermostDeclarationWithDelayedImmediateInvocations(); + if (!InitContext) + InitContext.emplace(Loc, Field, CurContext); + + // CWG1815: always rebuild, never share the AST built when the field was + // declared. Only a copy rebuilt here has its MaterializeTemporaryExprs + // collected in this context, which is what lets the aggregate initialization + // lifetime-extend them; sharing one AST would also make several uses of the + // same field fight over its extension. A mem-initializer has no such need, + // as its temporaries die at the end of the initializer itself. + ExprResult Init = BuildCXXDefaultInitInternal( + Loc, Field, MemberEntity, NestedDefaultChecking, /*NeedRebuild=*/true); + if (Init.isInvalid()) + return ExprError(); + + // Deliberately not finished as a full-expression: leaving the temporaries it + // created on ExprCleanupObjects lets PopExpressionEvaluationContext merge + // them into the enclosing context, which eventually wraps them all in a + // single ExprWithCleanups. They are then destroyed at the end of the + // containing full-expression, in reverse construction order. + + return CXXDefaultInitExpr::Create( + Context, InitContext->Loc, Field, InitContext->Context, + Init.get() == Field->getInClassInitializer() ? nullptr : Init.get()); } VariadicCallType Sema::getVariadicCallType(FunctionDecl *FDecl, diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 48ce51863c2c0..2b756b803e205 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -813,28 +813,15 @@ void InitListChecker::FillInEmptyInitForField(unsigned Init, FieldDecl *Field, if (VerifyOnly) return; - ExprResult DIE; - { - // Enter a default initializer rebuild context, then we can support - // lifetime extension of temporary created by aggregate initialization - // using a default member initializer. - // CWG1815 (https://wg21.link/CWG1815). - EnterExpressionEvaluationContext RebuildDefaultInit( - SemaRef, Sema::ExpressionEvaluationContext::PotentiallyEvaluated); - SemaRef.currentEvaluationContext().RebuildDefaultArgOrDefaultInit = - true; - SemaRef.currentEvaluationContext().DelayedDefaultInitializationContext = - SemaRef.parentEvaluationContext() - .DelayedDefaultInitializationContext; - SemaRef.currentEvaluationContext().InLifetimeExtendingContext = - SemaRef.parentEvaluationContext().InLifetimeExtendingContext; - DIE = SemaRef.BuildCXXDefaultInitExpr(Loc, Field); - } + // A default member initializer used in aggregate initialization is part + // of the full-expression containing the aggregate initialization. Do not + // create or finish a separate expression evaluation context here. + ExprResult DIE = + SemaRef.BuildCXXAggregateDefaultInitExpr(Loc, Field, MemberEntity); if (DIE.isInvalid()) { hadError = true; return; } - SemaRef.checkInitializerLifetime(MemberEntity, DIE.get()); if (Init < NumInits) ILE->setInit(Init, DIE.get()); else { @@ -6152,11 +6139,10 @@ static void TryOrBuildParenListInitialization( // C++ [dcl.init]p16.6.2.2 // The remaining elements are initialized with their default // member initializers, if any - ExprResult DIE = S.BuildCXXDefaultInitExpr( - Kind.getParenOrBraceRange().getEnd(), FD); + ExprResult DIE = S.BuildCXXAggregateDefaultInitExpr( + Kind.getParenOrBraceRange().getEnd(), FD, SubEntity); if (DIE.isInvalid()) return; - S.checkInitializerLifetime(SubEntity, DIE.get()); InitExprs.push_back(DIE.get()); } } else { diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 024f726b188b7..83c125cebde5d 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -3521,7 +3521,7 @@ class TreeTransform { /// routine to provide different behavior. ExprResult RebuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) { - return getSema().BuildCXXDefaultInitExpr(Loc, Field); + return getSema().BuildCXXCtorDefaultInitExpr(Loc, Field); } /// Build a new C++ zero-initialization expression. diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index 530fae9ee2dee..e2f30968e02ff 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -1958,32 +1958,47 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, ExplodedNodeSet Tmp; - const Expr *ArgE; - if (const auto *DefE = dyn_cast<CXXDefaultArgExpr>(S)) + bool HasRebuiltInit = false; + const Expr *ArgE = nullptr; + if (const auto *DefE = dyn_cast<CXXDefaultArgExpr>(S)) { ArgE = DefE->getExpr(); - else if (const auto *DefE = dyn_cast<CXXDefaultInitExpr>(S)) + HasRebuiltInit = DefE->hasRewrittenInit(); + } else if (const auto *DefE = dyn_cast<CXXDefaultInitExpr>(S)) { ArgE = DefE->getExpr(); - else + HasRebuiltInit = DefE->hasRewrittenInit(); + } else llvm_unreachable("unknown constant wrapper kind"); - bool IsTemporary = false; - if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(ArgE)) { - ArgE = MTE->getSubExpr(); - IsTemporary = true; - } + if (HasRebuiltInit) { + for (const auto N : PreVisit) { + const StackFrame *SF = N->getStackFrame(); + ProgramStateRef State = N->getState(); + State = State->BindExpr(cast<Expr>(S), SF, State->getSVal(ArgE, SF)); + Tmp.insert(Engine.makePostStmtNode(S, State, N)); + } + } else { + // If it's not rewritten, the contents of these expressions are not + // actually part of the current function, so we fall back to constant + // evaluation. + bool IsTemporary = false; + if (const auto *MTE = dyn_cast<MaterializeTemporaryExpr>(ArgE)) { + ArgE = MTE->getSubExpr(); + IsTemporary = true; + } - std::optional<SVal> ConstantVal = svalBuilder.getConstantVal(ArgE); - if (!ConstantVal) - ConstantVal = UnknownVal(); - - const StackFrame *SF = Pred->getStackFrame(); - for (const auto I : PreVisit) { - ProgramStateRef State = I->getState(); - State = State->BindExpr(cast<Expr>(S), SF, *ConstantVal); - if (IsTemporary) - State = createTemporaryRegionIfNeeded(State, SF, cast<Expr>(S), - cast<Expr>(S)); - Tmp.insert(Engine.makePostStmtNode(S, State, I)); + std::optional<SVal> ConstantVal = svalBuilder.getConstantVal(ArgE); + if (!ConstantVal) + ConstantVal = UnknownVal(); + + for (const auto I : PreVisit) { + const StackFrame *SF = I->getStackFrame(); + ProgramStateRef State = I->getState(); + State = State->BindExpr(cast<Expr>(S), SF, *ConstantVal); + if (IsTemporary) + State = createTemporaryRegionIfNeeded(State, SF, cast<Expr>(S), + cast<Expr>(S)); + Tmp.insert(Engine.makePostStmtNode(S, State, I)); + } } getCheckerManager().runCheckersForPostStmt(Dst, Tmp, S, *this); diff --git a/clang/test/AST/ByteCode/records.cpp b/clang/test/AST/ByteCode/records.cpp index 36b5cb62fe95f..f70c7f641120a 100644 --- a/clang/test/AST/ByteCode/records.cpp +++ b/clang/test/AST/ByteCode/records.cpp @@ -1302,11 +1302,30 @@ namespace { }; constexpr int a() { int x = 1; - int f = B{x}.x; + { + B b{x}; + } + return x; + } + static_assert(a() == 0); + + constexpr int discarded() { + int x = 1; B{x}; // both-warning {{expression result unused}} + return x; + } + static_assert(discarded() == 0); - return 1; + /// A const-qualified composite result is writable while under construction. + constexpr int decrement(int &x) { + return --x; } + struct DMIConstComposite { + int a; + int b = decrement(a); + }; + constexpr DMIConstComposite c{1}; + static_assert(c.a == 0); } #endif diff --git a/clang/test/AST/ast-dump-default-init.cpp b/clang/test/AST/ast-dump-default-init.cpp index 2c2d670486627..9f972bb3cc921 100644 --- a/clang/test/AST/ast-dump-default-init.cpp +++ b/clang/test/AST/ast-dump-default-init.cpp @@ -13,13 +13,12 @@ void test() { B b{}; } // CHECK: -CXXDefaultInitExpr 0x{{[^ ]*}} <{{.*}}> 'const A' lvalue has rewritten init -// CHECK-NEXT: `-ExprWithCleanups 0x{{[^ ]*}} <{{.*}}> 'const A' lvalue -// CHECK-NEXT: `-MaterializeTemporaryExpr 0x{{[^ ]*}} <{{.*}}> 'const A' lvalue extended by Var 0x{{[^ ]*}} 'b' 'B' -// CHECK-NEXT: `-ImplicitCastExpr 0x{{[^ ]*}} <{{.*}}> 'const A' <NoOp> -// CHECK-NEXT: `-CXXFunctionalCastExpr 0x{{[^ ]*}} <{{.*}}> 'A' functional cast to A <NoOp> -// CHECK-NEXT: `-InitListExpr 0x{{[^ ]*}} <{{.*}}> 'A' -// CHECK-NEXT: `-InitListExpr 0x{{[^ ]*}} <{{.*}}> 'int[1]' -// CHECK-NEXT: `-IntegerLiteral 0x{{[^ ]*}} <{{.*}}> 'int' 0 +// CHECK-NEXT: `-MaterializeTemporaryExpr 0x{{[^ ]*}} <{{.*}}> 'const A' lvalue extended by Var 0x{{[^ ]*}} 'b' 'B' +// CHECK-NEXT: `-ImplicitCastExpr 0x{{[^ ]*}} <{{.*}}> 'const A' <NoOp> +// CHECK-NEXT: `-CXXFunctionalCastExpr 0x{{[^ ]*}} <{{.*}}> 'A' functional cast to A <NoOp> +// CHECK-NEXT: `-InitListExpr 0x{{[^ ]*}} <{{.*}}> 'A' +// CHECK-NEXT: `-InitListExpr 0x{{[^ ]*}} <{{.*}}> 'int[1]' +// CHECK-NEXT: `-IntegerLiteral 0x{{[^ ]*}} <{{.*}}> 'int' 0 // JSON: "kind": "CXXDefaultInitExpr", // JSON: "type": { @@ -30,73 +29,65 @@ void test() { // JSON-NEXT: "inner": [ // JSON-NEXT: { // JSON-NEXT: "id": "0x{{.*}}", -// JSON-NEXT: "kind": "ExprWithCleanups", +// JSON-NEXT: "kind": "MaterializeTemporaryExpr", // JSON: "type": { // JSON-NEXT: "qualType": "const A" // JSON-NEXT: }, // JSON-NEXT: "valueCategory": "lvalue", +// JSON-NEXT: "extendingDecl": { +// JSON-NEXT: "id": "0x{{.*}}", +// JSON-NEXT: "kind": "VarDecl", +// JSON-NEXT: "name": "b", +// JSON-NEXT: "type": { +// JSON-NEXT: "qualType": "B" +// JSON-NEXT: } +// JSON-NEXT: }, +// JSON-NEXT: "storageDuration": "automatic", +// JSON-NEXT: "boundToLValueRef": true, // JSON-NEXT: "inner": [ // JSON-NEXT: { // JSON-NEXT: "id": "0x{{.*}}", -// JSON-NEXT: "kind": "MaterializeTemporaryExpr", +// JSON-NEXT: "kind": "ImplicitCastExpr", // JSON: "type": { // JSON-NEXT: "qualType": "const A" // JSON-NEXT: }, -// JSON-NEXT: "valueCategory": "lvalue", -// JSON-NEXT: "extendingDecl": { -// JSON-NEXT: "id": "0x{{.*}}", -// JSON-NEXT: "kind": "VarDecl", -// JSON-NEXT: "name": "b", -// JSON-NEXT: "type": { -// JSON-NEXT: "qualType": "B" -// JSON-NEXT: } -// JSON-NEXT: }, -// JSON-NEXT: "storageDuration": "automatic", -// JSON-NEXT: "boundToLValueRef": true, +// JSON-NEXT: "valueCategory": "prvalue", +// JSON-NEXT: "castKind": "NoOp", // JSON-NEXT: "inner": [ // JSON-NEXT: { // JSON-NEXT: "id": "0x{{.*}}", -// JSON-NEXT: "kind": "ImplicitCastExpr", +// JSON-NEXT: "kind": "CXXFunctionalCastExpr", // JSON: "type": { -// JSON-NEXT: "qualType": "const A" +// JSON-NEXT: "qualType": "A" // JSON-NEXT: }, // JSON-NEXT: "valueCategory": "prvalue", // JSON-NEXT: "castKind": "NoOp", // JSON-NEXT: "inner": [ // JSON-NEXT: { // JSON-NEXT: "id": "0x{{.*}}", -// JSON-NEXT: "kind": "CXXFunctionalCastExpr", +// JSON-NEXT: "kind": "InitListExpr", // JSON: "type": { // JSON-NEXT: "qualType": "A" // JSON-NEXT: }, // JSON-NEXT: "valueCategory": "prvalue", -// JSON-NEXT: "castKind": "NoOp", // JSON-NEXT: "inner": [ // JSON-NEXT: { // JSON-NEXT: "id": "0x{{.*}}", // JSON-NEXT: "kind": "InitListExpr", // JSON: "type": { -// JSON-NEXT: "qualType": "A" +// JSON-NEXT: "qualType": "int[1]" // JSON-NEXT: }, // JSON-NEXT: "valueCategory": "prvalue", // JSON-NEXT: "inner": [ // JSON-NEXT: { // JSON-NEXT: "id": "0x{{.*}}", -// JSON-NEXT: "kind": "InitListExpr", +// JSON-NEXT: "kind": "IntegerLiteral", // JSON: "type": { -// JSON-NEXT: "qualType": "int[1]" +// JSON-NEXT: "qualType": "int" // JSON-NEXT: }, // JSON-NEXT: "valueCategory": "prvalue", -// JSON-NEXT: "inner": [ -// JSON-NEXT: { -// JSON-NEXT: "id": "0x{{.*}}", -// JSON-NEXT: "kind": "IntegerLiteral", -// JSON: "type": { -// JSON-NEXT: "qualType": "int" -// JSON-NEXT: }, -// JSON-NEXT: "valueCategory": "prvalue", -// JSON-NEXT: "value": "0" -// JSON-NEXT: } -// JSON-NEXT: ] +// JSON-NEXT: "value": "0" // JSON-NEXT: } // JSON-NEXT: ] +// JSON-NEXT: } +// JSON-NEXT: ] diff --git a/clang/test/AST/ast-dump-recovery.cpp b/clang/test/AST/ast-dump-recovery.cpp index 3c1811b2c8d02..790b7bb2025fd 100644 --- a/clang/test/AST/ast-dump-recovery.cpp +++ b/clang/test/AST/ast-dump-recovery.cpp @@ -294,7 +294,7 @@ union U { // CHECK-NEXT: `-DeclStmt {{.*}} // CHECK-NEXT: `-VarDecl {{.*}} g 'U' listinit // CHECK-NEXT: `-InitListExpr {{.*}} 'U' contains-errors field Field {{.*}} 'f' 'int' -// CHECK-NEXT: `-CXXDefaultInitExpr {{.*}} 'int' contains-errors has rewritten init +// CHECK-NEXT: `-CXXDefaultInitExpr {{.*}} 'int' contains-errors // CHECK-NEXT: `-RecoveryExpr {{.*}} 'int' contains-errors // DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors void foo() { diff --git a/clang/test/Analysis/lifetime-extended-regions.cpp b/clang/test/Analysis/lifetime-extended-regions.cpp index 4458ad294af7c..02a1210d9af92 100644 --- a/clang/test/Analysis/lifetime-extended-regions.cpp +++ b/clang/test/Analysis/lifetime-extended-regions.cpp @@ -121,11 +121,10 @@ void aggregateWithReferences() { clang_analyzer_dump(viaReference.rx); // expected-warning-re {{&lifetime_extended_object{int, viaReference, S{{[0-9]+}}} }} clang_analyzer_dump(viaReference.ry); // expected-warning-re {{&lifetime_extended_object{Composite, viaReference, S{{[0-9]+}}} }} - // FIXME: clang currently support extending lifetime of object bound to reference members of aggregates, - // that are created from default member initializer. But CFG and ExprEngine need to be updated to address this change. - // The following expect warning: {{&lifetime_extended_object{Composite, defaultInitExtended, S{{[0-9]+}}} }} + // The lifetime of object bound to reference members of aggregates, + // that are created from default member initializer was extended. RefAggregate defaultInitExtended{i}; - clang_analyzer_dump(defaultInitExtended.ry); // expected-warning {{Unknown }} + clang_analyzer_dump(defaultInitExtended.ry); // expected-warning-re {{&lifetime_extended_object{Composite, defaultInitExtended, S{{[0-9]+}}} }} } void lambda() { diff --git a/clang/test/CodeGenCXX/aggregate-default-member-initializers.cpp b/clang/test/CodeGenCXX/aggregate-default-member-initializers.cpp new file mode 100644 index 0000000000000..40758d33de4b0 --- /dev/null +++ b/clang/test/CodeGenCXX/aggregate-default-member-initializers.cpp @@ -0,0 +1,42 @@ +// RUN: %clang_cc1 -std=c++20 -Wno-unused-value -emit-llvm -o - %s | FileCheck %s + +struct A { + int &x; + ~A() { x = 0; } +}; + +struct AA { + int &x; + ~AA() { x = -1; } +}; + +struct B { + int &x; + const A &a = A{x}; +}; + +struct BB { + int &x; + const AA &a = AA{x}; +}; + +// CHECK-LABEL: define{{.*}} i32 @_Z3onev() +int one() { + int x = 1; + B{x}; + // CHECK: call void @_ZN1AD{{[012]}}Ev + // CHECK-NEXT: load i32, ptr + return x; +} + +// The default initializers are part of the same full-expression, so their +// temporaries are destroyed in reverse construction order. +// CHECK-LABEL: define{{.*}} i32 @_Z3twov() +int two() { + int x = 1; + B{x}, BB{x}; + // CHECK: call void @_ZN2AAD{{[012]}}Ev + // CHECK-NEXT: call void @_ZN1AD{{[012]}}Ev + // CHECK-NEXT: load i32, ptr + return x; +} diff --git a/clang/test/SemaCXX/aggregate-default-member-initializers.cpp b/clang/test/SemaCXX/aggregate-default-member-initializers.cpp new file mode 100644 index 0000000000000..fb3db54cf1d91 --- /dev/null +++ b/clang/test/SemaCXX/aggregate-default-member-initializers.cpp @@ -0,0 +1,103 @@ +// RUN: %clang_cc1 -std=c++20 -Wno-unused-value -verify %s +// RUN: %clang_cc1 -std=c++23 -Wno-unused-value -verify %s +// RUN: %clang_cc1 -std=c++20 -Wno-unused-value -verify %s \ +// RUN: -fexperimental-new-constant-interpreter + +namespace lifetime { + +struct A { + int &x; + constexpr ~A() { x = 0; } +}; + +struct AA { + int &x; + constexpr ~AA() { x = -1; } +}; + +struct B { + int &x; + const A &a = A{x}; +}; + +struct BB { + int &x; + const AA &a = AA{x}; +}; + +constexpr int one() { + int x = 1; + B{x}; + return x; +} + +constexpr int two() { + int x = 1; + B{x}, BB{x}; + return x; +} + +constexpr int paren() { + int x = 1; + (B(x)); + return x; +} + +static_assert(one() == 0); +static_assert(two() == 0); +static_assert(paren() == 0); + +} // namespace lifetime + +namespace unevaluated { + +template <typename T> int noInstantiate() { + static_assert(false); + return 0; +} + +struct S { + int x = noInstantiate<int>(); +}; + +int size = sizeof(S{}); + +} // namespace unevaluated + +namespace immediate { + +struct Inner { + int a; + static consteval int decrement(int &x) { + return --x; + } + // FIXME: The aggregate result object does not exist yet when the immediate + // invocation is checked, so reading 'a' fails. This is long-standing and is + // independent of which full-expression the initializer belongs to. + int b = decrement(a); // expected-error {{call to consteval function 'immediate::Inner::decrement' is not a constant expression}} \ + // expected-note {{implicit use of 'this' pointer is only allowed within the evaluation of a call to a 'constexpr' member function}} \ + // expected-note {{declared here}} +}; + +struct Outer { + const Inner &inner = Inner{1}; // expected-note {{in the default initializer of 'b'}} +}; + +constexpr int value = Outer{}.inner.a; +static_assert(value == 0); + +consteval unsigned currentLine(unsigned line = __builtin_LINE()) { + return line; +} + +struct SourceAndRuntime { + unsigned line = currentLine(); + int runtime; +}; + +void sourceAndRuntime(int n) { + // The runtime initializer does not make currentLine() non-constant. + SourceAndRuntime value{.runtime = n}; +} + +} // namespace immediate diff --git a/clang/test/SemaCXX/cxx2c-placeholder-vars.cpp b/clang/test/SemaCXX/cxx2c-placeholder-vars.cpp index 8e428c0ef0427..37824c16f4f05 100644 --- a/clang/test/SemaCXX/cxx2c-placeholder-vars.cpp +++ b/clang/test/SemaCXX/cxx2c-placeholder-vars.cpp @@ -274,16 +274,16 @@ void f() { // CHECK: ClassTemplateSpecializationDecl {{.*}} struct A definition // CHECK: CXXConstructorDecl {{.*}} implicit used constexpr A 'void () noexcept' // CHECK-NEXT: CXXCtorInitializer Field {{.*}} '_' 'int' -// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init +// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' // CHECK-NEXT: IntegerLiteral {{.*}} 'int' 1 // CHECK-NEXT: CXXCtorInitializer Field {{.*}} '_' 'int' -// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init +// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' // CHECK-NEXT: IntegerLiteral {{.*}} 'int' 2 // CHECK-NEXT: CXXCtorInitializer Field {{.*}} 'a' 'int' -// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init +// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' // CHECK-NEXT: IntegerLiteral {{.*}} 'int' 3 // CHECK-NEXT: CXXCtorInitializer Field {{.*}} '_' 'int' -// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init +// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' // CHECK-NEXT: IntegerLiteral {{.*}} 'int' 4 // CHECK-NEXT: CompoundStmt {{.*}} diff --git a/clang/test/SemaCXX/warn-unreachable.cpp b/clang/test/SemaCXX/warn-unreachable.cpp index e6f5bc5ef8e12..79505a2427426 100644 --- a/clang/test/SemaCXX/warn-unreachable.cpp +++ b/clang/test/SemaCXX/warn-unreachable.cpp @@ -414,3 +414,107 @@ void tautological_compare(bool x, int y) { calledFun(); } + +namespace test_rebuilt_default_arg { +struct A { + explicit A(int = __builtin_LINE()); +}; + +int h(int a) { + return 3; + A(); // expected-warning {{will never be executed}} +} + +struct Temp { + Temp(); + ~Temp(); +}; + +struct B { + explicit B(const Temp &t = Temp()); +}; +int f(int a) { + return 3; + B(); // expected-warning {{will never be executed}} +} +} // namespace test_rebuilt_default_arg +namespace test_rebuilt_default_init { + +struct A { + A(); + ~A(); +}; + +struct B { + const A &t = A(); +}; +int f(int a) { + return 3; + A{}; // expected-warning {{will never be executed}} +} +} // namespace test_rebuilt_default_init + +// This issue reported by the comments in https://github.com/llvm/llvm-project/pull/117437. +// All block-level expressions should have already been IgnoreParens()ed. +namespace gh117437_ignore_parens_in_default_arg { + class Location { + public: + static Location Current(int = __builtin_LINE()); + }; + class DOMMatrix; + class BasicMember { + public: + BasicMember(DOMMatrix *); + }; + template <typename> using Member = BasicMember; + class ExceptionState { + public: + ExceptionState &ReturnThis(); + ExceptionState(Location); + }; + class NonThrowableExceptionState : public ExceptionState { + public: + NonThrowableExceptionState(Location location = Location::Current()) + : ExceptionState(location) {} + }; + class DOMMatrix { + public: + static DOMMatrix * + Create(int *, ExceptionState & = (NonThrowableExceptionState().ReturnThis())); + }; + class CSSMatrixComponent { + int CSSMatrixComponent_matrix; + CSSMatrixComponent() + : matrix_(DOMMatrix::Create(&CSSMatrixComponent_matrix)) {} + Member<DOMMatrix> matrix_; + }; +} // namespace gh117437_ignore_parens_in_default_arg + +class Location { + public: + static Location CurrentWithoutFunctionName( + const char* file_name = __builtin_FILE(), + int line_number = __builtin_LINE()); + }; + + class NotReachedNoreturnError { + public: + explicit NotReachedNoreturnError( + const Location& location = + Location::CurrentWithoutFunctionName()); + + [[noreturn]] [[clang::nomerge]] [[clang::noinline]] [[clang::not_tail_called]] + ~NotReachedNoreturnError(); + }; + + #define NOTREACHED() NotReachedNoreturnError() + + int f() { + NOTREACHED(); + return 4; // expected-warning {{will never be executed}} + } + + int g() { + return 4; + NOTREACHED(); // Ok, no diagnostic. + } diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp index f34568a072c32..76cab4618c5e6 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp @@ -2950,6 +2950,24 @@ TEST_P(UncheckedOptionalAccessTest, ConstructorOtherStructField) { )cc"); } +TEST_P(UncheckedOptionalAccessTest, + AggregateDefaultInitializerReferencesPriorField) { + ExpectDiagnosticsFor(R"cc( + #include "unchecked_optional_access_test.h" + struct NonTrivDtor { + NonTrivDtor(int x); + ~NonTrivDtor() {} + }; + struct Other { + $ns::$optional<int> x = $ns::nullopt; + NonTrivDtor y = x.has_value() ? NonTrivDtor(*x) : NonTrivDtor(-1); + }; + struct target { + target() { Other{}; } + }; + )cc"); +} + TEST_P(UncheckedOptionalAccessTest, AssertTrueGtestMacro) { ExpectDiagnosticsFor(R"cc( #include "unchecked_optional_access_test.h" >From 8c9405378f2e75a60948af441e5113d719dd6a71 Mon Sep 17 00:00:00 2001 From: yronglin <[email protected]> Date: Thu, 27 Aug 2026 13:57:57 -0700 Subject: [PATCH 2/3] Merge duplicated code Signed-off-by: yronglin <[email protected]> --- clang/lib/AST/ByteCode/Compiler.cpp | 50 ++++++++++++++--------------- clang/lib/AST/ByteCode/Compiler.h | 6 ++++ 2 files changed, 30 insertions(+), 26 deletions(-) diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp index 6b02a57a0deda..cf4945a3f2ff6 100644 --- a/clang/lib/AST/ByteCode/Compiler.cpp +++ b/clang/lib/AST/ByteCode/Compiler.cpp @@ -3493,17 +3493,8 @@ bool Compiler<Emitter>::VisitExprWithCleanups(const ExprWithCleanups *E) { LocalScope<Emitter> ES(this, ScopeKind::FullExpression); const Expr *SubExpr = E->getSubExpr(); - if (DiscardResult && !SubExpr->isGLValue() && - !canClassify(SubExpr->getType()) && containsDefaultInitExpr(SubExpr)) { - UnsignedOrNone LocalIndex = - allocateLocal(SubExpr, QualType(), ScopeKind::FullExpression); - if (!LocalIndex) - return false; - InitLinkScope<Emitter> ILS(this, InitLink::Temp(*LocalIndex)); - if (!this->emitGetPtrLocal(*LocalIndex, E)) - return false; - return this->visitInitializerPop(SubExpr) && ES.destroyLocals(E); - } + if (DiscardResult && this->discardNeedsResultObject(SubExpr)) + return this->discardIntoResultObject(SubExpr) && ES.destroyLocals(E); return this->delegate(SubExpr) && ES.destroyLocals(E); } @@ -4925,22 +4916,29 @@ bool Compiler<Emitter>::VisitStmtExpr(const StmtExpr *E) { return BS.destroyLocals(); } +template <class Emitter> +bool Compiler<Emitter>::discardNeedsResultObject(const Expr *E) const { + return !E->isGLValue() && !canClassify(E->getType()) && + containsDefaultInitExpr(E); +} + +template <class Emitter> +bool Compiler<Emitter>::discardIntoResultObject(const Expr *E) { + UnsignedOrNone LocalIndex = + allocateLocal(E, QualType(), ScopeKind::FullExpression); + if (!LocalIndex) + return false; + InitLinkScope<Emitter> ILS(this, InitLink::Temp(*LocalIndex)); + if (!this->emitGetPtrLocal(*LocalIndex, E)) + return false; + return this->visitInitializerPop(E); +} + template <class Emitter> bool Compiler<Emitter>::discard(const Expr *E) { - // A discarded composite prvalue still needs a result object when a default - // member initializer refers to previously initialized subobjects. Let an - // ExprWithCleanups establish its full-expression scope before allocating - // that object. - if (!isa<ExprWithCleanups>(E) && !E->isGLValue() && - !canClassify(E->getType()) && containsDefaultInitExpr(E)) { - UnsignedOrNone LocalIndex = - allocateLocal(E, QualType(), ScopeKind::FullExpression); - if (!LocalIndex) - return false; - InitLinkScope<Emitter> ILS(this, InitLink::Temp(*LocalIndex)); - if (!this->emitGetPtrLocal(*LocalIndex, E)) - return false; - return this->visitInitializerPop(E); - } + // Let an ExprWithCleanups establish its full-expression scope first; it + // allocates the result object itself. + if (!isa<ExprWithCleanups>(E) && this->discardNeedsResultObject(E)) + return this->discardIntoResultObject(E); OptionScope<Emitter> Scope(this, /*NewDiscardResult=*/true, /*NewInitializing=*/false, /*ToLValue=*/false); diff --git a/clang/lib/AST/ByteCode/Compiler.h b/clang/lib/AST/ByteCode/Compiler.h index f34809cd0f14c..6e22f57749e43 100644 --- a/clang/lib/AST/ByteCode/Compiler.h +++ b/clang/lib/AST/ByteCode/Compiler.h @@ -321,6 +321,12 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>, bool visitAsLValue(const Expr *E); /// Evaluates an expression for side effects and discards the result. bool discard(const Expr *E); + /// Whether discarding \p E still requires a result object: a composite + /// prvalue whose default member initializer may refer to previously + /// initialized subobjects, so that `this` has something to denote. + bool discardNeedsResultObject(const Expr *E) const; + /// Allocate that result object and initialize \p E into it. + bool discardIntoResultObject(const Expr *E); /// Just pass evaluation on to \p E. This leaves all the parsing flags /// intact. bool delegate(const Expr *E); >From 4df9f9fa29270275eb6c3258ca24cbedc18e6a68 Mon Sep 17 00:00:00 2001 From: yronglin <[email protected]> Date: Thu, 27 Aug 2026 20:34:05 -0700 Subject: [PATCH 3/3] [clang] Don't need rebuild ctor DIE Signed-off-by: yronglin <[email protected]> --- clang/lib/Sema/SemaExpr.cpp | 20 +++++++++++++++++-- .../aggregate-default-member-initializers.cpp | 2 +- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 31b308ad4b52b..34a0bff7c5289 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -5979,7 +5979,6 @@ ExprResult Sema::BuildCXXCtorDefaultInitExpr(SourceLocation Loc, assert(Field->hasInClassInitializer()); bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer(); - bool NeedRebuild = needsRebuildOfDefaultArgOrInit(); // C++11 [class.base.init]p7: // The initialization of each base and member constitutes a @@ -5994,10 +5993,18 @@ ExprResult Sema::BuildCXXCtorDefaultInitExpr(SourceLocation Loc, if (!InitContext) InitContext.emplace(Loc, Field, CurContext); + // [class.temporary]/p7: + // If such a temporary object would otherwise be destroyed at the end of the + // for-range-initializer full-expression, the object persists for the lifetime + // of the reference initialized by the for-range-initializer. + // + // A default member initializer used by a constructor is a separate + // full-expression, we don't need extend temporaries lifetime in this + // situation, the NeedRebuild will always false. ExprResult Init = BuildCXXDefaultInitInternal( Loc, Field, InitializedEntity::InitializeMemberFromDefaultMemberInitializer(Field), - NestedDefaultChecking, NeedRebuild); + NestedDefaultChecking, /*NeedRebuild=*/false); if (Init.isInvalid()) return ExprError(); @@ -6033,6 +6040,15 @@ Sema::BuildCXXAggregateDefaultInitExpr(SourceLocation Loc, FieldDecl *Field, if (!InitContext) InitContext.emplace(Loc, Field, CurContext); + // [class.temporary]/p7: + // If such a temporary object would otherwise be destroyed at the end of the + // for-range-initializer full-expression, the object persists for the lifetime + // of the reference initialized by the for-range-initializer. + // + // A default member initializer used by an aggregate initialization belongs to + // the full-expression containing the aggregate initialization. we need extend + // temporaries lifetime in this situation, the NeedRebuild will always true. + // CWG1815: always rebuild, never share the AST built when the field was // declared. Only a copy rebuilt here has its MaterializeTemporaryExprs // collected in this context, which is what lets the aggregate initialization diff --git a/clang/test/CodeGenCXX/aggregate-default-member-initializers.cpp b/clang/test/CodeGenCXX/aggregate-default-member-initializers.cpp index 40758d33de4b0..b0a8d63450aff 100644 --- a/clang/test/CodeGenCXX/aggregate-default-member-initializers.cpp +++ b/clang/test/CodeGenCXX/aggregate-default-member-initializers.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -std=c++20 -Wno-unused-value -emit-llvm -o - %s | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++20 -Wno-unused-value -emit-llvm -o - %s | FileCheck %s struct A { int &x; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
