Index: clang/include/clang/Basic/DiagnosticGroups.td =================================================================== --- clang/include/clang/Basic/DiagnosticGroups.td +++ clang/include/clang/Basic/DiagnosticGroups.td @@ -555,6 +555,8 @@ def PessimizingMove : DiagGroup<"pessimizing-move">; def ReturnStdMove : DiagGroup<"return-std-move">; +def PessimizingReturnByConst : DiagGroup<"pessimizing-return-by-const">; + def GNUPointerArith : DiagGroup<"gnu-pointer-arith">; def PointerArith : DiagGroup<"pointer-arith", [GNUPointerArith]>; @@ -990,6 +992,7 @@ MissingBraces, Move, MultiChar, + PessimizingReturnByConst, RangeLoopConstruct, Reorder, ReturnType, Index: clang/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6627,6 +6627,13 @@ InGroup, DefaultIgnore; def note_remove_move : Note<"remove std::move call here">; +def warn_pessimizing_return_by_const : Warning< + "const qualifying the return value prevents move semantics">, + InGroup; +def note_pessimizing_return_by_const : Note< + "copy assignment is invoked here even if move assignment " + "is supported for type %0">; + def warn_string_plus_int : Warning< "adding %0 to a string does not append to the string">, InGroup; Index: clang/include/clang/Sema/Sema.h =================================================================== --- clang/include/clang/Sema/Sema.h +++ clang/include/clang/Sema/Sema.h @@ -5070,6 +5070,9 @@ void DiagnoseSelfMove(const Expr *LHSExpr, const Expr *RHSExpr, SourceLocation OpLoc); + void DiagnosePessimizingConstReturn(const Expr *RHSExpr, + SourceLocation OpLoc); + /// Warn if we're implicitly casting from a _Nullable pointer type to a /// _Nonnull one. void diagnoseNullableToNonnullConversion(QualType DstType, QualType SrcType, Index: clang/lib/Sema/SemaChecking.cpp =================================================================== --- clang/lib/Sema/SemaChecking.cpp +++ clang/lib/Sema/SemaChecking.cpp @@ -16834,6 +16834,49 @@ << RHSExpr->getSourceRange(); } +void Sema::DiagnosePessimizingConstReturn(const Expr *RHSExpr, + SourceLocation OpLoc) { + + if (!getLangOpts().CPlusPlus11 || inTemplateInstantiation()) + return; + + RHSExpr = RHSExpr->IgnoreImplicit(); + + // Is RHS a function call expression? + const CallExpr *CE = dyn_cast(RHSExpr); + if (!CE) + return; + + const Decl *CD = CE->getCalleeDecl(); + if (!CD) + return; + + const FunctionDecl *FD = dyn_cast(CD); + if (!FD) + return; + + // Is the return type by-const-value of a struct or class type? + const QualType RT = FD->getReturnType(); + if (!RT->isStructureOrClassType() || !RT.isConstQualified()) + return; + + // Is the move assignment operator explicitly deleted? + bool MoveAssignmentIsDeleted = false; + for (const CXXMethodDecl *iter : RT->getAsCXXRecordDecl()->methods()) + if (iter->isMoveAssignmentOperator() && iter->isDeleted()) + MoveAssignmentIsDeleted = true; + + if (RT->getAsCXXRecordDecl()->hasDefinition() && + RT->getAsCXXRecordDecl()->hasMoveAssignment() && + RT->getAsCXXRecordDecl()->hasNonTrivialMoveAssignment() && + !MoveAssignmentIsDeleted) { + const SourceRange ReturnTypeLoc = FD->getReturnTypeSourceRange(); + Diag(ReturnTypeLoc.getBegin(), diag::warn_pessimizing_return_by_const); + Diag(OpLoc, diag::note_pessimizing_return_by_const) + << RT.getUnqualifiedType(); + } +} + //===--- Layout compatibility ----------------------------------------------// static bool isLayoutCompatible(ASTContext &C, QualType T1, QualType T2); Index: clang/lib/Sema/SemaOverload.cpp =================================================================== --- clang/lib/Sema/SemaOverload.cpp +++ clang/lib/Sema/SemaOverload.cpp @@ -13810,9 +13810,10 @@ ArgsArray = ArgsArray.slice(1); } - // Check for a self move. - if (Op == OO_Equal) + if (Op == OO_Equal) { DiagnoseSelfMove(Args[0], Args[1], OpLoc); + DiagnosePessimizingConstReturn(Args[1], OpLoc); + } if (ImplicitThis) { QualType ThisType = Context.getPointerType(ImplicitThis->getType()); Index: clang/test/Misc/warning-wall.c =================================================================== --- clang/test/Misc/warning-wall.c +++ clang/test/Misc/warning-wall.c @@ -33,6 +33,7 @@ CHECK-NEXT: -Wreturn-std-move CHECK-NEXT: -Wself-move CHECK-NEXT: -Wmultichar +CHECK-NEXT: -Wpessimizing-return-by-const CHECK-NEXT: -Wrange-loop-construct CHECK-NEXT: -Wreorder CHECK-NEXT: -Wreorder-ctor Index: clang/test/SemaCXX/warn-pessimizing-return-by-const.cpp =================================================================== --- /dev/null +++ clang/test/SemaCXX/warn-pessimizing-return-by-const.cpp @@ -0,0 +1,293 @@ +// RUN: %clang_cc1 -fsyntax-only -Wpessimizing-return-by-const -std=c++11 -verify %s + +// A class with non-trivial move assignment operator +struct NT1 { + char *d; + NT1() { + d = new char[1]; + d[0] = '\0'; + } + + NT1(const char *c_str) { + const int to_alloc = length(c_str) + 1; + d = new char[to_alloc]; + copy(d, c_str); + } + + ~NT1() { + delete[] d; + } + + NT1 &operator=(NT1 &&rhs) { + if (this == &rhs) + return *this; + delete[] d; + d = rhs.d; + rhs.d = new char[1]; + rhs.d[0] = '\0'; + return *this; + } + + NT1(const NT1 &rhs) { + const int to_alloc = length(rhs.d) + 1; + d = new char[to_alloc]; + copy(d, rhs.d); + } + + NT1 &operator=(const NT1 &rhs) { + if (this == &rhs) + return *this; + delete[] d; + const int to_alloc = length(rhs.d) + 1; + d = new char[to_alloc]; + copy(d, rhs.d); + return *this; + } + + // c_str : null-terminated c string. If empty, c_str[0] is '\0'. Cannot be nullptr. + // Returns the length of the string, excluding the null-terminating byte. + // If given an empty string, returns 0. + static int length(const char *c_str) { + int ret = 0; + while (*c_str++) + ++ret; + return ret; + } + + // dst : must be pre-allocated + // Copies src to dst, including the null-terminator. + static void copy(char *dst, const char *src) { + while ((*dst++ = *src++)) + ; + } + + bool operator==(const NT1 &rhs) { + if (length(d) != length(rhs.d)) + return false; + int cnt = 0; + const char *s1 = d; + const char *s2 = rhs.d; + while (cnt++ < length(d)) + if (*s1++ != *s2++) + return false; + return true; + } +}; + +NT1 f_nt1() { + return NT1{"f_nt1"}; +} + +const NT1 f_nt1_const() { // expected-warning{{const qualifying the return value prevents move semantics}} + return NT1{"f_nt1_const"}; +} + +void run_nt1() { + NT1 nt1; + nt1 = f_nt1(); + nt1 = f_nt1_const(); // expected-note{{copy assignment is invoked here even if move assignment is supported for type 'NT1'}} +} + +// NT2 has non-trivial move assignment operator since the move assignment operator selected +// to move the base class NT1 is non-trivial +struct NT2 : public NT1 { + NT2() : NT1() {} + NT2(const char *c_str) : NT1(c_str) {} +}; + +NT2 f_nt2() { + return NT2{"f_nt2"}; +} + +const NT2 f_nt2_const() { // expected-warning{{const qualifying the return value prevents move semantics}} + return NT2{"f_nt2_const"}; +} + +void run_nt2() { + NT2 nt2; + nt2 = f_nt2(); + nt2 = f_nt2_const(); // expected-note{{copy assignment is invoked here even if move assignment is supported for type 'NT2'}} +} + +// NT3 has non-trivial move assignment operator since the non-static data member of NT1 +// has non-trivial move assignment operator +struct NT3 { + NT1 nt1; + NT3() : nt1(NT1{}) {} + NT3(const char *c_str) : nt1(NT1(c_str)) {} + + bool operator==(const NT3 &rhs) { + return nt1 == rhs.nt1; + } +}; + +NT3 f_nt3() { + return NT3{"f_nt3"}; +} + +const NT3 f_nt3_const() { // expected-warning{{const qualifying the return value prevents move semantics}} + return NT3{"f_nt3_const"}; +} + +void run_nt3() { + NT3 nt3; + nt3 = f_nt3(); + nt3 = f_nt3_const(); // expected-note{{copy assignment is invoked here even if move assignment is supported for type 'NT3'}} +} + +// NT4 has defaulted move assignment operator which is still non-trivial +struct NT4 { + NT1 nt1; + NT4() : nt1(NT1{}) {} + NT4(const char *c_str) : nt1(NT1(c_str)) {} + + NT4 &operator=(NT4 &&) = default; + + NT4(const NT4 &) = default; + NT4 &operator=(const NT4 &) = default; + + bool operator==(const NT4 &rhs) { + return nt1 == rhs.nt1; + } +}; + +NT4 f_nt4() { + return NT4{"f_nt4"}; +} + +const NT4 f_nt4_const() { // expected-warning{{const qualifying the return value prevents move semantics}} + return NT4{"f_nt4_const"}; +} + +void run_nt4() { + NT4 nt4; + nt4 = f_nt4(); + nt4 = f_nt4_const(); // expected-note{{copy assignment is invoked here even if move assignment is supported for type 'NT4'}} +} + +// NT5 have deleted move assignment operator. We do not warn in this case +struct NT5 { + NT1 nt1; + NT5() : nt1(NT1{}) {} + NT5(const char *c_str) : nt1(NT1(c_str)) {} + + NT5 &operator=(NT5 &&) = delete; + + NT5(const NT5 &) = default; + NT5 &operator=(const NT5 &) = default; + + bool operator==(const NT5 &rhs) { + return nt1 == rhs.nt1; + } +}; + +const NT5 f_nt5_const() { + return NT5{"f_nt5_const"}; +} + +void run_nt5() { + NT5 nt5; + nt5 = f_nt5_const(); +} + +// NT6 has deleted move assignment operator because its member(NT5) has deleted move assignment operator. +// We do not warn in this case. +struct NT6 { + NT5 nt5; +}; + +const NT6 f_nt6_const() { + return NT6{"f_nt6_const"}; +} + +void run_nt6() { + NT6 nt6; + nt6 = f_nt6_const(); +} + +// warning is suppressed for trivial move assignment operator + +struct T1 { + int i1, i2, i3, i4; +}; + +const T1 f_t1_const() { + return T1{}; +} + +void run_t1() { + T1 t1; + t1 = f_t1_const(); +} + +struct T2 { + T2() = default; + T2(const T2 &) = default; + T2 &operator=(const T2 &) = default; + T2 &operator=(T2 &&) = default; +}; + +const T2 f_t2_const() { + return T2{}; +} + +void run_t2() { + T2 t2; + t2 = f_t2_const(); +} + +struct T3 { + T3() = default; + T3(const T3 &) = default; + T3 &operator=(const T3 &) = default; +}; + +const T3 f_t3_const() { + return T3{}; +} + +void run_t3() { + T3 t3; + t3 = f_t3_const(); +} + +struct T4 { + T4() = default; + T4(const T4 &) = default; + T4 &operator=(const T4 &) = default; + T4 &operator=(T4 &&) = delete; +}; + +const T4 f_t4_const() { + return T4{}; +} + +void run_t4() { + T4 t4; + t4 = f_t4_const(); +} + +struct T5 { + T1 t1; +}; + +const T5 f_t5_const() { + return T5{}; +} + +void run_t5() { + T5 t5; + t5 = f_t5_const(); +} + +struct T6 : public T1 { +}; + +const T6 f_t6_const() { + return T6{}; +} + +void run_t6() { + T6 t6; + t6 = f_t6_const(); +}