[Clang] Extend lifetime of temporaries in mem-default-init for P2718R0 #86960
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
yronglin
force-pushed
the
extend_default_member_init
branch
from
3ca525d to
2e05cb4
Compare
yronglin
changed the title
yronglin
force-pushed
the
extend_default_member_init
branch
from
2e05cb4 to
8bd2742
Compare
yronglin
requested review from
AaronBallman,
zygoloid,
cor3ntin and
hubert-reinterpretcast
yronglin
force-pushed
the
extend_default_member_init
branch
2 times, most recently
from
297e4f6 to
55f6fe0
Compare
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-codegen Author: None (yronglin) ChangesFixes #85613. In #76361, we've not implement the lifetime extensions for the temporaries which in There are two main modifications made to this PR:
TODO: The lit test not add yet, because I'm not sure it's the correct approach, but I've posted some cases in #85613. Patch is 27.12 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/86960.diff 15 Files Affected:
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index f49bc724c96c89..0a07e850c74e47 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -5078,34 +5078,6 @@ class Sema final : public SemaBase {
/// example, in a for-range initializer).
bool InLifetimeExtendingContext = false;
- /// Whether we are currently in a context in which all temporaries must be
- /// materialized.
- ///
- /// [class.temporary]/p2:
- /// The materialization of a temporary object is generally delayed as long
- /// as possible in order to avoid creating unnecessary temporary objects.
- ///
- /// Temporary objects are materialized:
- /// (2.1) when binding a reference to a prvalue ([dcl.init.ref],
- /// [expr.type.conv], [expr.dynamic.cast], [expr.static.cast],
- /// [expr.const.cast], [expr.cast]),
- ///
- /// (2.2) when performing member access on a class prvalue ([expr.ref],
- /// [expr.mptr.oper]),
- ///
- /// (2.3) when performing an array-to-pointer conversion or subscripting
- /// on an array prvalue ([conv.array], [expr.sub]),
- ///
- /// (2.4) when initializing an object of type
- /// std::initializer_list<T> from a braced-init-list
- /// ([dcl.init.list]),
- ///
- /// (2.5) for certain unevaluated operands ([expr.typeid], [expr.sizeof])
- ///
- /// (2.6) when a prvalue that has type other than cv void appears as a
- /// discarded-value expression ([expr.context]).
- bool InMaterializeTemporaryObjectContext = false;
-
// When evaluating immediate functions in the initializer of a default
// argument or default member initializer, this is the declaration whose
// default initializer is being evaluated and the location of the call
@@ -6386,19 +6358,6 @@ class Sema final : public SemaBase {
}
}
- /// keepInMaterializeTemporaryObjectContext - Pull down
- /// InMaterializeTemporaryObjectContext flag from previous context.
- void keepInMaterializeTemporaryObjectContext() {
- if (ExprEvalContexts.size() > 2 &&
- ExprEvalContexts[ExprEvalContexts.size() - 2]
- .InMaterializeTemporaryObjectContext) {
- auto &LastRecord = ExprEvalContexts.back();
- auto &PrevRecord = ExprEvalContexts[ExprEvalContexts.size() - 2];
- LastRecord.InMaterializeTemporaryObjectContext =
- PrevRecord.InMaterializeTemporaryObjectContext;
- }
- }
-
DefaultedComparisonKind getDefaultedComparisonKind(const FunctionDecl *FD) {
return getDefaultedFunctionKind(FD).asComparison();
}
@@ -6542,12 +6501,6 @@ class Sema final : public SemaBase {
/// used in initializer of the field.
llvm::MapVector<FieldDecl *, DeleteLocs> DeleteExprs;
- bool isInMaterializeTemporaryObjectContext() const {
- assert(!ExprEvalContexts.empty() &&
- "Must be in an expression evaluation context");
- return ExprEvalContexts.back().InMaterializeTemporaryObjectContext;
- }
-
ParsedType getInheritingConstructorName(CXXScopeSpec &SS,
SourceLocation NameLoc,
IdentifierInfo &Name);
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 2480972f1432f7..27f616d497a2c5 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -274,9 +274,9 @@ void CodeGenFunction::EmitAnyExprToMem(const Expr *E,
llvm_unreachable("bad evaluation kind");
}
-static void
-pushTemporaryCleanup(CodeGenFunction &CGF, const MaterializeTemporaryExpr *M,
- const Expr *E, Address ReferenceTemporary) {
+void CodeGenFunction::pushTemporaryCleanup(const MaterializeTemporaryExpr *M,
+ const Expr *E,
+ Address ReferenceTemporary) {
// Objective-C++ ARC:
// If we are binding a reference to a temporary that has ownership, we
// need to perform retain/release operations on the temporary.
@@ -311,9 +311,9 @@ pushTemporaryCleanup(CodeGenFunction &CGF, const MaterializeTemporaryExpr *M,
CleanupKind CleanupKind;
if (Lifetime == Qualifiers::OCL_Strong) {
const ValueDecl *VD = M->getExtendingDecl();
- bool Precise =
- VD && isa<VarDecl>(VD) && VD->hasAttr<ObjCPreciseLifetimeAttr>();
- CleanupKind = CGF.getARCCleanupKind();
+ bool Precise = isa_and_nonnull<VarDecl>(VD) &&
+ VD->hasAttr<ObjCPreciseLifetimeAttr>();
+ CleanupKind = getARCCleanupKind();
Destroy = Precise ? &CodeGenFunction::destroyARCStrongPrecise
: &CodeGenFunction::destroyARCStrongImprecise;
} else {
@@ -323,13 +323,12 @@ pushTemporaryCleanup(CodeGenFunction &CGF, const MaterializeTemporaryExpr *M,
Destroy = &CodeGenFunction::destroyARCWeak;
}
if (Duration == SD_FullExpression)
- CGF.pushDestroy(CleanupKind, ReferenceTemporary,
- M->getType(), *Destroy,
- CleanupKind & EHCleanup);
+ pushDestroy(CleanupKind, ReferenceTemporary, M->getType(), *Destroy,
+ CleanupKind & EHCleanup);
else
- CGF.pushLifetimeExtendedDestroy(CleanupKind, ReferenceTemporary,
- M->getType(),
- *Destroy, CleanupKind & EHCleanup);
+ pushLifetimeExtendedDestroy(CleanupKind, ReferenceTemporary,
+ M->getType(), *Destroy,
+ CleanupKind & EHCleanup);
return;
case SD_Dynamic:
@@ -358,32 +357,31 @@ pushTemporaryCleanup(CodeGenFunction &CGF, const MaterializeTemporaryExpr *M,
llvm::FunctionCallee CleanupFn;
llvm::Constant *CleanupArg;
if (E->getType()->isArrayType()) {
- CleanupFn = CodeGenFunction(CGF.CGM).generateDestroyHelper(
- ReferenceTemporary, E->getType(),
- CodeGenFunction::destroyCXXObject, CGF.getLangOpts().Exceptions,
+ CleanupFn = CodeGenFunction(CGM).generateDestroyHelper(
+ ReferenceTemporary, E->getType(), CodeGenFunction::destroyCXXObject,
+ getLangOpts().Exceptions,
dyn_cast_or_null<VarDecl>(M->getExtendingDecl()));
- CleanupArg = llvm::Constant::getNullValue(CGF.Int8PtrTy);
+ CleanupArg = llvm::Constant::getNullValue(Int8PtrTy);
} else {
- CleanupFn = CGF.CGM.getAddrAndTypeOfCXXStructor(
+ CleanupFn = CGM.getAddrAndTypeOfCXXStructor(
GlobalDecl(ReferenceTemporaryDtor, Dtor_Complete));
- CleanupArg = cast<llvm::Constant>(ReferenceTemporary.emitRawPointer(CGF));
+ CleanupArg =
+ cast<llvm::Constant>(ReferenceTemporary.emitRawPointer(*this));
}
- CGF.CGM.getCXXABI().registerGlobalDtor(
- CGF, *cast<VarDecl>(M->getExtendingDecl()), CleanupFn, CleanupArg);
+ CGM.getCXXABI().registerGlobalDtor(
+ *this, *cast<VarDecl>(M->getExtendingDecl()), CleanupFn, CleanupArg);
break;
}
case SD_FullExpression:
- CGF.pushDestroy(NormalAndEHCleanup, ReferenceTemporary, E->getType(),
- CodeGenFunction::destroyCXXObject,
- CGF.getLangOpts().Exceptions);
+ pushDestroy(NormalAndEHCleanup, ReferenceTemporary, E->getType(),
+ CodeGenFunction::destroyCXXObject, getLangOpts().Exceptions);
break;
case SD_Automatic:
- CGF.pushLifetimeExtendedDestroy(NormalAndEHCleanup,
- ReferenceTemporary, E->getType(),
- CodeGenFunction::destroyCXXObject,
- CGF.getLangOpts().Exceptions);
+ pushLifetimeExtendedDestroy(NormalAndEHCleanup, ReferenceTemporary,
+ E->getType(), CodeGenFunction::destroyCXXObject,
+ getLangOpts().Exceptions);
break;
case SD_Dynamic:
@@ -490,7 +488,7 @@ EmitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *M) {
}
}
- pushTemporaryCleanup(*this, M, E, Object);
+ pushTemporaryCleanup(M, E, Object);
return RefTempDst;
}
@@ -579,7 +577,13 @@ EmitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *M) {
}
EmitAnyExprToMem(E, Object, Qualifiers(), /*IsInit*/true);
}
- pushTemporaryCleanup(*this, M, E, Object);
+
+ // If this temporary extended by for-range variable, delay to emitting
+ // cleanup.
+ if (CurLexicalScope && CurLexicalScope->isExtendedByForRangeVar(M))
+ CurLexicalScope->addForRangeInitTemp(M, Object);
+ else
+ pushTemporaryCleanup(M, E, Object);
// Perform derived-to-base casts and/or field accesses, to get from the
// temporary object we created (and, potentially, for which we extended
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index 576fe2f7a2d46f..d277d15b17df13 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -1230,11 +1230,29 @@ CodeGenFunction::EmitCXXForRangeStmt(const CXXForRangeStmt &S,
JumpDest LoopExit = getJumpDestInCurrentScope("for.end");
LexicalScope ForScope(*this, S.getSourceRange());
+ const DeclStmt *RangeDS = cast<DeclStmt>(S.getRangeStmt());
+ const VarDecl *RangeVar = cast<VarDecl>(RangeDS->getSingleDecl());
+ if (getLangOpts().CPlusPlus23)
+ ForScope.setForRangeVar(RangeVar);
// Evaluate the first pieces before the loop.
if (S.getInit())
EmitStmt(S.getInit());
EmitStmt(S.getRangeStmt());
+
+ // Emit cleanup for tempories in for-range-init expression.
+ if (getLangOpts().CPlusPlus23) {
+ RunCleanupsScope Scope(*this);
+ auto LifetimeExtendTemps = ForScope.getForRangeInitTemps();
+ for (const auto &Temp : LifetimeExtendTemps) {
+ // llvm::errs() << "===============Begin===============\n";
+ auto [M, Object] = Temp;
+ // M->dumpColor();
+ pushTemporaryCleanup(M, M->getSubExpr(), Object);
+ // llvm::errs() << "================End================\n";
+ }
+ }
+
EmitStmt(S.getBeginStmt());
EmitStmt(S.getEndStmt());
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index e2a7e28c8211ea..167ac71fefb722 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -875,6 +875,9 @@ class CodeGenFunction : public CodeGenTypeCache {
new (Buffer + sizeof(Header) + sizeof(T)) RawAddress(ActiveFlag);
}
+ void pushTemporaryCleanup(const MaterializeTemporaryExpr *M, const Expr *E,
+ Address ReferenceTemporary);
+
/// Set up the last cleanup that was pushed as a conditional
/// full-expression cleanup.
void initFullExprCleanup() {
@@ -982,11 +985,24 @@ class CodeGenFunction : public CodeGenTypeCache {
EHScopeStack::stable_iterator CurrentCleanupScopeDepth =
EHScopeStack::stable_end();
+ struct ForRangeInitLifetimeExtendTemporary {
+ const MaterializeTemporaryExpr *M;
+ RawAddress Object;
+ };
+
class LexicalScope : public RunCleanupsScope {
SourceRange Range;
SmallVector<const LabelDecl*, 4> Labels;
+ SmallVector<ForRangeInitLifetimeExtendTemporary, 4> ForRangeInitTemps;
LexicalScope *ParentScope;
+ // This will be set to `__range` variable when we emitting a
+ // CXXForRangeStmt. It was used to check whether we are emitting a
+ // materialized temporary which in for-range-init and lifetime-extended by
+ // __range var. If so, the codegen of cleanup for that temporary object
+ // needs to be delayed.
+ const VarDecl *ForRangeVar = nullptr;
+
LexicalScope(const LexicalScope &) = delete;
void operator=(const LexicalScope &) = delete;
@@ -1004,6 +1020,21 @@ class CodeGenFunction : public CodeGenTypeCache {
Labels.push_back(label);
}
+ void addForRangeInitTemp(const MaterializeTemporaryExpr *M,
+ RawAddress Object) {
+ assert(PerformCleanup && "adding temps to dead scope?");
+ ForRangeInitTemps.push_back({M, Object});
+ }
+
+ ArrayRef<ForRangeInitLifetimeExtendTemporary> getForRangeInitTemps() const {
+ return ForRangeInitTemps;
+ }
+
+ void setForRangeVar(const VarDecl *Var) { ForRangeVar = Var; }
+ bool isExtendedByForRangeVar(const MaterializeTemporaryExpr *M) const {
+ return M && ForRangeVar && M->getExtendingDecl() == ForRangeVar;
+ }
+
/// Exit this cleanup scope, emitting any accumulated
/// cleanups.
~LexicalScope() {
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 0aa14b0510746b..ae5d2cae06e536 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -2379,10 +2379,6 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
if (getLangOpts().CPlusPlus23) {
auto &LastRecord = Actions.ExprEvalContexts.back();
LastRecord.InLifetimeExtendingContext = true;
-
- // Materialize non-`cv void` prvalue temporaries in discarded
- // expressions. These materialized temporaries may be lifetime-extented.
- LastRecord.InMaterializeTemporaryObjectContext = true;
}
if (getLangOpts().OpenMP)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ffe7e44e2387f4..2caf8a3a3c8e83 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6331,7 +6331,6 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
// Pass down lifetime extending flag, and collect temporaries in
// CreateMaterializeTemporaryExpr when we rewrite the call argument.
keepInLifetimeExtendingContext();
- keepInMaterializeTemporaryObjectContext();
EnsureImmediateInvocationInDefaultArgs Immediate(*this);
ExprResult Res;
runWithSufficientStackSpace(CallLoc, [&] {
@@ -6377,7 +6376,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
Expr *Init = nullptr;
bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();
-
+ bool InLifetimeExtendingContext = isInLifetimeExtendingContext();
EnterExpressionEvaluationContext EvalContext(
*this, ExpressionEvaluationContext::PotentiallyEvaluated, Field);
@@ -6412,19 +6411,21 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
ImmediateCallVisitor V(getASTContext());
if (!NestedDefaultChecking)
V.TraverseDecl(Field);
- if (V.HasImmediateCalls) {
+ if (V.HasImmediateCalls || InLifetimeExtendingContext) {
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.
+ keepInLifetimeExtendingContext();
EnsureImmediateInvocationInDefaultArgs Immediate(*this);
ExprResult Res;
runWithSufficientStackSpace(Loc, [&] {
Res = Immediate.TransformInitializer(Field->getInClassInitializer(),
/*CXXDirectInit=*/false);
});
- if (!Res.isInvalid())
+ if (Res.isUsable())
Res = ConvertMemberDefaultInitExpression(Field, Res.get(), Loc);
if (Res.isInvalid()) {
Field->setInvalidDecl();
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index db84f181012268..9f13e3827c742d 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -8361,7 +8361,7 @@ ExprResult Sema::IgnoredValueConversions(Expr *E) {
// unnecessary temporary objects. If we skip this step, IR generation is
// able to synthesize the storage for itself in the aggregate case, and
// adding the extra node to the AST is just clutter.
- if (isInMaterializeTemporaryObjectContext() && getLangOpts().CPlusPlus17 &&
+ if (isInLifetimeExtendingContext() && getLangOpts().CPlusPlus17 &&
E->isPRValue() && !E->getType()->isVoidType()) {
ExprResult Res = TemporaryMaterializationConversion(E);
if (Res.isInvalid())
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index a75e9925a43146..c2269206988efc 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -710,6 +710,26 @@ void InitListChecker::FillInEmptyInitForField(unsigned Init, FieldDecl *Field,
if (VerifyOnly)
return;
+ // Enter a lifetime extension context, then we can support lifetime
+ // extension of temporary created by aggregate initialization using a
+ // default member initializer (DR1815 https://wg21.link/CWG1815).
+ //
+ // In a lifetime extension context, BuildCXXDefaultInitExpr will clone the
+ // initializer expression on each use that would lifetime extend its
+ // temporaries.
+ EnterExpressionEvaluationContext LifetimeExtensionContext(
+ SemaRef, Sema::ExpressionEvaluationContext::PotentiallyEvaluated,
+ /*LambdaContextDecl=*/nullptr,
+ Sema::ExpressionEvaluationContextRecord::EK_Other, true);
+
+ // Lifetime extension in default-member-init.
+ auto &LastRecord = SemaRef.ExprEvalContexts.back();
+
+ // Just copy previous record, make sure we haven't forget anything.
+ LastRecord =
+ SemaRef.ExprEvalContexts[SemaRef.ExprEvalContexts.size() - 2];
+ LastRecord.InLifetimeExtendingContext = true;
+
ExprResult DIE = SemaRef.BuildCXXDefaultInitExpr(Loc, Field);
if (DIE.isInvalid()) {
hadError = true;
@@ -7699,6 +7719,8 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
// Step into CXXDefaultInitExprs so we can diagnose cases where a
// constructor inherits one as an implicit mem-initializer.
if (auto *DIE = dyn_cast<CXXDefaultInitExpr>(Init)) {
+ assert(DIE->hasRewrittenInit() &&
+ "CXXDefaultInitExpr must has rewritten init");
Path.push_back(
{IndirectLocalPathEntry::DefaultInit, DIE, DIE->getField()});
Init = DIE->getExpr();
@@ -8194,24 +8216,12 @@ void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
switch (shouldLifetimeExtendThroughPath(Path)) {
case PathLifetimeKind::Extend:
+ case PathLifetimeKind::ShouldExtend:
// Update the storage duration of the materialized temporary.
- // FIXME: Rebuild the expression instead of mutating it.
MTE->setExtendingDecl(ExtendingEntity->getDecl(),
ExtendingEntity->allocateManglingNumber());
- // Also visit the temporaries lifetime-extended by this initializer.
return true;
- case PathLifetimeKind::ShouldExtend:
- // We're supposed to lifetime-extend the temporary along this path (per
- // the resolution of DR1815), but we don't support that yet.
- //
- // FIXME: Properly handle this situation. Perhaps the easiest approach
- // would be to clone the initializer expression on each use that would
- // lifetime extend its temporaries.
- Diag(DiagLoc, diag::warn_unsupported_lifetime_extension)
- << RK << DiagRange;
- break;
-
case PathLifetimeKind::NoExtend:
// If the path goes through the initialization of a variable or field,
// it can't possibly reach a temporary created in this full-expression.
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 1cb071e4eb7d1c..15d0b8a69bcd2c 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5477,7 +5477,6 @@ void Sema::InstantiateVariableInitializer(
*this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, Var);
keepInLifetimeExtendingContext();
- keepInMaterializeTemporaryObjectContext();
// Instantiate the initializer.
ExprResult Init;
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 7df352c24e8648..818837064a38b7 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransfo...
[truncated]
|
|
@llvm/pr-subscribers-clang Author: None (yronglin) ChangesFixes #85613. In #76361, we've not implement the lifetime extensions for the temporaries which in There are two main modifications made to this PR:
TODO: The lit test not add yet, because I'm not sure it's the correct approach, but I've posted some cases in #85613. Patch is 27.12 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/86960.diff 15 Files Affected:
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index f49bc724c96c89..0a07e850c74e47 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -5078,34 +5078,6 @@ class Sema final : public SemaBase {
/// example, in a for-range initializer).
bool InLifetimeExtendingContext = false;
- /// Whether we are currently in a context in which all temporaries must be
- /// materialized.
- ///
- /// [class.temporary]/p2:
- /// The materialization of a temporary object is generally delayed as long
- /// as possible in order to avoid creating unnecessary temporary objects.
- ///
- /// Temporary objects are materialized:
- /// (2.1) when binding a reference to a prvalue ([dcl.init.ref],
- /// [expr.type.conv], [expr.dynamic.cast], [expr.static.cast],
- /// [expr.const.cast], [expr.cast]),
- ///
- /// (2.2) when performing member access on a class prvalue ([expr.ref],
- /// [expr.mptr.oper]),
- ///
- /// (2.3) when performing an array-to-pointer conversion or subscripting
- /// on an array prvalue ([conv.array], [expr.sub]),
- ///
- /// (2.4) when initializing an object of type
- /// std::initializer_list<T> from a braced-init-list
- /// ([dcl.init.list]),
- ///
- /// (2.5) for certain unevaluated operands ([expr.typeid], [expr.sizeof])
- ///
- /// (2.6) when a prvalue that has type other than cv void appears as a
- /// discarded-value expression ([expr.context]).
- bool InMaterializeTemporaryObjectContext = false;
-
// When evaluating immediate functions in the initializer of a default
// argument or default member initializer, this is the declaration whose
// default initializer is being evaluated and the location of the call
@@ -6386,19 +6358,6 @@ class Sema final : public SemaBase {
}
}
- /// keepInMaterializeTemporaryObjectContext - Pull down
- /// InMaterializeTemporaryObjectContext flag from previous context.
- void keepInMaterializeTemporaryObjectContext() {
- if (ExprEvalContexts.size() > 2 &&
- ExprEvalContexts[ExprEvalContexts.size() - 2]
- .InMaterializeTemporaryObjectContext) {
- auto &LastRecord = ExprEvalContexts.back();
- auto &PrevRecord = ExprEvalContexts[ExprEvalContexts.size() - 2];
- LastRecord.InMaterializeTemporaryObjectContext =
- PrevRecord.InMaterializeTemporaryObjectContext;
- }
- }
-
DefaultedComparisonKind getDefaultedComparisonKind(const FunctionDecl *FD) {
return getDefaultedFunctionKind(FD).asComparison();
}
@@ -6542,12 +6501,6 @@ class Sema final : public SemaBase {
/// used in initializer of the field.
llvm::MapVector<FieldDecl *, DeleteLocs> DeleteExprs;
- bool isInMaterializeTemporaryObjectContext() const {
- assert(!ExprEvalContexts.empty() &&
- "Must be in an expression evaluation context");
- return ExprEvalContexts.back().InMaterializeTemporaryObjectContext;
- }
-
ParsedType getInheritingConstructorName(CXXScopeSpec &SS,
SourceLocation NameLoc,
IdentifierInfo &Name);
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 2480972f1432f7..27f616d497a2c5 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -274,9 +274,9 @@ void CodeGenFunction::EmitAnyExprToMem(const Expr *E,
llvm_unreachable("bad evaluation kind");
}
-static void
-pushTemporaryCleanup(CodeGenFunction &CGF, const MaterializeTemporaryExpr *M,
- const Expr *E, Address ReferenceTemporary) {
+void CodeGenFunction::pushTemporaryCleanup(const MaterializeTemporaryExpr *M,
+ const Expr *E,
+ Address ReferenceTemporary) {
// Objective-C++ ARC:
// If we are binding a reference to a temporary that has ownership, we
// need to perform retain/release operations on the temporary.
@@ -311,9 +311,9 @@ pushTemporaryCleanup(CodeGenFunction &CGF, const MaterializeTemporaryExpr *M,
CleanupKind CleanupKind;
if (Lifetime == Qualifiers::OCL_Strong) {
const ValueDecl *VD = M->getExtendingDecl();
- bool Precise =
- VD && isa<VarDecl>(VD) && VD->hasAttr<ObjCPreciseLifetimeAttr>();
- CleanupKind = CGF.getARCCleanupKind();
+ bool Precise = isa_and_nonnull<VarDecl>(VD) &&
+ VD->hasAttr<ObjCPreciseLifetimeAttr>();
+ CleanupKind = getARCCleanupKind();
Destroy = Precise ? &CodeGenFunction::destroyARCStrongPrecise
: &CodeGenFunction::destroyARCStrongImprecise;
} else {
@@ -323,13 +323,12 @@ pushTemporaryCleanup(CodeGenFunction &CGF, const MaterializeTemporaryExpr *M,
Destroy = &CodeGenFunction::destroyARCWeak;
}
if (Duration == SD_FullExpression)
- CGF.pushDestroy(CleanupKind, ReferenceTemporary,
- M->getType(), *Destroy,
- CleanupKind & EHCleanup);
+ pushDestroy(CleanupKind, ReferenceTemporary, M->getType(), *Destroy,
+ CleanupKind & EHCleanup);
else
- CGF.pushLifetimeExtendedDestroy(CleanupKind, ReferenceTemporary,
- M->getType(),
- *Destroy, CleanupKind & EHCleanup);
+ pushLifetimeExtendedDestroy(CleanupKind, ReferenceTemporary,
+ M->getType(), *Destroy,
+ CleanupKind & EHCleanup);
return;
case SD_Dynamic:
@@ -358,32 +357,31 @@ pushTemporaryCleanup(CodeGenFunction &CGF, const MaterializeTemporaryExpr *M,
llvm::FunctionCallee CleanupFn;
llvm::Constant *CleanupArg;
if (E->getType()->isArrayType()) {
- CleanupFn = CodeGenFunction(CGF.CGM).generateDestroyHelper(
- ReferenceTemporary, E->getType(),
- CodeGenFunction::destroyCXXObject, CGF.getLangOpts().Exceptions,
+ CleanupFn = CodeGenFunction(CGM).generateDestroyHelper(
+ ReferenceTemporary, E->getType(), CodeGenFunction::destroyCXXObject,
+ getLangOpts().Exceptions,
dyn_cast_or_null<VarDecl>(M->getExtendingDecl()));
- CleanupArg = llvm::Constant::getNullValue(CGF.Int8PtrTy);
+ CleanupArg = llvm::Constant::getNullValue(Int8PtrTy);
} else {
- CleanupFn = CGF.CGM.getAddrAndTypeOfCXXStructor(
+ CleanupFn = CGM.getAddrAndTypeOfCXXStructor(
GlobalDecl(ReferenceTemporaryDtor, Dtor_Complete));
- CleanupArg = cast<llvm::Constant>(ReferenceTemporary.emitRawPointer(CGF));
+ CleanupArg =
+ cast<llvm::Constant>(ReferenceTemporary.emitRawPointer(*this));
}
- CGF.CGM.getCXXABI().registerGlobalDtor(
- CGF, *cast<VarDecl>(M->getExtendingDecl()), CleanupFn, CleanupArg);
+ CGM.getCXXABI().registerGlobalDtor(
+ *this, *cast<VarDecl>(M->getExtendingDecl()), CleanupFn, CleanupArg);
break;
}
case SD_FullExpression:
- CGF.pushDestroy(NormalAndEHCleanup, ReferenceTemporary, E->getType(),
- CodeGenFunction::destroyCXXObject,
- CGF.getLangOpts().Exceptions);
+ pushDestroy(NormalAndEHCleanup, ReferenceTemporary, E->getType(),
+ CodeGenFunction::destroyCXXObject, getLangOpts().Exceptions);
break;
case SD_Automatic:
- CGF.pushLifetimeExtendedDestroy(NormalAndEHCleanup,
- ReferenceTemporary, E->getType(),
- CodeGenFunction::destroyCXXObject,
- CGF.getLangOpts().Exceptions);
+ pushLifetimeExtendedDestroy(NormalAndEHCleanup, ReferenceTemporary,
+ E->getType(), CodeGenFunction::destroyCXXObject,
+ getLangOpts().Exceptions);
break;
case SD_Dynamic:
@@ -490,7 +488,7 @@ EmitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *M) {
}
}
- pushTemporaryCleanup(*this, M, E, Object);
+ pushTemporaryCleanup(M, E, Object);
return RefTempDst;
}
@@ -579,7 +577,13 @@ EmitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *M) {
}
EmitAnyExprToMem(E, Object, Qualifiers(), /*IsInit*/true);
}
- pushTemporaryCleanup(*this, M, E, Object);
+
+ // If this temporary extended by for-range variable, delay to emitting
+ // cleanup.
+ if (CurLexicalScope && CurLexicalScope->isExtendedByForRangeVar(M))
+ CurLexicalScope->addForRangeInitTemp(M, Object);
+ else
+ pushTemporaryCleanup(M, E, Object);
// Perform derived-to-base casts and/or field accesses, to get from the
// temporary object we created (and, potentially, for which we extended
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index 576fe2f7a2d46f..d277d15b17df13 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -1230,11 +1230,29 @@ CodeGenFunction::EmitCXXForRangeStmt(const CXXForRangeStmt &S,
JumpDest LoopExit = getJumpDestInCurrentScope("for.end");
LexicalScope ForScope(*this, S.getSourceRange());
+ const DeclStmt *RangeDS = cast<DeclStmt>(S.getRangeStmt());
+ const VarDecl *RangeVar = cast<VarDecl>(RangeDS->getSingleDecl());
+ if (getLangOpts().CPlusPlus23)
+ ForScope.setForRangeVar(RangeVar);
// Evaluate the first pieces before the loop.
if (S.getInit())
EmitStmt(S.getInit());
EmitStmt(S.getRangeStmt());
+
+ // Emit cleanup for tempories in for-range-init expression.
+ if (getLangOpts().CPlusPlus23) {
+ RunCleanupsScope Scope(*this);
+ auto LifetimeExtendTemps = ForScope.getForRangeInitTemps();
+ for (const auto &Temp : LifetimeExtendTemps) {
+ // llvm::errs() << "===============Begin===============\n";
+ auto [M, Object] = Temp;
+ // M->dumpColor();
+ pushTemporaryCleanup(M, M->getSubExpr(), Object);
+ // llvm::errs() << "================End================\n";
+ }
+ }
+
EmitStmt(S.getBeginStmt());
EmitStmt(S.getEndStmt());
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index e2a7e28c8211ea..167ac71fefb722 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -875,6 +875,9 @@ class CodeGenFunction : public CodeGenTypeCache {
new (Buffer + sizeof(Header) + sizeof(T)) RawAddress(ActiveFlag);
}
+ void pushTemporaryCleanup(const MaterializeTemporaryExpr *M, const Expr *E,
+ Address ReferenceTemporary);
+
/// Set up the last cleanup that was pushed as a conditional
/// full-expression cleanup.
void initFullExprCleanup() {
@@ -982,11 +985,24 @@ class CodeGenFunction : public CodeGenTypeCache {
EHScopeStack::stable_iterator CurrentCleanupScopeDepth =
EHScopeStack::stable_end();
+ struct ForRangeInitLifetimeExtendTemporary {
+ const MaterializeTemporaryExpr *M;
+ RawAddress Object;
+ };
+
class LexicalScope : public RunCleanupsScope {
SourceRange Range;
SmallVector<const LabelDecl*, 4> Labels;
+ SmallVector<ForRangeInitLifetimeExtendTemporary, 4> ForRangeInitTemps;
LexicalScope *ParentScope;
+ // This will be set to `__range` variable when we emitting a
+ // CXXForRangeStmt. It was used to check whether we are emitting a
+ // materialized temporary which in for-range-init and lifetime-extended by
+ // __range var. If so, the codegen of cleanup for that temporary object
+ // needs to be delayed.
+ const VarDecl *ForRangeVar = nullptr;
+
LexicalScope(const LexicalScope &) = delete;
void operator=(const LexicalScope &) = delete;
@@ -1004,6 +1020,21 @@ class CodeGenFunction : public CodeGenTypeCache {
Labels.push_back(label);
}
+ void addForRangeInitTemp(const MaterializeTemporaryExpr *M,
+ RawAddress Object) {
+ assert(PerformCleanup && "adding temps to dead scope?");
+ ForRangeInitTemps.push_back({M, Object});
+ }
+
+ ArrayRef<ForRangeInitLifetimeExtendTemporary> getForRangeInitTemps() const {
+ return ForRangeInitTemps;
+ }
+
+ void setForRangeVar(const VarDecl *Var) { ForRangeVar = Var; }
+ bool isExtendedByForRangeVar(const MaterializeTemporaryExpr *M) const {
+ return M && ForRangeVar && M->getExtendingDecl() == ForRangeVar;
+ }
+
/// Exit this cleanup scope, emitting any accumulated
/// cleanups.
~LexicalScope() {
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 0aa14b0510746b..ae5d2cae06e536 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -2379,10 +2379,6 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
if (getLangOpts().CPlusPlus23) {
auto &LastRecord = Actions.ExprEvalContexts.back();
LastRecord.InLifetimeExtendingContext = true;
-
- // Materialize non-`cv void` prvalue temporaries in discarded
- // expressions. These materialized temporaries may be lifetime-extented.
- LastRecord.InMaterializeTemporaryObjectContext = true;
}
if (getLangOpts().OpenMP)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ffe7e44e2387f4..2caf8a3a3c8e83 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6331,7 +6331,6 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
// Pass down lifetime extending flag, and collect temporaries in
// CreateMaterializeTemporaryExpr when we rewrite the call argument.
keepInLifetimeExtendingContext();
- keepInMaterializeTemporaryObjectContext();
EnsureImmediateInvocationInDefaultArgs Immediate(*this);
ExprResult Res;
runWithSufficientStackSpace(CallLoc, [&] {
@@ -6377,7 +6376,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
Expr *Init = nullptr;
bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();
-
+ bool InLifetimeExtendingContext = isInLifetimeExtendingContext();
EnterExpressionEvaluationContext EvalContext(
*this, ExpressionEvaluationContext::PotentiallyEvaluated, Field);
@@ -6412,19 +6411,21 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
ImmediateCallVisitor V(getASTContext());
if (!NestedDefaultChecking)
V.TraverseDecl(Field);
- if (V.HasImmediateCalls) {
+ if (V.HasImmediateCalls || InLifetimeExtendingContext) {
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.
+ keepInLifetimeExtendingContext();
EnsureImmediateInvocationInDefaultArgs Immediate(*this);
ExprResult Res;
runWithSufficientStackSpace(Loc, [&] {
Res = Immediate.TransformInitializer(Field->getInClassInitializer(),
/*CXXDirectInit=*/false);
});
- if (!Res.isInvalid())
+ if (Res.isUsable())
Res = ConvertMemberDefaultInitExpression(Field, Res.get(), Loc);
if (Res.isInvalid()) {
Field->setInvalidDecl();
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index db84f181012268..9f13e3827c742d 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -8361,7 +8361,7 @@ ExprResult Sema::IgnoredValueConversions(Expr *E) {
// unnecessary temporary objects. If we skip this step, IR generation is
// able to synthesize the storage for itself in the aggregate case, and
// adding the extra node to the AST is just clutter.
- if (isInMaterializeTemporaryObjectContext() && getLangOpts().CPlusPlus17 &&
+ if (isInLifetimeExtendingContext() && getLangOpts().CPlusPlus17 &&
E->isPRValue() && !E->getType()->isVoidType()) {
ExprResult Res = TemporaryMaterializationConversion(E);
if (Res.isInvalid())
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index a75e9925a43146..c2269206988efc 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -710,6 +710,26 @@ void InitListChecker::FillInEmptyInitForField(unsigned Init, FieldDecl *Field,
if (VerifyOnly)
return;
+ // Enter a lifetime extension context, then we can support lifetime
+ // extension of temporary created by aggregate initialization using a
+ // default member initializer (DR1815 https://wg21.link/CWG1815).
+ //
+ // In a lifetime extension context, BuildCXXDefaultInitExpr will clone the
+ // initializer expression on each use that would lifetime extend its
+ // temporaries.
+ EnterExpressionEvaluationContext LifetimeExtensionContext(
+ SemaRef, Sema::ExpressionEvaluationContext::PotentiallyEvaluated,
+ /*LambdaContextDecl=*/nullptr,
+ Sema::ExpressionEvaluationContextRecord::EK_Other, true);
+
+ // Lifetime extension in default-member-init.
+ auto &LastRecord = SemaRef.ExprEvalContexts.back();
+
+ // Just copy previous record, make sure we haven't forget anything.
+ LastRecord =
+ SemaRef.ExprEvalContexts[SemaRef.ExprEvalContexts.size() - 2];
+ LastRecord.InLifetimeExtendingContext = true;
+
ExprResult DIE = SemaRef.BuildCXXDefaultInitExpr(Loc, Field);
if (DIE.isInvalid()) {
hadError = true;
@@ -7699,6 +7719,8 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
// Step into CXXDefaultInitExprs so we can diagnose cases where a
// constructor inherits one as an implicit mem-initializer.
if (auto *DIE = dyn_cast<CXXDefaultInitExpr>(Init)) {
+ assert(DIE->hasRewrittenInit() &&
+ "CXXDefaultInitExpr must has rewritten init");
Path.push_back(
{IndirectLocalPathEntry::DefaultInit, DIE, DIE->getField()});
Init = DIE->getExpr();
@@ -8194,24 +8216,12 @@ void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
switch (shouldLifetimeExtendThroughPath(Path)) {
case PathLifetimeKind::Extend:
+ case PathLifetimeKind::ShouldExtend:
// Update the storage duration of the materialized temporary.
- // FIXME: Rebuild the expression instead of mutating it.
MTE->setExtendingDecl(ExtendingEntity->getDecl(),
ExtendingEntity->allocateManglingNumber());
- // Also visit the temporaries lifetime-extended by this initializer.
return true;
- case PathLifetimeKind::ShouldExtend:
- // We're supposed to lifetime-extend the temporary along this path (per
- // the resolution of DR1815), but we don't support that yet.
- //
- // FIXME: Properly handle this situation. Perhaps the easiest approach
- // would be to clone the initializer expression on each use that would
- // lifetime extend its temporaries.
- Diag(DiagLoc, diag::warn_unsupported_lifetime_extension)
- << RK << DiagRange;
- break;
-
case PathLifetimeKind::NoExtend:
// If the path goes through the initialization of a variable or field,
// it can't possibly reach a temporary created in this full-expression.
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 1cb071e4eb7d1c..15d0b8a69bcd2c 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5477,7 +5477,6 @@ void Sema::InstantiateVariableInitializer(
*this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, Var);
keepInLifetimeExtendingContext();
- keepInMaterializeTemporaryObjectContext();
// Instantiate the initializer.
ExprResult Init;
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 7df352c24e8648..818837064a38b7 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransfo...
[truncated]
|
yronglin
force-pushed
the
extend_default_member_init
branch
from
55f6fe0 to
e5f2bfb
Compare
yronglin
commented
Apr 6, 2024
clang/lib/CodeGen/CGStmt.cpp
Outdated
| @@ -1230,11 +1230,26 @@ CodeGenFunction::EmitCXXForRangeStmt(const CXXForRangeStmt &S, | |||
| JumpDest LoopExit = getJumpDestInCurrentScope("for.end"); | |||
|
|
|||
| LexicalScope ForScope(*this, S.getSourceRange()); | |||
| const DeclStmt *RangeDS = cast<DeclStmt>(S.getRangeStmt()); | |||
| const VarDecl *RangeVar = cast<VarDecl>(RangeDS->getSingleDecl()); | |||
| if (getLangOpts().CPlusPlus23) | |||
There was a problem hiding this comment.
I think we need to continue to improve here. We can decide whether to extend the lifetime in Sema. For example: do not store temporaries into ExpressionEvaluationContext::ForRangeLifetimeExtendTemps when building for-range-init expressions.
There was a problem hiding this comment.
What makes temporaries in for-range statements different from other temporaries? The standard explicitly defines for-range in terms of a regular for-loop; it shouldn't require any special handling unless our AST is somehow deficient.
There was a problem hiding this comment.
Thanks for your review! P2718R0 introduce lifetime extensions in for-range loops. Basic support for P2718R0 has been implemented in #76361 and everything works fine. When I works on this PR, I found that the lifetime of temporaries inCXXDefaultInitExpr was not extended, even if MaterializedTemporaryExpr sets ExtendingDecl and bound to __range variable. So I thought about my current approach and I'm not sure if this is the correct approach. Can you provide a direction for improvement? Should we modify the AST instead of handling it in CodeGen?
There was a problem hiding this comment.
for (auto &&x : B{A()}) {} currently works correctly, as far as I know (where B is a class with a member of type const A&). And if I'm understanding correctly, for (auto &&x : B{}) {} is supposed to be semantically equivalent. If the AST is significantly different, you should be addressing that in Sema, I think.
There was a problem hiding this comment.
In InitListChecker::FillInEmptyInitForField, we will create a CXXDefaultInitExpr and fill in empty field const A a& = A{}; for for (auto &&x : B{}) {} . Also there will have a ExprWithCleanup in CXXDefaultInitExpr.
This is what I know about the difference in AST between the two cases, when I change the ExtendingDecl of MaterializedTemporaryExpr, it does not generate correct cleanups in CodeGen.
There was a problem hiding this comment.
If you think "A temporary expression bound to a reference member from a default member initializer is ill-formed" doesn't refer to this case, what does it refer to?
There was a problem hiding this comment.
The quoted rule is [class.base.init]/11, and in that context is describing the behavior of the initialization of class members in a constructor.
Aggregate initialization is handled by the general rule in [class.temporary] but see also the note in [dcl.init.general] that explicitly mentions this case.
There was a problem hiding this comment.
Oh, so it's actually supposed to say "If a temporary expression is bound to a reference member from a default member initializer, and that member initializer is used by a constructor, the constructor is ill-formed" or something like that? I guess that makes sense in context, but the language could be improved.
Back to the issue we were discussing here, when are the destructors for non-lifetime-extended temporaries supposed to run? If they're supposed to stay live for the whole expression, probably the rewriting code in Sema should just rewrite out the ExprWithCleanups.
If they're supposed to be destroyed, you might need to add a special-case to ExprWithCleanups handling to special-case the cleanups for the temporaries that are supposed to be lifetime-extended. (If you're going to touch this code, make sure you're working on latest main, since #85398 landed recently.)
There was a problem hiding this comment.
The AST in the
B{A()}case looks like the following. If your patch changes the AST forB{}to look the same, it should be fine, I think? From your description, I'm not exactly sure what's different about what CXXDefaultInitExpr rewriting generates.| `-VarDecl 0xc4927c0 <col:113, col:118> col:113 implicit used __range1 'B &&' cinit | `-ExprWithCleanups 0xc492aa8 <col:113, col:118> 'B' xvalue | `-MaterializeTemporaryExpr 0xc492a00 <col:113, col:118> 'B' xvalue extended by Var 0xc4927c0 '__range1' 'B &&' | `-CXXFunctionalCastExpr 0xc492678 <col:113, col:118> 'B' functional cast to B <NoOp> | `-InitListExpr 0xc492508 <col:114, col:118> 'B' | `-MaterializeTemporaryExpr 0xc492568 <col:115, col:117> 'const A' lvalue extended by Var 0xc4927c0 '__range1' 'B &&' | `-ImplicitCastExpr 0xc492550 <col:115, col:117> 'const A' <NoOp> | `-CXXBindTemporaryExpr 0xc4718c8 <col:115, col:117> 'A' (CXXTemporary 0xc4718c8) | `-CXXTemporaryObjectExpr 0xc471890 <col:115, col:117> 'A' 'void ()'
I have addressed this suggestion, seems it's works fine now, and avoid the changes in CodeGen.
yronglin
commented
Apr 6, 2024
clang/lib/CodeGen/CGStmt.cpp
Outdated
| @@ -1230,11 +1230,26 @@ CodeGenFunction::EmitCXXForRangeStmt(const CXXForRangeStmt &S, | |||
| JumpDest LoopExit = getJumpDestInCurrentScope("for.end"); | |||
|
|
|||
| LexicalScope ForScope(*this, S.getSourceRange()); | |||
| const DeclStmt *RangeDS = cast<DeclStmt>(S.getRangeStmt()); | |||
| const VarDecl *RangeVar = cast<VarDecl>(RangeDS->getSingleDecl()); | |||
| if (getLangOpts().CPlusPlus23) | |||
There was a problem hiding this comment.
What makes temporaries in for-range statements different from other temporaries? The standard explicitly defines for-range in terms of a regular for-loop; it shouldn't require any special handling unless our AST is somehow deficient.
|
This PR does so many things that I will split it into multiple smaller PRs to make it easier to review. |
yronglin
added a commit
that referenced
this pull request
May 23, 2024
#93207) This issue was found in #86960. But I'd like to avoid mixing together a bunch of cleanups with actual changes. Signed-off-by: yronglin <[email protected]>
|
Is this still relevant? What about #85613 ? |
… P2718R0 Signed-off-by: yronglin <[email protected]>
yronglin
force-pushed
the
extend_default_member_init
branch
from
ee028de to
a8a8748
Compare
Signed-off-by: yronglin <[email protected]>
|
friendly ping~ |
cor3ntin
approved these changes
Oct 3, 2024
Signed-off-by: yronglin <[email protected]>
Thanks for the review, added ReleaseNotes. |
|
I plan to merge in 24 hours if there are no further comments. |
h-vetinari
reviewed
Oct 10, 2024
yronglin
added a commit
that referenced
this pull request
Oct 11, 2024
Fix a typo in ReleaseNotes that introduced by #86960. Signed-off-by: yronglin <[email protected]>
DanielCChen
pushed a commit
to DanielCChen/llvm-project
that referenced
this pull request
Oct 16, 2024
llvm#86960) Depends on [CWG1815](llvm#108039). Fixes llvm#85613. In [[Clang] Implement P2718R0 "Lifetime extension in range-based for loops"](llvm#76361), we've not implement the lifetime extensions for the temporaries which in `CXXDefaultInitExpr`. As the confirmation in llvm#85613, we should extend lifetime for that. To avoid modifying current CodeGen rules, in a lifetime extension context, the cleanup of `CXXDefaultInitExpr` was ignored. --------- Signed-off-by: yronglin <[email protected]>
DanielCChen
pushed a commit
to DanielCChen/llvm-project
that referenced
this pull request
Oct 16, 2024
Fix a typo in ReleaseNotes that introduced by llvm#86960. Signed-off-by: yronglin <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on CWG1815.
Fixes #85613.
In [Clang] Implement P2718R0 "Lifetime extension in range-based for loops", we've not implement the lifetime extensions for the temporaries which in
CXXDefaultInitExpr. As the confirmation in #85613, we should extend lifetime for that.To avoid modifying current CodeGen rules, in a lifetime extension context, the cleanup of
CXXDefaultInitExprwas ignored.