Quuxplusone created this revision.
Quuxplusone added a project: clang.
Herald added a subscriber: cfe-commits.
This patch adds two new diagnostics, which are off by default:
**-Wreturn-std-move**
Diagnose cases of `return x` or `throw x`, where `x` is the name of a local
variable or parameter, in which a copy is made.
The user probably expected a move, but they're not getting a move, perhaps
because the type of "x" is different from the return type of the function. A
place where this comes up in the wild is `stdext::inplace_function<Sig, N>`
which implements conversion via a conversion operator rather than a converting
constructor; see https://github.com/WG21-SG14/SG14/issues/125#issue-297201412
Another place where this has come up in the wild, but where the fix ended up
being different, was
try { ... } catch (ExceptionType ex) {
throw ex;
}
where the appropriate fix in that case was to replace `throw ex;` with
`throw;`, and incidentally to catch by reference instead of by value. (But one
could contrive a scenario where the slicing was intentional, in which case
throw-by-move would have been the appropriate fix after all.)
**-Wreturn-std-move-in-c++11**
Diagnose cases of "return x;" or "throw x;" which in this version of Clang *do*
produce moves, but which prior to Clang 3.9 / GCC 5.1 produced copies instead.
This is useful in codebases which care about portability to those older
compilers.
The name "-in-c++11" is not technically correct; what caused the
version-to-version change in behavior here was actually CWG 1579, not C++14. So
I'm using the CWG issue number in the diagnostic but "C++11" in the name of the
option. I think it's likely that codebases that need portability to GCC
4.9-and-earlier may understand "C++11" as a colloquialism for "older compilers."
**Discussion**
I fully expect there to be lots of discussion on this patch, and for my first
draft to be missing lots of pieces. I do hope that we can hammer it into a
shape suitable for upstreaming, but milestone number 1 is just to get it out
there for people to use on their own codebases.
Notice that this patch is kind of a belated negative-space version of Richard
Trieu's `-Wpessimizing-move`. That diagnostic warns about cases of `return
std::move(x)` that should be `return x` for speed. These diagnostics warn about
cases of `return x` that should be `return std::move(x)` for speed. (The two
diagnostics' bailiwicks do not overlap.)
Repository:
rC Clang
https://reviews.llvm.org/D43322
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/Sema/SemaExprCXX.cpp
lib/Sema/SemaStmt.cpp
lib/Sema/SemaTemplateInstantiateDecl.cpp
test/SemaCXX/warn-return-std-move.cpp
Index: test/SemaCXX/warn-return-std-move.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/warn-return-std-move.cpp
@@ -0,0 +1,262 @@
+// RUN: %clang_cc1 -fsyntax-only -Wreturn-std-move -Wreturn-std-move-in-cxx11 -std=c++11 -verify %s
+
+// definitions for std::move
+namespace std {
+inline namespace foo {
+template <class T> struct remove_reference { typedef T type; };
+template <class T> struct remove_reference<T&> { typedef T type; };
+template <class T> struct remove_reference<T&&> { typedef T type; };
+
+template <class T> typename remove_reference<T>::type &&move(T &&t);
+}
+}
+
+struct Instrument {
+ Instrument() {}
+ Instrument(Instrument&&) { /* MOVE */ }
+ Instrument(const Instrument&) { /* COPY */ }
+};
+struct ConvertFromBase { Instrument i; };
+struct ConvertFromDerived { Instrument i; };
+struct Base {
+ Instrument i;
+ operator ConvertFromBase() const& { return ConvertFromBase{i}; }
+ operator ConvertFromBase() && { return ConvertFromBase{std::move(i)}; }
+};
+struct Derived : public Base {
+ operator ConvertFromDerived() const& { return ConvertFromDerived{i}; }
+ operator ConvertFromDerived() && { return ConvertFromDerived{std::move(i)}; }
+};
+struct ConstructFromBase {
+ Instrument i;
+ ConstructFromBase(const Base& b): i(b.i) {}
+ ConstructFromBase(Base&& b): i(std::move(b.i)) {}
+};
+struct ConstructFromDerived {
+ Instrument i;
+ ConstructFromDerived(const Derived& d): i(d.i) {}
+ ConstructFromDerived(Derived&& d): i(std::move(d.i)) {}
+};
+
+struct TrivialInstrument {
+ int i = 42;
+};
+struct ConvertFromTrivialBase { TrivialInstrument i; };
+struct ConvertFromTrivialDerived { TrivialInstrument i; };
+struct TrivialBase {
+ TrivialInstrument i;
+ operator ConvertFromTrivialBase() const& { return ConvertFromTrivialBase{i}; }
+ operator ConvertFromTrivialBase() && { return ConvertFromTrivialBase{std::move(i)}; }
+};
+struct TrivialDerived : public TrivialBase {
+ operator ConvertFromTrivialDerived() const& { return ConvertFromTrivialDerived{i}; }
+ operator ConvertFromTrivialDerived() && { return ConvertFromTrivialDerived{std::move(i)}; }
+};
+struct ConstructFromTrivialBase {
+ TrivialInstrument i;
+ ConstructFromTrivialBase(const TrivialBase& b): i(b.i) {}
+ ConstructFromTrivialBase(TrivialBase&& b): i(std::move(b.i)) {}
+};
+struct ConstructFromTrivialDerived {
+ TrivialInstrument i;
+ ConstructFromTrivialDerived(const TrivialDerived& d): i(d.i) {}
+ ConstructFromTrivialDerived(TrivialDerived&& d): i(std::move(d.i)) {}
+};
+
+Derived test1() {
+ Derived d1;
+ return d1; // ok
+}
+Base test2() {
+ Derived d2;
+ return d2; // e1
+ // expected-warning@-1{{would select a better conversion sequence}}
+}
+ConstructFromDerived test3() {
+ Derived d3;
+ return d3; // e2-cxx11
+ // expected-warning@-1{{would have selected a better conversion sequence}}
+}
+ConstructFromBase test4() {
+ Derived d4;
+ return d4; // e3
+ // expected-warning@-1{{would select a better conversion sequence}}
+}
+ConvertFromDerived test5() {
+ Derived d5;
+ return d5; // e4
+ // expected-warning@-1{{would select a better conversion sequence}}
+}
+ConvertFromBase test6() {
+ Derived d6;
+ return d6; // e5
+ // expected-warning@-1{{would select a better conversion sequence}}
+}
+
+// These test cases should not produce the warning.
+Derived ok1() { Derived d; return d; }
+Base ok2() { Derived d; return static_cast<Derived&&>(d); }
+ConstructFromDerived ok3() { Derived d; return static_cast<Derived&&>(d); }
+ConstructFromBase ok4() { Derived d; return static_cast<Derived&&>(d); }
+ConvertFromDerived ok5() { Derived d; return static_cast<Derived&&>(d); }
+ConvertFromBase ok6() { Derived d; return static_cast<Derived&&>(d); }
+
+// If the target is an lvalue reference, assume it's not safe to move from.
+Derived ok_lvalue1(Derived& d) { return d; }
+Base ok_lvalue2(Derived& d) { return d; }
+ConstructFromDerived ok_lvalue3(Derived& d) { return d; }
+ConstructFromBase ok_lvalue4(Derived& d) { return d; }
+ConvertFromDerived ok_lvalue5(Derived& d) { return d; }
+ConvertFromBase ok_lvalue6(Derived& d) { return d; }
+
+// If the target is a global, assume it's not safe to move from.
+static Derived global_d;
+Derived ok_global1() { return global_d; }
+Base ok_global2() { return global_d; }
+ConstructFromDerived ok_global3() { return global_d; }
+ConstructFromBase ok_global4() { return global_d; }
+ConvertFromDerived ok_global5() { return global_d; }
+ConvertFromBase ok_global6() { return global_d; }
+
+// If the target's copy constructor is trivial, assume the programmer doesn't care.
+TrivialDerived ok_trivial1(TrivialDerived d) { return d; }
+TrivialBase ok_trivial2(TrivialDerived d) { return d; }
+ConstructFromTrivialDerived ok_trivial3(TrivialDerived d) { return d; }
+ConstructFromTrivialBase ok_trivial4(TrivialDerived d) { return d; }
+ConvertFromTrivialDerived ok_trivial5(TrivialDerived d) { return d; }
+ConvertFromTrivialBase ok_trivial6(TrivialDerived d) { return d; }
+
+// If the target is a parameter, do apply the diagnostic.
+Derived testParam1(Derived d) { return d; }
+Base testParam2(Derived d) {
+ return d; // e6
+ // expected-warning@-1{{would select a better conversion sequence}}
+}
+ConstructFromDerived testParam3(Derived d) {
+ return d; // e7-cxx11
+ // expected-warning@-1{{would have selected a better conversion sequence}}
+}
+ConstructFromBase testParam4(Derived d) {
+ return d; // e8
+ // expected-warning@-1{{would select a better conversion sequence}}
+}
+ConvertFromDerived testParam5(Derived d) {
+ return d; // e9
+ // expected-warning@-1{{would select a better conversion sequence}}
+}
+ConvertFromBase testParam6(Derived d) {
+ return d; // e10
+ // expected-warning@-1{{would select a better conversion sequence}}
+}
+
+// If the target is an rvalue reference parameter, do apply the diagnostic.
+Derived testRParam1(Derived&& d) {
+ return d; // e11
+ // expected-warning@-1{{would select a better conversion sequence}}
+}
+Base testRParam2(Derived&& d) {
+ return d; // e12
+ // expected-warning@-1{{would select a better conversion sequence}}
+}
+ConstructFromDerived testRParam3(Derived&& d) {
+ return d; // e13
+ // expected-warning@-1{{would select a better conversion sequence}}
+}
+ConstructFromBase testRParam4(Derived&& d) {
+ return d; // e14
+ // expected-warning@-1{{would select a better conversion sequence}}
+}
+ConvertFromDerived testRParam5(Derived&& d) {
+ return d; // e15
+ // expected-warning@-1{{would select a better conversion sequence}}
+}
+ConvertFromBase testRParam6(Derived&& d) {
+ return d; // e16
+ // expected-warning@-1{{would select a better conversion sequence}}
+}
+
+// If the target is a catch-handler parameter, do apply the diagnostic.
+void throw_derived();
+Derived testEParam1() {
+ try { throw_derived(); } catch (Derived d) { return d; } // e17
+ // expected-warning@-1{{would select a better conversion sequence}}
+ __builtin_unreachable();
+}
+Base testEParam2() {
+ try { throw_derived(); } catch (Derived d) { return d; } // e18
+ // expected-warning@-1{{would select a better conversion sequence}}
+ __builtin_unreachable();
+}
+ConstructFromDerived testEParam3() {
+ try { throw_derived(); } catch (Derived d) { return d; } // e19
+ // expected-warning@-1{{would select a better conversion sequence}}
+ __builtin_unreachable();
+}
+ConstructFromBase testEParam4() {
+ try { throw_derived(); } catch (Derived d) { return d; } // e20
+ // expected-warning@-1{{would select a better conversion sequence}}
+ __builtin_unreachable();
+}
+ConvertFromDerived testEParam5() {
+ try { throw_derived(); } catch (Derived d) { return d; } // e21
+ // expected-warning@-1{{would select a better conversion sequence}}
+ __builtin_unreachable();
+}
+ConvertFromBase testEParam6() {
+ try { throw_derived(); } catch (Derived d) { return d; } // e22
+ // expected-warning@-1{{would select a better conversion sequence}}
+ __builtin_unreachable();
+}
+
+// Even if the exception variable is an lvalue reference, we still own it.
+Derived testREParam1() {
+ try { throw_derived(); } catch (Derived& d) { return d; } // e23
+ // expected-warning@-1{{would select a better conversion sequence}}
+ __builtin_unreachable();
+}
+Base testREParam2() {
+ try { throw_derived(); } catch (Derived& d) { return d; } // e24
+ // expected-warning@-1{{would select a better conversion sequence}}
+ __builtin_unreachable();
+}
+ConstructFromDerived testREParam3() {
+ try { throw_derived(); } catch (Derived& d) { return d; } // e25
+ // expected-warning@-1{{would select a better conversion sequence}}
+ __builtin_unreachable();
+}
+ConstructFromBase testREParam4() {
+ try { throw_derived(); } catch (Derived& d) { return d; } // e26
+ // expected-warning@-1{{would select a better conversion sequence}}
+ __builtin_unreachable();
+}
+ConvertFromDerived testREParam5() {
+ try { throw_derived(); } catch (Derived& d) { return d; } // e27
+ // expected-warning@-1{{would select a better conversion sequence}}
+ __builtin_unreachable();
+}
+ConvertFromBase testREParam6() {
+ try { throw_derived(); } catch (Derived& d) { return d; } // e28
+ // expected-warning@-1{{would select a better conversion sequence}}
+ __builtin_unreachable();
+}
+
+Derived ok_CEParam1() { try { throw_derived(); } catch (const Derived& d) { return d; } __builtin_unreachable(); }
+Base ok_CEParam2() { try { throw_derived(); } catch (const Derived& d) { return d; } __builtin_unreachable(); }
+ConstructFromDerived ok_CEParam3() { try { throw_derived(); } catch (const Derived& d) { return d; } __builtin_unreachable(); }
+ConstructFromBase ok_CEParam4() { try { throw_derived(); } catch (const Derived& d) { return d; } __builtin_unreachable(); }
+ConvertFromDerived ok_CEParam5() { try { throw_derived(); } catch (const Derived& d) { return d; } __builtin_unreachable(); }
+ConvertFromBase ok_CEParam6() { try { throw_derived(); } catch (const Derived& d) { return d; } __builtin_unreachable(); }
+
+
+// If rvalue overload resolution would find a copy constructor anyway,
+// or if the copy constructor actually selected is trivial, then don't warn.
+struct TriviallyCopyable {};
+struct OnlyCopyable {
+ OnlyCopyable() = default;
+ OnlyCopyable(const OnlyCopyable&) {}
+};
+
+TriviallyCopyable ok_copy1() { TriviallyCopyable c; return c; }
+OnlyCopyable ok_copy2() { OnlyCopyable c; return c; }
+TriviallyCopyable ok_copyparam1(TriviallyCopyable c) { return c; }
+OnlyCopyable ok_copyparam2(OnlyCopyable c) { return c; }
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===================================================================
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -741,7 +741,7 @@
if (D->isNRVOVariable()) {
QualType ReturnType = cast<FunctionDecl>(DC)->getReturnType();
- if (SemaRef.isCopyElisionCandidate(ReturnType, Var, false))
+ if (SemaRef.isCopyElisionCandidate(ReturnType, Var, Sema::CES_Strict))
Var->setNRVOVariable(true);
}
Index: lib/Sema/SemaStmt.cpp
===================================================================
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -2858,7 +2858,7 @@
/// \param E The expression being returned from the function or block, or
/// being thrown.
///
-/// \param AllowParamOrMoveConstructible Whether we allow function parameters or
+/// \param CESK Whether we allow function parameters or
/// id-expressions that could be moved out of the function to be considered NRVO
/// candidates. C++ prohibits these for NRVO itself, but we re-use this logic to
/// determine whether we should try to move as part of a return or throw (which
@@ -2867,7 +2867,7 @@
/// \returns The NRVO candidate variable, if the return statement may use the
/// NRVO, or NULL if there is no such candidate.
VarDecl *Sema::getCopyElisionCandidate(QualType ReturnType, Expr *E,
- bool AllowParamOrMoveConstructible) {
+ CopyElisionSemanticsKind CESK) {
if (!getLangOpts().CPlusPlus)
return nullptr;
@@ -2880,13 +2880,13 @@
if (!VD)
return nullptr;
- if (isCopyElisionCandidate(ReturnType, VD, AllowParamOrMoveConstructible))
+ if (isCopyElisionCandidate(ReturnType, VD, CESK))
return VD;
return nullptr;
}
bool Sema::isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD,
- bool AllowParamOrMoveConstructible) {
+ CopyElisionSemanticsKind CESK) {
QualType VDType = VD->getType();
// - in a return statement in a function with ...
// ... a class return type ...
@@ -2895,16 +2895,17 @@
return false;
// ... the same cv-unqualified type as the function return type ...
// When considering moving this expression out, allow dissimilar types.
- if (!AllowParamOrMoveConstructible && !VDType->isDependentType() &&
+ if (!(CESK & CES_AllowMoveConstructible) && !VDType->isDependentType() &&
!Context.hasSameUnqualifiedType(ReturnType, VDType))
return false;
}
// ...object (other than a function or catch-clause parameter)...
if (VD->getKind() != Decl::Var &&
- !(AllowParamOrMoveConstructible && VD->getKind() == Decl::ParmVar))
+ !((CESK & CES_AllowParameters) && VD->getKind() == Decl::ParmVar))
+ return false;
+ if (!(CESK & CES_AllowExceptionVariables) && VD->isExceptionVariable())
return false;
- if (VD->isExceptionVariable()) return false;
// ...automatic...
if (!VD->hasLocalStorage()) return false;
@@ -2914,7 +2915,7 @@
// variable will no longer be used.
if (VD->hasAttr<BlocksAttr>()) return false;
- if (AllowParamOrMoveConstructible)
+ if (CESK & CES_AllowMoveConstructible)
return true;
// ...non-volatile...
@@ -2929,6 +2930,70 @@
return true;
}
+static void AttemptMoveInitialization(Sema& S,
+ const InitializedEntity &Entity,
+ const VarDecl *NRVOCandidate,
+ QualType ResultType,
+ Expr *Value,
+ bool IsFake,
+ ExprResult &Res)
+{
+ ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, Value->getType(),
+ CK_NoOp, Value, VK_XValue);
+
+ Expr *InitExpr = &AsRvalue;
+
+ InitializationKind Kind = InitializationKind::CreateCopy(
+ Value->getLocStart(), Value->getLocStart());
+
+ InitializationSequence Seq(S, Entity, Kind, InitExpr);
+ if (Seq) {
+ 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 (isa<CXXConstructorDecl>(FD)) {
+ if (IsFake) {
+ // 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 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 if (IsFake && isa<CXXMethodDecl>(FD)) {
+ if (cast<CXXMethodDecl>(FD)->getRefQualifier() != RQ_RValue)
+ break;
+ } else {
+ continue;
+ }
+
+ // 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);
+
+ // Complete type-checking the initialization of the return type
+ // using the constructor we found.
+ Res = Seq.Perform(S, Entity, Kind, Value);
+ }
+ }
+}
+
/// \brief Perform the initialization of a potentially-movable value, which
/// is the result of return value.
///
@@ -2952,52 +3017,47 @@
// were designated by an rvalue.
ExprResult Res = ExprError();
- if (AllowNRVO && !NRVOCandidate)
- NRVOCandidate = getCopyElisionCandidate(ResultType, Value, true);
-
- if (AllowNRVO && NRVOCandidate) {
- ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, Value->getType(),
- CK_NoOp, Value, VK_XValue);
-
- Expr *InitExpr = &AsRvalue;
-
- InitializationKind Kind = InitializationKind::CreateCopy(
- Value->getLocStart(), Value->getLocStart());
+ if (AllowNRVO) {
+ bool AffectedByCWG1579 = false;
- InitializationSequence Seq(*this, Entity, Kind, InitExpr);
- if (Seq) {
- for (const InitializationSequence::Step &Step : Seq.steps()) {
- if (!(Step.Kind ==
- InitializationSequence::SK_ConstructorInitialization ||
- (Step.Kind == InitializationSequence::SK_UserConversion &&
- isa<CXXConstructorDecl>(Step.Function.Function))))
- continue;
-
- CXXConstructorDecl *Constructor =
- cast<CXXConstructorDecl>(Step.Function.Function);
-
- const RValueReferenceType *RRefType
- = Constructor->getParamDecl(0)->getType()
- ->getAs<RValueReferenceType>();
-
- // [...] 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.
- if (!RRefType ||
- !Context.hasSameUnqualifiedType(RRefType->getPointeeType(),
- NRVOCandidate->getType()))
- break;
+ if (!NRVOCandidate) {
+ NRVOCandidate = getCopyElisionCandidate(ResultType, Value, CES_Default);
+ if (NRVOCandidate && !getDiagnostics().isIgnored(diag::warn_return_std_move_in_cxx11, Value->getExprLoc())) {
+ const auto *NRVOCandidateInCXX11 = getCopyElisionCandidate(ResultType, Value, CES_FormerDefault);
+ AffectedByCWG1579 = (!NRVOCandidateInCXX11);
+ }
+ }
- // Promote "AsRvalue" to the heap, since we now need this
- // expression node to persist.
- Value = ImplicitCastExpr::Create(Context, Value->getType(), CK_NoOp,
- Value, nullptr, VK_XValue);
+ if (NRVOCandidate) {
+ AttemptMoveInitialization(*this, Entity, NRVOCandidate, ResultType, Value, false, Res);
+ }
- // Complete type-checking the initialization of the return type
- // using the constructor we found.
- Res = Seq.Perform(*this, Entity, Kind, Value);
+ if (!Res.isInvalid() && AffectedByCWG1579) {
+ const auto QT = NRVOCandidate->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 {
+ // The most common case for this is returning T from a function that returns Expected<T>.
+ // This is totally fine in a post-CWG1579 world, but was not fine before.
+ Diag(Value->getExprLoc(), diag::warn_return_std_move_in_cxx11);
+ }
+ } else if (Res.isInvalid() && !getDiagnostics().isIgnored(diag::warn_return_std_move, Value->getExprLoc())) {
+ const VarDecl *FakeNRVOCandidate = getCopyElisionCandidate(QualType(), Value, CES_AsIfByStdMove);
+ if (FakeNRVOCandidate) {
+ const auto QT = FakeNRVOCandidate->getType();
+ if (QT->isLValueReferenceType() && !FakeNRVOCandidate->isExceptionVariable()) {
+ // 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();
+ AttemptMoveInitialization(*this, Entity, FakeNRVOCandidate, ResultType, Value, true, FakeRes);
+ if (!FakeRes.isInvalid()) {
+ Diag(Value->getExprLoc(), diag::warn_return_std_move);
+ }
+ }
}
}
}
@@ -3145,7 +3205,7 @@
// In C++ the return statement is handled via a copy initialization.
// the C version of which boils down to CheckSingleAssignmentConstraints.
- NRVOCandidate = getCopyElisionCandidate(FnRetType, RetValExp, false);
+ NRVOCandidate = getCopyElisionCandidate(FnRetType, RetValExp, CES_Strict);
InitializedEntity Entity = InitializedEntity::InitializeResult(ReturnLoc,
FnRetType,
NRVOCandidate != nullptr);
@@ -3158,7 +3218,7 @@
RetValExp = Res.get();
CheckReturnValExpr(RetValExp, FnRetType, ReturnLoc);
} else {
- NRVOCandidate = getCopyElisionCandidate(FnRetType, RetValExp, false);
+ NRVOCandidate = getCopyElisionCandidate(FnRetType, RetValExp, CES_Strict);
}
if (RetValExp) {
@@ -3528,7 +3588,7 @@
// In C++ the return statement is handled via a copy initialization,
// the C version of which boils down to CheckSingleAssignmentConstraints.
if (RetValExp)
- NRVOCandidate = getCopyElisionCandidate(FnRetType, RetValExp, false);
+ NRVOCandidate = getCopyElisionCandidate(FnRetType, RetValExp, CES_Strict);
if (!HasDependentReturnType && !RetValExp->isTypeDependent()) {
// we have a non-void function with an expression, continue checking
InitializedEntity Entity = InitializedEntity::InitializeResult(ReturnLoc,
Index: lib/Sema/SemaExprCXX.cpp
===================================================================
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -728,7 +728,7 @@
// exception object
const VarDecl *NRVOVariable = nullptr;
if (IsThrownVarInScope)
- NRVOVariable = getCopyElisionCandidate(QualType(), Ex, false);
+ NRVOVariable = getCopyElisionCandidate(QualType(), Ex, CES_Strict);
InitializedEntity Entity = InitializedEntity::InitializeException(
OpLoc, ExceptionObjectTy,
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -3810,10 +3810,21 @@
RecordDecl *CreateCapturedStmtRecordDecl(CapturedDecl *&CD,
SourceLocation Loc,
unsigned NumParams);
+
+ enum CopyElisionSemanticsKind {
+ CES_Strict = 0,
+ CES_AllowParameters = 1,
+ CES_AllowMoveConstructible = 2,
+ CES_AllowExceptionVariables = 4,
+ CES_FormerDefault = (CES_AllowParameters),
+ CES_Default = (CES_AllowParameters | CES_AllowMoveConstructible),
+ CES_AsIfByStdMove = (CES_AllowParameters | CES_AllowMoveConstructible | CES_AllowExceptionVariables),
+ };
+
VarDecl *getCopyElisionCandidate(QualType ReturnType, Expr *E,
- bool AllowParamOrMoveConstructible);
+ CopyElisionSemanticsKind CESK);
bool isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD,
- bool AllowParamOrMoveConstructible);
+ CopyElisionSemanticsKind CESK);
StmtResult ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
Scope *CurScope);
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -19,6 +19,14 @@
def note_callee_decl : Note<"%0 declared here">;
def note_defined_here : Note<"%0 defined here">;
+def warn_return_std_move : Warning<
+ "adding 'std::move' here would select a better conversion sequence">,
+ InGroup<ReturnStdMove>, DefaultIgnore;
+
+def warn_return_std_move_in_cxx11 : Warning<
+ "adding 'std::move' here would have selected a better conversion sequence prior to CWG 1579">,
+ InGroup<ReturnStdMoveInCXX11>, DefaultIgnore;
+
// For loop analysis
def warn_variables_not_in_loop_body : Warning<
"variable%select{s| %1|s %1 and %2|s %1, %2, and %3|s %1, %2, %3, and %4}0 "
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -380,7 +380,11 @@
def ExplicitInitializeCall : DiagGroup<"explicit-initialize-call">;
def Packed : DiagGroup<"packed">;
def Padded : DiagGroup<"padded">;
+
def PessimizingMove : DiagGroup<"pessimizing-move">;
+def ReturnStdMoveInCXX11 : DiagGroup<"return-std-move-in-c++11">;
+def ReturnStdMove : DiagGroup<"return-std-move">;
+
def PointerArith : DiagGroup<"pointer-arith">;
def PoundWarning : DiagGroup<"#warnings">;
def PoundPragmaMessage : DiagGroup<"#pragma-messages">,
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
