Skip to content

Reapply "[Clang][CWG1815] Support lifetime extension of temporary created by aggregate initialization using a default member initializer" #97308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Sep 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ C++ Language Changes
- Allow single element access of GCC vector/ext_vector_type object to be
constant expression. Supports the `V.xyzw` syntax and other tidbits
as seen in OpenCL. Selecting multiple elements is left as a future work.
- Implement `CWG1815 <https://wg21.link/CWG1815>`_. Support lifetime extension
of temporary created by aggregate initialization using a default member
initializer.

C++2c Feature Support
^^^^^^^^^^^^^^^^^^^^^
Expand Down
7 changes: 0 additions & 7 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -10159,13 +10159,6 @@ def warn_dangling_pointer_assignment : Warning<
"will be destroyed at the end of the full-expression">,
InGroup<DanglingAssignment>;

def warn_unsupported_lifetime_extension : Warning<
"lifetime extension of "
"%select{temporary|backing array of initializer list}0 created "
"by aggregate initialization using a default member initializer "
"is not yet supported; lifetime of %select{temporary|backing array}0 "
"will end at the end of the full-expression">, InGroup<Dangling>;

// For non-floating point, expressions of the form x == x or x != x
// should result in a warning, since these always evaluate to a constant.
// Array comparisons have similar warnings
Expand Down
23 changes: 8 additions & 15 deletions clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -6382,6 +6382,9 @@ class Sema final : public SemaBase {
/// example, in a for-range initializer).
bool InLifetimeExtendingContext = false;

/// Whether we should rebuild CXXDefaultArgExpr and CXXDefaultInitExpr.
bool RebuildDefaultArgOrDefaultInit = 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
Expand Down Expand Up @@ -7802,9 +7805,11 @@ class Sema final : public SemaBase {
}

bool isInLifetimeExtendingContext() const {
assert(!ExprEvalContexts.empty() &&
"Must be in an expression evaluation context");
return ExprEvalContexts.back().InLifetimeExtendingContext;
return currentEvaluationContext().InLifetimeExtendingContext;
}

bool needRebuildDefaultArgOrInit() const {
return currentEvaluationContext().RebuildDefaultArgOrDefaultInit;
}

bool isCheckingDefaultArgumentOrInitializer() const {
Expand Down Expand Up @@ -7846,18 +7851,6 @@ class Sema final : public SemaBase {
return Res;
}

/// keepInLifetimeExtendingContext - Pull down InLifetimeExtendingContext
/// flag from previous context.
void keepInLifetimeExtendingContext() {
if (ExprEvalContexts.size() > 2 &&
parentEvaluationContext().InLifetimeExtendingContext) {
auto &LastRecord = ExprEvalContexts.back();
auto &PrevRecord = parentEvaluationContext();
LastRecord.InLifetimeExtendingContext =
PrevRecord.InLifetimeExtendingContext;
}
}

DefaultedComparisonKind getDefaultedComparisonKind(const FunctionDecl *FD) {
return getDefaultedFunctionKind(FD).asComparison();
}
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2484,8 +2484,9 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,

// P2718R0 - Lifetime extension in range-based for loops.
if (getLangOpts().CPlusPlus23) {
auto &LastRecord = Actions.ExprEvalContexts.back();
auto &LastRecord = Actions.currentEvaluationContext();
LastRecord.InLifetimeExtendingContext = true;
LastRecord.RebuildDefaultArgOrDefaultInit = true;
}

if (getLangOpts().OpenMP)
Expand Down
18 changes: 1 addition & 17 deletions clang/lib/Sema/CheckExprLifetime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -895,11 +895,6 @@ static void visitLocalsRetainedByInitializer(IndirectLocalPath &Path,
enum PathLifetimeKind {
/// Lifetime-extend along this path.
Extend,
/// We should lifetime-extend, but we don't because (due to technical
/// limitations) we can't. This happens for default member initializers,
/// which we don't clone for every use, so we don't have a unique
/// MaterializeTemporaryExpr to update.
ShouldExtend,
/// Do not lifetime extend along this path.
NoExtend
};
Expand All @@ -911,7 +906,7 @@ shouldLifetimeExtendThroughPath(const IndirectLocalPath &Path) {
PathLifetimeKind Kind = PathLifetimeKind::Extend;
for (auto Elem : Path) {
if (Elem.Kind == IndirectLocalPathEntry::DefaultInit)
Kind = PathLifetimeKind::ShouldExtend;
return PathLifetimeKind::Extend;
else if (Elem.Kind != IndirectLocalPathEntry::LambdaCaptureInit)
return PathLifetimeKind::NoExtend;
}
Expand Down Expand Up @@ -1043,17 +1038,6 @@ static void checkExprLifetimeImpl(Sema &SemaRef,
// 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.
SemaRef.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.
Expand Down
36 changes: 27 additions & 9 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5427,6 +5427,8 @@ struct EnsureImmediateInvocationInDefaultArgs
EnsureImmediateInvocationInDefaultArgs(Sema &SemaRef)
: TreeTransform(SemaRef) {}

bool AlwaysRebuild() { return true; }

// Lambda can only have immediate invocations in the default
// args of their parameters, which is transformed upon calling the closure.
// The body is not a subexpression, so we have nothing to do.
Expand Down Expand Up @@ -5455,7 +5457,7 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
assert(Param->hasDefaultArg() && "can't build nonexistent default arg");

bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();
bool InLifetimeExtendingContext = isInLifetimeExtendingContext();
bool NeedRebuild = needRebuildDefaultArgOrInit();
std::optional<ExpressionEvaluationContextRecord::InitializationContext>
InitializationContext =
OutermostDeclarationWithDelayedImmediateInvocations();
Expand Down Expand Up @@ -5491,13 +5493,15 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,

// Rewrite the call argument that was created from the corresponding
// parameter's default argument.
if (V.HasImmediateCalls || InLifetimeExtendingContext) {
if (V.HasImmediateCalls ||
(NeedRebuild && isa_and_present<ExprWithCleanups>(Param->getInit()))) {
if (V.HasImmediateCalls)
ExprEvalContexts.back().DelayedDefaultInitializationContext = {
CallLoc, Param, CurContext};
// Pass down lifetime extending flag, and collect temporaries in
// CreateMaterializeTemporaryExpr when we rewrite the call argument.
keepInLifetimeExtendingContext();
currentEvaluationContext().InLifetimeExtendingContext =
parentEvaluationContext().InLifetimeExtendingContext;
EnsureImmediateInvocationInDefaultArgs Immediate(*this);
ExprResult Res;
runWithSufficientStackSpace(CallLoc, [&] {
Expand Down Expand Up @@ -5543,7 +5547,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
Expr *Init = nullptr;

bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();

bool NeedRebuild = needRebuildDefaultArgOrInit();
EnterExpressionEvaluationContext EvalContext(
*this, ExpressionEvaluationContext::PotentiallyEvaluated, Field);

Expand Down Expand Up @@ -5578,12 +5582,27 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
ImmediateCallVisitor V(getASTContext());
if (!NestedDefaultChecking)
V.TraverseDecl(Field);
if (V.HasImmediateCalls) {

// CWG1815
// Support lifetime extension of temporary created by aggregate
// initialization using a default member initializer. We should rebuild
// the initializer in a lifetime extension context if the initializer
// 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() &&
(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.
currentEvaluationContext().InLifetimeExtendingContext =
parentEvaluationContext().InLifetimeExtendingContext;
EnsureImmediateInvocationInDefaultArgs Immediate(*this);
ExprResult Res;
runWithSufficientStackSpace(Loc, [&] {
Expand Down Expand Up @@ -17635,11 +17654,10 @@ void Sema::PopExpressionEvaluationContext() {

// Append the collected materialized temporaries into previous context before
// exit if the previous also is a lifetime extending context.
auto &PrevRecord = parentEvaluationContext();
if (getLangOpts().CPlusPlus23 && Rec.InLifetimeExtendingContext &&
PrevRecord.InLifetimeExtendingContext &&
parentEvaluationContext().InLifetimeExtendingContext &&
!Rec.ForRangeLifetimeExtendTemps.empty()) {
PrevRecord.ForRangeLifetimeExtendTemps.append(
parentEvaluationContext().ForRangeLifetimeExtendTemps.append(
Rec.ForRangeLifetimeExtendTemps);
}

Expand Down
3 changes: 0 additions & 3 deletions clang/lib/Sema/SemaExprCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1539,9 +1539,6 @@ Sema::BuildCXXTypeConstructExpr(TypeSourceInfo *TInfo,
bool ListInitialization) {
QualType Ty = TInfo->getType();
SourceLocation TyBeginLoc = TInfo->getTypeLoc().getBeginLoc();

assert((!ListInitialization || Exprs.size() == 1) &&
"List initialization must have exactly one expression.");
SourceRange FullRange = SourceRange(TyBeginLoc, RParenOrBraceLoc);

InitializedEntity Entity =
Expand Down
22 changes: 16 additions & 6 deletions clang/lib/Sema/SemaInit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -750,8 +750,20 @@ void InitListChecker::FillInEmptyInitForField(unsigned Init, FieldDecl *Field,
if (Field->hasInClassInitializer()) {
if (VerifyOnly)
return;

ExprResult DIE = SemaRef.BuildCXXDefaultInitExpr(Loc, Field);
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);
// Just copy previous record, make sure we haven't forget anything.
SemaRef.currentEvaluationContext() = SemaRef.parentEvaluationContext();
SemaRef.currentEvaluationContext().RebuildDefaultArgOrDefaultInit =
true;
DIE = SemaRef.BuildCXXDefaultInitExpr(Loc, Field);
}
if (DIE.isInvalid()) {
hadError = true;
return;
Expand Down Expand Up @@ -7475,10 +7487,8 @@ Sema::CreateMaterializeTemporaryExpr(QualType T, Expr *Temporary,
// are done in both CreateMaterializeTemporaryExpr and MaybeBindToTemporary,
// but there may be a chance to merge them.
Cleanup.setExprNeedsCleanups(false);
if (isInLifetimeExtendingContext()) {
auto &Record = ExprEvalContexts.back();
Record.ForRangeLifetimeExtendTemps.push_back(MTE);
}
if (isInLifetimeExtendingContext())
currentEvaluationContext().ForRangeLifetimeExtendTemps.push_back(MTE);
return MTE;
}

Expand Down
5 changes: 4 additions & 1 deletion clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5478,7 +5478,10 @@ void Sema::InstantiateVariableInitializer(
EnterExpressionEvaluationContext Evaluated(
*this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated, Var);

keepInLifetimeExtendingContext();
currentEvaluationContext().InLifetimeExtendingContext =
parentEvaluationContext().InLifetimeExtendingContext;
currentEvaluationContext().RebuildDefaultArgOrDefaultInit =
parentEvaluationContext().RebuildDefaultArgOrDefaultInit;
// Instantiate the initializer.
ExprResult Init;

Expand Down
20 changes: 14 additions & 6 deletions clang/lib/Sema/TreeTransform.h
Original file line number Diff line number Diff line change
Expand Up @@ -4254,7 +4254,10 @@ ExprResult TreeTransform<Derived>::TransformInitializer(Expr *Init,
getSema(), EnterExpressionEvaluationContext::InitList,
Construct->isListInitialization());

getSema().keepInLifetimeExtendingContext();
getSema().currentEvaluationContext().InLifetimeExtendingContext =
getSema().parentEvaluationContext().InLifetimeExtendingContext;
getSema().currentEvaluationContext().RebuildDefaultArgOrDefaultInit =
getSema().parentEvaluationContext().RebuildDefaultArgOrDefaultInit;
SmallVector<Expr*, 8> NewArgs;
bool ArgChanged = false;
if (getDerived().TransformExprs(Construct->getArgs(), Construct->getNumArgs(),
Expand Down Expand Up @@ -8893,8 +8896,9 @@ TreeTransform<Derived>::TransformCXXForRangeStmt(CXXForRangeStmt *S) {

// P2718R0 - Lifetime extension in range-based for loops.
if (getSema().getLangOpts().CPlusPlus23) {
auto &LastRecord = getSema().ExprEvalContexts.back();
auto &LastRecord = getSema().currentEvaluationContext();
LastRecord.InLifetimeExtendingContext = true;
LastRecord.RebuildDefaultArgOrDefaultInit = true;
}
StmtResult Init =
S->getInit() ? getDerived().TransformStmt(S->getInit()) : StmtResult();
Expand Down Expand Up @@ -14412,6 +14416,13 @@ TreeTransform<Derived>::TransformCXXTemporaryObjectExpr(
if (TransformExprs(E->getArgs(), E->getNumArgs(), true, Args,
&ArgumentChanged))
return ExprError();

if (E->isListInitialization() && !E->isStdInitListInitialization()) {
ExprResult Res = RebuildInitList(E->getBeginLoc(), Args, E->getEndLoc());
if (Res.isInvalid())
return ExprError();
Args = {Res.get()};
}
}

if (!getDerived().AlwaysRebuild() &&
Expand All @@ -14423,12 +14434,9 @@ TreeTransform<Derived>::TransformCXXTemporaryObjectExpr(
return SemaRef.MaybeBindToTemporary(E);
}

// FIXME: We should just pass E->isListInitialization(), but we're not
// prepared to handle list-initialization without a child InitListExpr.
SourceLocation LParenLoc = T->getTypeLoc().getEndLoc();
return getDerived().RebuildCXXTemporaryObjectExpr(
T, LParenLoc, Args, E->getEndLoc(),
/*ListInitialization=*/LParenLoc.isInvalid());
T, LParenLoc, Args, E->getEndLoc(), E->isListInitialization());
}

template<typename Derived>
Expand Down
6 changes: 3 additions & 3 deletions clang/test/AST/ast-dump-default-init-json.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -789,10 +789,10 @@ void test() {
// CHECK-NEXT: "valueCategory": "lvalue",
// CHECK-NEXT: "extendingDecl": {
// CHECK-NEXT: "id": "0x{{.*}}",
// CHECK-NEXT: "kind": "FieldDecl",
// CHECK-NEXT: "name": "a",
// CHECK-NEXT: "kind": "VarDecl",
// CHECK-NEXT: "name": "b",
// CHECK-NEXT: "type": {
// CHECK-NEXT: "qualType": "const A &"
// CHECK-NEXT: "qualType": "B"
// CHECK-NEXT: }
// CHECK-NEXT: },
// CHECK-NEXT: "storageDuration": "automatic",
Expand Down
2 changes: 1 addition & 1 deletion clang/test/AST/ast-dump-default-init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ void test() {
}
// 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 Field 0x{{[^ ]*}} 'a' 'const A &'
// 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'
Expand Down
9 changes: 5 additions & 4 deletions clang/test/Analysis/lifetime-extended-regions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,11 @@ void aggregateWithReferences() {
clang_analyzer_dump(viaReference); // expected-warning-re {{&lifetime_extended_object{RefAggregate, viaReference, S{{[0-9]+}}} }}
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]+}}} }}

// clang does not currently implement extending lifetime of object bound to reference members of aggregates,
// that are created from default member initializer (see `warn_unsupported_lifetime_extension` from `-Wdangling`)
RefAggregate defaultInitExtended{i}; // clang-bug does not extend `Composite`

// 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]+}}} }}
RefAggregate defaultInitExtended{i};
clang_analyzer_dump(defaultInitExtended.ry); // expected-warning {{Unknown }}
}

Expand Down
23 changes: 21 additions & 2 deletions clang/test/CXX/drs/cwg16xx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,27 @@ namespace cwg1696 { // cwg1696: 7
// since-cxx14-note@-2 {{default member initializer declared here}}
};
A a{a, a};

struct A1 {
A1() : v(42) {}
// since-cxx14-error@-1 {{reference member 'v' binds to a temporary object whose lifetime would be shorter than the lifetime of the constructed object}}
// since-cxx14-note@#cwg1696-A1 {{reference member declared here}}
const int &v; // #cwg1696-A1
};

struct A2 {
A2() = default;
// since-cxx14-error@-1 {{reference member 'v' binds to a temporary object whose lifetime would be shorter than the lifetime of the constructed object}}
// since-cxx14-note-re@#cwg1696-A2-b {{in defaulted default constructor for {{.*}} first required here}}
// since-cxx14-note@#cwg1696-A2-a {{initializing field 'v' with default member initializer}}
A2(int v) : v(v) {}
// since-cxx14-warning@-1 {{binding reference member 'v' to stack allocated parameter 'v'}}
// since-cxx14-note@#cwg1696-A2-a {{reference member declared here}}
const int &v = 42; // #cwg1696-A2-a
};
A2 a1; // #cwg1696-A2-b

A2 a2(1); // OK, unfortunately
#endif
}

Expand Down Expand Up @@ -483,8 +504,6 @@ namespace cwg1696 { // cwg1696: 7
const A &a = A(); // #cwg1696-D1-a
};
D1 d1 = {}; // #cwg1696-d1
// since-cxx14-warning@-1 {{lifetime extension of temporary created by aggregate initialization using a default member initializer is not yet supported; lifetime of temporary will end at the end of the full-expression}}
// since-cxx14-note@#cwg1696-D1-a {{initializing field 'a' with default member initializer}}

struct D2 {
const A &a = A(); // #cwg1696-D2-a
Expand Down
Loading