mizvekov updated this revision to Diff 335688.
mizvekov added a comment.
.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100000/new/
https://reviews.llvm.org/D100000
Files:
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaCoroutine.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaStmt.cpp
Index: clang/lib/Sema/SemaStmt.cpp
===================================================================
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3118,7 +3118,7 @@
/// \returns An aggregate which contains the Candidate and isMoveEligible
/// and isCopyElidable methods. If Candidate is non-null, it means
/// isMoveEligible() would be true under the most permissive language standard.
-Sema::NRVOResult Sema::getNRVOResult(Expr *&E, bool ForceCXX2b) {
+Sema::NRVOResult Sema::getNRVOResult(Expr *&E) {
if (!E)
return NRVOResult();
bool Parenthesized = isa<ParenExpr>(E);
@@ -3128,12 +3128,38 @@
if (!DR || DR->refersToEnclosingVariableOrCapture())
return NRVOResult();
const auto *VD = dyn_cast<VarDecl>(DR->getDecl());
- NRVOResult Res = getNRVOResult(VD, Parenthesized, /*ForceCXX20=*/ForceCXX2b);
- if (Res.Candidate && E->getValueKind() != VK_XValue &&
- (ForceCXX2b || getLangOpts().CPlusPlus2b)) {
- E = ImplicitCastExpr::Create(Context, VD->getType().getNonReferenceType(),
- CK_NoOp, E, nullptr, VK_XValue,
- FPOptionsOverride());
+ NRVOResult Res = getNRVOResult(VD, Parenthesized);
+ if (Res.S >= NRVOResult::MoveEligible) {
+ ExprValueKind CastToKind = VK_XValue;
+ if (E->getValueKind() != CastToKind) {
+ E = ImplicitCastExpr::Create(Context, VD->getType().getNonReferenceType(),
+ CK_NoOp, E, nullptr, CastToKind,
+ FPOptionsOverride());
+ }
+ } else if (Res.Candidate &&
+ !getDiagnostics().isIgnored(diag::warn_return_std_move,
+ E->getExprLoc())) {
+ QualType QT = Res.Candidate->getType();
+ if (QT->isLValueReferenceType()) {
+ // Adding 'std::move' around an lvalue reference variable's name is
+ // dangerous. Don't suggest it.
+ } else if (QT.getNonReferenceType()
+ .getUnqualifiedType()
+ .isTriviallyCopyableType(Context)) {
+ // Adding 'std::move' around a trivially copyable variable is probably
+ // pointless. Don't suggest it.
+ } else {
+ bool IsThrow =
+ false; //(Entity.getKind() == InitializedEntity::EK_Exception);
+ SmallString<32> Str;
+ Str += "std::move(";
+ Str += Res.Candidate->getDeclName().getAsString();
+ Str += ")";
+ Diag(E->getExprLoc(), diag::warn_return_std_move)
+ << E->getSourceRange() << Res.Candidate->getDeclName() << IsThrow;
+ Diag(E->getExprLoc(), diag::note_add_std_move)
+ << FixItHint::CreateReplacement(E->getSourceRange(), Str);
+ }
}
return Res;
}
@@ -3185,160 +3211,6 @@
downgradeNRVOResult(Res, getLangOpts().CPlusPlus11);
}
-/// Try to perform the initialization of a potentially-movable value,
-/// which is the operand to a return or throw statement.
-///
-/// This routine implements C++20 [class.copy.elision]p3, which attempts to
-/// treat returned lvalues as rvalues in certain cases (to prefer move
-/// construction), then falls back to treating them as lvalues if that failed.
-///
-/// \param ConvertingConstructorsOnly If true, follow [class.copy.elision]p3 and
-/// reject resolutions that find non-constructors, such as derived-to-base
-/// conversions or `operator T()&&` member functions. If false, do consider such
-/// conversion sequences.
-///
-/// \param Res We will fill this in if move-initialization was possible.
-/// If move-initialization is not possible, such that we must fall back to
-/// treating the operand as an lvalue, we will leave Res in its original
-/// invalid state.
-///
-/// \returns Whether we need to do the second overload resolution. If the first
-/// overload resolution fails, or if the first overload resolution succeeds but
-/// the selected constructor/operator doesn't match the additional criteria, we
-/// need to do the second overload resolution.
-static bool TryMoveInitialization(Sema &S, const InitializedEntity &Entity,
- const VarDecl *NRVOCandidate, Expr *&Value,
- bool ConvertingConstructorsOnly,
- bool IsDiagnosticsCheck, ExprResult &Res) {
- ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, Value->getType(),
- CK_NoOp, Value, VK_XValue, FPOptionsOverride());
-
- Expr *InitExpr = &AsRvalue;
-
- InitializationKind Kind = InitializationKind::CreateCopy(
- Value->getBeginLoc(), Value->getBeginLoc());
-
- InitializationSequence Seq(S, Entity, Kind, InitExpr);
-
- bool NeedSecondOverloadResolution = true;
- if (!Seq &&
- (IsDiagnosticsCheck || Seq.getFailedOverloadResult() != OR_Deleted)) {
- return NeedSecondOverloadResolution;
- }
-
- for (const InitializationSequence::Step &Step : Seq.steps()) {
- if (Step.Kind != InitializationSequence::SK_ConstructorInitialization &&
- Step.Kind != InitializationSequence::SK_UserConversion)
- continue;
-
- FunctionDecl *FD = Step.Function.Function;
- if (ConvertingConstructorsOnly) {
- if (isa<CXXConstructorDecl>(FD)) {
- // C++11 [class.copy]p32:
- // C++14 [class.copy]p32:
- // C++17 [class.copy.elision]p3:
- // [...] if the type of the first parameter of the selected constructor
- // is not an rvalue reference to the object's type (possibly
- // cv-qualified), overload resolution is performed again, considering
- // the object as an lvalue.
- const RValueReferenceType *RRefType =
- FD->getParamDecl(0)->getType()->getAs<RValueReferenceType>();
- if (!RRefType)
- break;
- if (!S.Context.hasSameUnqualifiedType(RRefType->getPointeeType(),
- NRVOCandidate->getType()))
- break;
- } else {
- continue;
- }
- } else {
- if (isa<CXXConstructorDecl>(FD)) {
- // Check that overload resolution selected a constructor taking an
- // rvalue reference. If it selected an lvalue reference, then we
- // didn't need to cast this thing to an rvalue in the first place.
- if (IsDiagnosticsCheck &&
- !isa<RValueReferenceType>(FD->getParamDecl(0)->getType()))
- break;
- } else if (isa<CXXMethodDecl>(FD)) {
- // Check that overload resolution selected a conversion operator
- // taking an rvalue reference.
- if (cast<CXXMethodDecl>(FD)->getRefQualifier() != RQ_RValue)
- break;
- } else {
- continue;
- }
- }
-
- NeedSecondOverloadResolution = false;
- // Promote "AsRvalue" to the heap, since we now need this
- // expression node to persist.
- Value =
- ImplicitCastExpr::Create(S.Context, Value->getType(), CK_NoOp, Value,
- nullptr, VK_XValue, FPOptionsOverride());
-
- // Complete type-checking the initialization of the return type
- // using the constructor we found.
- Res = Seq.Perform(S, Entity, Kind, Value);
- }
-
- return NeedSecondOverloadResolution;
-}
-
-/// Perform the initialization of a potentially-movable value, which
-/// is the result of return value.
-///
-/// This routine implements C++20 [class.copy.elision]p3, which attempts to
-/// treat returned lvalues as rvalues in certain cases (to prefer move
-/// construction), then falls back to treating them as lvalues if that failed.
-ExprResult
-Sema::PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
- const NRVOResult &NRVORes, Expr *Value) {
-
- if (NRVORes.Candidate && !getLangOpts().CPlusPlus2b) {
- if (NRVORes.isMoveEligible()) {
- ExprResult Res;
- if (!TryMoveInitialization(*this, Entity, NRVORes.Candidate, Value,
- !getLangOpts().CPlusPlus20, false, Res))
- return Res;
- }
- if (!getDiagnostics().isIgnored(diag::warn_return_std_move,
- Value->getExprLoc())) {
- QualType QT = NRVORes.Candidate->getType();
- if (QT->isLValueReferenceType()) {
- // Adding 'std::move' around an lvalue reference variable's name is
- // dangerous. Don't suggest it.
- } else if (QT.getNonReferenceType()
- .getUnqualifiedType()
- .isTriviallyCopyableType(Context)) {
- // Adding 'std::move' around a trivially copyable variable is probably
- // pointless. Don't suggest it.
- } else {
- ExprResult FakeRes = ExprError();
- Expr *FakeValue = Value;
- TryMoveInitialization(*this, Entity, NRVORes.Candidate, FakeValue,
- false, true, FakeRes);
- if (!FakeRes.isInvalid()) {
- bool IsThrow = (Entity.getKind() == InitializedEntity::EK_Exception);
- SmallString<32> Str;
- Str += "std::move(";
- Str += NRVORes.Candidate->getDeclName().getAsString();
- Str += ")";
- Diag(Value->getExprLoc(), diag::warn_return_std_move)
- << Value->getSourceRange() << NRVORes.Candidate->getDeclName()
- << IsThrow;
- Diag(Value->getExprLoc(), diag::note_add_std_move)
- << FixItHint::CreateReplacement(Value->getSourceRange(), Str);
- }
- }
- }
- }
-
- // Either we didn't meet the criteria for treating an lvalue as an rvalue,
- // above, or overload resolution failed. Either way, we need to try
- // (again) now with the return value expression as written.
- return PerformCopyInitialization(Entity, SourceLocation(), Value);
-}
-
/// Determine whether the declared return type of the specified function
/// contains 'auto'.
static bool hasDeducedReturnType(FunctionDecl *FD) {
@@ -3483,7 +3355,7 @@
InitializedEntity Entity = InitializedEntity::InitializeResult(
ReturnLoc, FnRetType, NRVORes.isCopyElidable());
ExprResult Res =
- PerformMoveOrCopyInitialization(Entity, NRVORes, RetValExp);
+ PerformCopyInitialization(Entity, SourceLocation(), RetValExp);
if (Res.isInvalid()) {
// FIXME: Cleanup temporaries here, anyway?
return StmtError();
@@ -3894,7 +3766,7 @@
InitializedEntity Entity = InitializedEntity::InitializeResult(
ReturnLoc, RetType, NRVORes.isCopyElidable());
ExprResult Res =
- PerformMoveOrCopyInitialization(Entity, NRVORes, RetValExp);
+ PerformCopyInitialization(Entity, SourceLocation(), RetValExp);
if (Res.isInvalid()) {
// FIXME: Clean up temporaries here anyway?
return StmtError();
Index: clang/lib/Sema/SemaExprCXX.cpp
===================================================================
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -876,7 +876,7 @@
InitializedEntity Entity = InitializedEntity::InitializeException(
OpLoc, ExceptionObjectTy,
/*NRVO=*/NRVORes.isCopyElidable());
- ExprResult Res = PerformMoveOrCopyInitialization(Entity, NRVORes, Ex);
+ ExprResult Res = PerformCopyInitialization(Entity, SourceLocation(), Ex);
if (Res.isInvalid())
return ExprError();
Ex = Res.get();
Index: clang/lib/Sema/SemaCoroutine.cpp
===================================================================
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -997,7 +997,7 @@
VarDecl *Promise = FSI->CoroutinePromise;
ExprResult PC;
if (E && (isa<InitListExpr>(E) || !E->getType()->isVoidType())) {
- getNRVOResult(E, /*ForceCXX2b=*/true);
+ getNRVOResult(E);
PC = buildPromiseCall(*this, Promise, Loc, "return_value", E);
} else {
E = MakeFullDiscardedValueExpr(E).get();
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -4730,18 +4730,12 @@
Status S;
bool isParenthesized;
- bool isMoveEligible() const { return S >= MoveEligible; };
bool isCopyElidable() const { return S == MoveEligibleAndCopyElidable; }
};
- NRVOResult getNRVOResult(Expr *&E, bool ForceCXX2b = false);
- NRVOResult getNRVOResult(const VarDecl *VD, bool Parenthesized = false,
- bool ForceCXX20 = false);
+ NRVOResult getNRVOResult(Expr *&E);
+ NRVOResult getNRVOResult(const VarDecl *VD, bool Parenthesized = false);
void updNRVOResultWithRetType(NRVOResult &Res, QualType ReturnType);
- ExprResult PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
- const NRVOResult &NRVORes,
- Expr *Value);
-
StmtResult ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
Scope *CurScope);
StmtResult BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
