summaryrefslogtreecommitdiffstats
path: root/clang/lib
diff options
context:
space:
mode:
Diffstat (limited to 'clang/lib')
-rw-r--r--clang/lib/Sema/SemaStmt.cpp127
1 files changed, 110 insertions, 17 deletions
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 8963f4c564e..c3a627f1d36 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -2905,7 +2905,8 @@ bool Sema::isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD,
if (VD->getKind() != Decl::Var &&
!((CESK & CES_AllowParameters) && VD->getKind() == Decl::ParmVar))
return false;
- if (VD->isExceptionVariable()) return false;
+ if (!(CESK & CES_AllowExceptionVariables) && VD->isExceptionVariable())
+ return false;
// ...automatic...
if (!VD->hasLocalStorage()) return false;
@@ -2937,6 +2938,11 @@ bool Sema::isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD,
/// 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]p32 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
@@ -2946,6 +2952,7 @@ static void TryMoveInitialization(Sema& S,
const VarDecl *NRVOCandidate,
QualType ResultType,
Expr *&Value,
+ bool ConvertingConstructorsOnly,
ExprResult &Res) {
ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, Value->getType(),
CK_NoOp, Value, VK_XValue);
@@ -2966,22 +2973,39 @@ static void TryMoveInitialization(Sema& S,
continue;
FunctionDecl *FD = Step.Function.Function;
- if (isa<CXXConstructorDecl>(FD)) {
- // C++14 [class.copy]p32:
- // [...] If the first overload resolution fails or was not performed,
- // or 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;
+ if (ConvertingConstructorsOnly) {
+ if (isa<CXXConstructorDecl>(FD)) {
+ // C++14 [class.copy]p32:
+ // [...] If the first overload resolution fails or was not performed,
+ // or 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 {
- continue;
+ 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 (!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;
+ }
}
// Promote "AsRvalue" to the heap, since we now need this
@@ -3019,13 +3043,82 @@ Sema::PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
ExprResult Res = ExprError();
if (AllowNRVO) {
+ bool AffectedByCWG1579 = false;
+
if (!NRVOCandidate) {
NRVOCandidate = getCopyElisionCandidate(ResultType, Value, CES_Default);
+ if (NRVOCandidate &&
+ !getDiagnostics().isIgnored(diag::warn_return_std_move_in_cxx11,
+ Value->getExprLoc())) {
+ const VarDecl *NRVOCandidateInCXX11 =
+ getCopyElisionCandidate(ResultType, Value, CES_FormerDefault);
+ AffectedByCWG1579 = (!NRVOCandidateInCXX11);
+ }
}
if (NRVOCandidate) {
TryMoveInitialization(*this, Entity, NRVOCandidate, ResultType, Value,
- Res);
+ true, Res);
+ }
+
+ if (!Res.isInvalid() && AffectedByCWG1579) {
+ QualType QT = NRVOCandidate->getType();
+ if (QT.getNonReferenceType()
+ .getUnqualifiedType()
+ .isTriviallyCopyableType(Context)) {
+ // Adding 'std::move' around a trivially copyable variable is probably
+ // pointless. Don't suggest it.
+ } else {
+ // Common cases for this are returning unique_ptr<Derived> from a
+ // function of return type unique_ptr<Base>, or returning T from a
+ // function of return type Expected<T>. This is totally fine in a
+ // post-CWG1579 world, but was not fine before.
+ assert(!ResultType.isNull());
+ SmallString<32> Str;
+ Str += "std::move(";
+ Str += NRVOCandidate->getDeclName().getAsString();
+ Str += ")";
+ Diag(Value->getExprLoc(), diag::warn_return_std_move_in_cxx11)
+ << Value->getSourceRange()
+ << NRVOCandidate->getDeclName() << ResultType << QT;
+ Diag(Value->getExprLoc(), diag::note_add_std_move_in_cxx11)
+ << FixItHint::CreateReplacement(Value->getSourceRange(), Str);
+ }
+ } else if (Res.isInvalid() &&
+ !getDiagnostics().isIgnored(diag::warn_return_std_move,
+ Value->getExprLoc())) {
+ const VarDecl *FakeNRVOCandidate =
+ getCopyElisionCandidate(QualType(), Value, CES_AsIfByStdMove);
+ if (FakeNRVOCandidate) {
+ QualType QT = FakeNRVOCandidate->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, FakeNRVOCandidate, ResultType,
+ FakeValue, false, FakeRes);
+ if (!FakeRes.isInvalid()) {
+ bool IsThrow =
+ (Entity.getKind() == InitializedEntity::EK_Exception);
+ SmallString<32> Str;
+ Str += "std::move(";
+ Str += FakeNRVOCandidate->getDeclName().getAsString();
+ Str += ")";
+ Diag(Value->getExprLoc(), diag::warn_return_std_move)
+ << Value->getSourceRange()
+ << FakeNRVOCandidate->getDeclName() << IsThrow;
+ Diag(Value->getExprLoc(), diag::note_add_std_move)
+ << FixItHint::CreateReplacement(Value->getSourceRange(), Str);
+ }
+ }
+ }
}
}
OpenPOWER on IntegriCloud