mizvekov created this revision.
mizvekov updated this revision to Diff 372616.
mizvekov added a comment.
mizvekov updated this revision to Diff 372828.
mizvekov retitled this revision from "test commit." to "[clang] don't mark as
Elidable CXXConstruct expressions used in NRVO".
mizvekov edited the summary of this revision.
Herald added a subscriber: lxfind.
mizvekov published this revision for review.
mizvekov added reviewers: rsmith, rjmccall.
mizvekov added a subscriber: Quuxplusone.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
.
mizvekov added a comment.
.
See PR51862.
The consumers of the Elidable flag in CXXConstructExpr assume that
an elidable construction just goes through a single copy/move construction,
so that the source object is immediately passed as an argument and is the same
type as the parameter itself.
With the implementation of P2266 <https://reviews.llvm.org/P2266> and after
some adjustments to the
implementation of P1825 <https://reviews.llvm.org/P1825>, we started
(correctly, as per standard)
allowing more cases where the copy initialization goes through
converting constructors and user defined conversions.
With this patch we stop using this flag in NRVO contexts, to preserve code
that relies on that assumption.
This causes no known functional changes, we just stop firing some asserts
in a cople of included test cases.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D109800
Files:
clang/include/clang/Sema/Initialization.h
clang/lib/CodeGen/CGExprCXX.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaCoroutine.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaLambda.cpp
clang/lib/Sema/SemaObjCProperty.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/CodeGen/nrvo-tracking.cpp
Index: clang/test/CodeGen/nrvo-tracking.cpp
===================================================================
--- clang/test/CodeGen/nrvo-tracking.cpp
+++ clang/test/CodeGen/nrvo-tracking.cpp
@@ -282,3 +282,40 @@
}
} // namespace test_alignas
+
+namespace PR51862 {
+
+template<class T> T test() {
+ T a;
+ T b;
+ if (0)
+ return a;
+ return b;
+}
+
+struct A {
+ A();
+ A(A &);
+ A(int);
+ operator int();
+};
+
+// CHECK-LABEL: define{{.*}} void @_ZN7PR518624testINS_1AEEET_v
+// CHECK: call i32 @_ZN7PR518621AcviEv
+// CHECK-NEXT: call void @_ZN7PR518621AC1Ei
+// CHECK-NEXT: call void @llvm.lifetime.end
+template A test<A>();
+
+struct BSub {};
+struct B : BSub {
+ B();
+ B(B &);
+ B(const BSub &);
+};
+
+// CHECK-LABEL: define{{.*}} void @_ZN7PR518624testINS_1BEEET_v
+// CHECK: call void @_ZN7PR518621BC1ERKNS_4BSubE
+// CHECK-NEXT: call void @llvm.lifetime.end
+template B test<B>();
+
+} // namespace PR51862
Index: clang/lib/Sema/SemaStmt.cpp
===================================================================
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3653,8 +3653,8 @@
// In C++ the return statement is handled via a copy initialization.
// the C version of which boils down to CheckSingleAssignmentConstraints.
- InitializedEntity Entity = InitializedEntity::InitializeResult(
- ReturnLoc, FnRetType, NRVOCandidate != nullptr);
+ InitializedEntity Entity =
+ InitializedEntity::InitializeResult(ReturnLoc, FnRetType);
ExprResult Res = PerformMoveOrCopyInitialization(
Entity, NRInfo, RetValExp, SupressSimplerImplicitMoves);
if (Res.isInvalid()) {
@@ -4085,8 +4085,8 @@
// the C version of which boils down to CheckSingleAssignmentConstraints.
if (!HasDependentReturnType && !RetValExp->isTypeDependent()) {
// we have a non-void function with an expression, continue checking
- InitializedEntity Entity = InitializedEntity::InitializeResult(
- ReturnLoc, RetType, NRVOCandidate != nullptr);
+ InitializedEntity Entity =
+ InitializedEntity::InitializeResult(ReturnLoc, RetType);
ExprResult Res = PerformMoveOrCopyInitialization(
Entity, NRInfo, RetValExp, SupressSimplerImplicitMoves);
if (Res.isInvalid()) {
Index: clang/lib/Sema/SemaObjCProperty.cpp
===================================================================
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -1467,8 +1467,7 @@
LoadSelfExpr, true, true);
ExprResult Res = PerformCopyInitialization(
InitializedEntity::InitializeResult(PropertyDiagLoc,
- getterMethod->getReturnType(),
- /*NRVO=*/false),
+ getterMethod->getReturnType()),
PropertyDiagLoc, IvarRefExpr);
if (!Res.isInvalid()) {
Expr *ResExpr = Res.getAs<Expr>();
Index: clang/lib/Sema/SemaLambda.cpp
===================================================================
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -1978,8 +1978,7 @@
CallOperator->markUsed(Context);
ExprResult Init = PerformCopyInitialization(
- InitializedEntity::InitializeLambdaToBlock(ConvLocation, Src->getType(),
- /*NRVO=*/false),
+ InitializedEntity::InitializeLambdaToBlock(ConvLocation, Src->getType()),
CurrentLocation, Src);
if (!Init.isInvalid())
Init = ActOnFinishFullExpr(Init.get(), /*DiscardedValue*/ false);
Index: clang/lib/Sema/SemaExprCXX.cpp
===================================================================
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -893,9 +893,8 @@
if (CheckCXXThrowOperand(OpLoc, ExceptionObjectTy, Ex))
return ExprError();
- InitializedEntity Entity = InitializedEntity::InitializeException(
- OpLoc, ExceptionObjectTy,
- /*NRVO=*/NRInfo.isCopyElidable());
+ InitializedEntity Entity =
+ InitializedEntity::InitializeException(OpLoc, ExceptionObjectTy);
ExprResult Res = PerformMoveOrCopyInitialization(Entity, NRInfo, Ex);
if (Res.isInvalid())
return ExprError();
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15711,7 +15711,7 @@
if (!Result.isInvalid()) {
Result = PerformCopyInitialization(
InitializedEntity::InitializeBlock(Var->getLocation(),
- Cap.getCaptureType(), false),
+ Cap.getCaptureType()),
Loc, Result.get());
}
Index: clang/lib/Sema/SemaCoroutine.cpp
===================================================================
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -1533,7 +1533,7 @@
if (GroType->isVoidType()) {
// Trigger a nice error message.
InitializedEntity Entity =
- InitializedEntity::InitializeResult(Loc, FnRetType, false);
+ InitializedEntity::InitializeResult(Loc, FnRetType);
S.PerformCopyInitialization(Entity, SourceLocation(), ReturnValue);
noteMemberDeclaredHere(S, ReturnValue, Fn);
return false;
Index: clang/lib/Sema/Sema.cpp
===================================================================
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -2020,7 +2020,7 @@
Expr *VarRef =
new (S.Context) DeclRefExpr(S.Context, VD, false, T, VK_LValue, Loc);
ExprResult Result;
- auto IE = InitializedEntity::InitializeBlock(Loc, T, false);
+ auto IE = InitializedEntity::InitializeBlock(Loc, T);
if (S.getLangOpts().CPlusPlus2b) {
auto *E = ImplicitCastExpr::Create(S.Context, T, CK_NoOp, VarRef, nullptr,
VK_XValue, FPOptionsOverride());
Index: clang/lib/CodeGen/CGExprCXX.cpp
===================================================================
--- clang/lib/CodeGen/CGExprCXX.cpp
+++ clang/lib/CodeGen/CGExprCXX.cpp
@@ -612,12 +612,12 @@
// The temporary check is required because Sema sets this on NRVO
// returns.
if (getLangOpts().ElideConstructors && E->isElidable()) {
- assert(getContext().hasSameUnqualifiedType(E->getType(),
- E->getArg(0)->getType()));
- if (E->getArg(0)->isTemporaryObject(getContext(), CD->getParent())) {
- EmitAggExpr(E->getArg(0), Dest);
- return;
- }
+ const Expr *SrcObj = E->getArg(0);
+ assert(SrcObj->isTemporaryObject(getContext(), CD->getParent()));
+ assert(
+ getContext().hasSameUnqualifiedType(E->getType(), SrcObj->getType()));
+ EmitAggExpr(SrcObj, Dest);
+ return;
}
if (const ArrayType *arrayType
Index: clang/include/clang/Sema/Initialization.h
===================================================================
--- clang/include/clang/Sema/Initialization.h
+++ clang/include/clang/Sema/Initialization.h
@@ -298,8 +298,8 @@
/// Create the initialization entity for the result of a function.
static InitializedEntity InitializeResult(SourceLocation ReturnLoc,
- QualType Type, bool NRVO) {
- return InitializedEntity(EK_Result, ReturnLoc, Type, NRVO);
+ QualType Type) {
+ return InitializedEntity(EK_Result, ReturnLoc, Type);
}
static InitializedEntity InitializeStmtExprResult(SourceLocation ReturnLoc,
@@ -308,20 +308,20 @@
}
static InitializedEntity InitializeBlock(SourceLocation BlockVarLoc,
- QualType Type, bool NRVO) {
- return InitializedEntity(EK_BlockElement, BlockVarLoc, Type, NRVO);
+ QualType Type) {
+ return InitializedEntity(EK_BlockElement, BlockVarLoc, Type);
}
static InitializedEntity InitializeLambdaToBlock(SourceLocation BlockVarLoc,
- QualType Type, bool NRVO) {
+ QualType Type) {
return InitializedEntity(EK_LambdaToBlockConversionBlockElement,
- BlockVarLoc, Type, NRVO);
+ BlockVarLoc, Type);
}
/// Create the initialization entity for an exception object.
static InitializedEntity InitializeException(SourceLocation ThrowLoc,
- QualType Type, bool NRVO) {
- return InitializedEntity(EK_Exception, ThrowLoc, Type, NRVO);
+ QualType Type) {
+ return InitializedEntity(EK_Exception, ThrowLoc, Type);
}
/// Create the initialization entity for an object allocated via new.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
