-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Reapply "[Clang][CWG1815] Support lifetime extension of temporary created by aggregate initialization using a default member initializer" #92527
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14102,6 +14102,13 @@ TreeTransform<Derived>::TransformCXXTemporaryObjectExpr( | |
if (TransformExprs(E->getArgs(), E->getNumArgs(), true, Args, | ||
&ArgumentChanged)) | ||
return ExprError(); | ||
|
||
if (E->isListInitialization() && !E->isStdInitListInitialization()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it is list initialization, but the constructor is called, and it is not the case of std::initializer_list, we should recreate an InitListExpr. https://godbolt.org/z/K4rzxosx3 |
||
ExprResult Res = RebuildInitList(E->getBeginLoc(), Args, E->getEndLoc()); | ||
if (Res.isInvalid()) | ||
return ExprError(); | ||
Args = {Res.get()}; | ||
} | ||
} | ||
|
||
if (!getDerived().AlwaysRebuild() && | ||
|
@@ -14113,12 +14120,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()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The issue in #87933 caused by |
||
} | ||
|
||
template<typename Derived> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,80 @@ class MemInit { | |
C m = s; | ||
}; | ||
|
||
namespace std { | ||
typedef decltype(sizeof(int)) size_t; | ||
|
||
// libc++'s implementation | ||
template <class _E> class initializer_list { | ||
const _E *__begin_; | ||
size_t __size_; | ||
|
||
initializer_list(const _E *__b, size_t __s) : __begin_(__b), __size_(__s) {} | ||
|
||
public: | ||
typedef _E value_type; | ||
typedef const _E &reference; | ||
typedef const _E &const_reference; | ||
typedef size_t size_type; | ||
|
||
typedef const _E *iterator; | ||
typedef const _E *const_iterator; | ||
|
||
initializer_list() : __begin_(nullptr), __size_(0) {} | ||
|
||
size_t size() const { return __size_; } | ||
const _E *begin() const { return __begin_; } | ||
const _E *end() const { return __begin_ + __size_; } | ||
}; | ||
} // namespace std | ||
|
||
#if __cplusplus >= 201703L | ||
namespace test_rebuild { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add more test that from #87933 's comments. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! |
||
template <typename T, int> class C { | ||
public: | ||
C(std::initializer_list<T>); | ||
}; | ||
|
||
template <typename T> using Ptr = __remove_pointer(T) *; | ||
template <typename T> C(T) -> C<Ptr<T>, sizeof(T)>; | ||
|
||
class A { | ||
public: | ||
template <typename T1, typename T2> T1 *some_func(T2 &&); | ||
}; | ||
|
||
struct B : A { | ||
// Test CXXDefaultInitExpr rebuild issue in | ||
// https://github.com/llvm/llvm-project/pull/87933 | ||
int *ar = some_func<int>(C{some_func<int>(0)}); | ||
B() {} | ||
}; | ||
|
||
int TestBody_got; | ||
template <int> class Vector { | ||
public: | ||
Vector(std::initializer_list<int>); | ||
}; | ||
template <typename... Ts> Vector(Ts...) -> Vector<sizeof...(Ts)>; | ||
class ProgramBuilder { | ||
public: | ||
template <typename T, typename ARGS> int *create(ARGS); | ||
}; | ||
|
||
struct TypeTest : ProgramBuilder { | ||
int *str_f16 = create<int>(Vector{0}); | ||
TypeTest() {} | ||
}; | ||
class TypeTest_Element_Test : TypeTest { | ||
void TestBody(); | ||
}; | ||
void TypeTest_Element_Test::TestBody() { | ||
int *expect = str_f16; | ||
&TestBody_got != expect; // expected-warning {{inequality comparison result unused}} | ||
} | ||
} // namespace test_rebuild | ||
#endif // __cplusplus >= 201703L | ||
|
||
#if __cplusplus >= 202002L | ||
// This test ensures cleanup expressions are correctly produced | ||
// in the presence of default member initializers. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we suppress diagnostic messages? Perhaps the initializer with errors should not be rebuilt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to detect if there is an error in the initializer? For example, access errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I get the question? The SFINAETrap IS suppressing at least some diagnostics.
We can see if there are already errors pre-transformation via
containsError
(or if it is nullptr, depending on RecoveryExpr). So if there was previously detected error, one of those should catch it.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your review! Here we add
SFINAETrap Trap(*this)
. It was used to resolve the issue which looks like https://github.com/llvm/llvm-project/blob/main/clang/test/CXX/dcl.decl/dcl.init/p14-0x.cpp .A shorter reproducer:
Field->getInClassInitializer()
contains anbad access error
, then we try to rebuild this initializer, the diagnostic will emit 3 times. (The result ofField->getInClassInitializer()->containsErrors()
3 times arefalse
).1st time: parser member initializer.
2nd time: After rebuild, Line 5726
Res = ConvertMemberDefaultInitExpression(Field, Res.get(), Loc);
3rd time: After rebuild, Line 5726
Res = ConvertMemberDefaultInitExpression(Field, Res.get(), Loc);