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 3 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 @@ -78,6 +78,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++17 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 @@ -10148,13 +10148,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
18 changes: 1 addition & 17 deletions clang/lib/Sema/CheckExprLifetime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -912,11 +912,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 @@ -928,7 +923,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 @@ -1060,17 +1055,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
33 changes: 25 additions & 8 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5416,6 +5416,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 @@ -5493,10 +5495,9 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
Res = Immediate.TransformInitializer(Param->getInit(),
/*NotCopy=*/false);
});
if (Res.isInvalid())
return ExprError();
Res = ConvertParamDefaultArgument(Param, Res.get(),
Res.get()->getBeginLoc());
if (Res.isUsable())
Res = ConvertParamDefaultArgument(Param, Res.get(),
Res.get()->getBeginLoc());
if (Res.isInvalid())
return ExprError();
Init = Res.get();
Expand Down Expand Up @@ -5532,7 +5533,7 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
Expr *Init = nullptr;

bool NestedDefaultChecking = isCheckingDefaultArgumentOrInitializer();

bool InLifetimeExtendingContext = isInLifetimeExtendingContext();
EnterExpressionEvaluationContext EvalContext(
*this, ExpressionEvaluationContext::PotentiallyEvaluated, Field);

Expand Down Expand Up @@ -5567,19 +5568,35 @@ 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 always rebuild
// the initializer if it contains any temporaries (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 (V.HasImmediateCalls || InLifetimeExtendingContext ||
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.
keepInLifetimeExtendingContext();
EnsureImmediateInvocationInDefaultArgs Immediate(*this);
ExprResult Res;

// Rebuild CXXDefaultInitExpr might cause diagnostics.
SFINAETrap Trap(*this);
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();
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
12 changes: 8 additions & 4 deletions clang/lib/Sema/TreeTransform.h
Original file line number Diff line number Diff line change
Expand Up @@ -14402,6 +14402,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 @@ -14413,12 +14420,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
2 changes: 0 additions & 2 deletions clang/test/CXX/drs/cwg16xx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -483,8 +483,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
19 changes: 14 additions & 5 deletions clang/test/CXX/drs/cwg18xx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -206,19 +206,28 @@ namespace cwg1814 { // cwg1814: yes
#endif
}

namespace cwg1815 { // cwg1815: no
namespace cwg1815 { // cwg1815: 19
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be // cwg1815: 20 I think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be the case, yeah.
From what I see, this effort was started back before 19 has branched, and was never backported to 19 branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I missed this! I'll fix it. Thank you for point out this.

#if __cplusplus >= 201402L
// FIXME: needs codegen test
struct A { int &&r = 0; }; // #cwg1815-A
struct A { int &&r = 0; };
A a = {};
// 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}} FIXME
// since-cxx14-note@#cwg1815-A {{initializing field 'r' with default member initializer}}

struct B { int &&r = 0; }; // #cwg1815-B
// since-cxx14-error@-1 {{reference member 'r' binds to a temporary object whose lifetime would be shorter than the lifetime of the constructed object}}
// since-cxx14-note@#cwg1815-B {{initializing field 'r' with default member initializer}}
// since-cxx14-note@#cwg1815-b {{in implicit default constructor for 'cwg1815::B' first required here}}
B b; // #cwg1815-b

#if __cplusplus >= 201703L
struct C { const int &r = 0; };
constexpr C c = {}; // OK, since cwg1815
static_assert(c.r == 0);

constexpr int f() {
A a = {}; // OK, since cwg1815
return a.r;
}
static_assert(f() == 0);
#endif
#endif
}

Expand Down
34 changes: 34 additions & 0 deletions clang/test/CXX/special/class.temporary/p6.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,40 @@ void init_capture_init_list() {
// CHECK: }
}

void check_dr1815() { // dr1815: yes
#if __cplusplus >= 201402L

struct A {
int &&r = 0;
~A() {}
};

struct B {
A &&a = A{};
~B() {}
};
B a = {};

// CHECK: call {{.*}}block_scope_begin_function
extern void block_scope_begin_function();
extern void block_scope_end_function();
block_scope_begin_function();
{
// CHECK: call void @_ZZ12check_dr1815vEN1BD1Ev
// CHECK: call void @_ZZ12check_dr1815vEN1AD1Ev
B b = {};
}
// CHECK: call {{.*}}block_scope_end_function
block_scope_end_function();

// CHECK: call {{.*}}some_other_function
extern void some_other_function();
some_other_function();
// CHECK: call void @_ZZ12check_dr1815vEN1BD1Ev
// CHECK: call void @_ZZ12check_dr1815vEN1AD1Ev
#endif
}

namespace P2718R0 {
namespace basic {
template <typename E> using T2 = std::list<E>;
Expand Down
4 changes: 2 additions & 2 deletions clang/test/SemaCXX/constexpr-default-arg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ void test_default_arg2() {
}

// Check that multiple CXXDefaultInitExprs don't cause an assertion failure.
struct A { int &&r = 0; }; // expected-note 2{{default member initializer}}
struct A { int &&r = 0; };
struct B { A x, y; };
B b = {}; // expected-warning 2{{lifetime extension of temporary created by aggregate initialization using a default member initializer is not yet supported}}
B b = {}; // expected-no-diagnostics

}
Loading
Loading