From 53472c9fc35126a8473db35e8ea8daabadf761b4 Mon Sep 17 00:00:00 2001 From: Richard Trieu Date: Sat, 10 Jan 2015 06:04:18 +0000 Subject: [PATCH] Add a new warning, -Wself-move, to Clang. -Wself-move is similiar to -Wself-assign. This warning is triggered when a value is attempted to be moved to itself. See r221008 for a bug that would have been caught with this warning. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@225581 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticGroups.td | 2 + include/clang/Basic/DiagnosticSemaKinds.td | 3 + lib/Sema/SemaExpr.cpp | 88 +++++++++++++++++++++- test/SemaCXX/warn-self-move.cpp | 40 ++++++++++ 4 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 test/SemaCXX/warn-self-move.cpp diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td index d4610e0a3f..75d40b17a6 100644 --- a/include/clang/Basic/DiagnosticGroups.td +++ b/include/clang/Basic/DiagnosticGroups.td @@ -294,6 +294,7 @@ def BindToTemporaryCopy : DiagGroup<"bind-to-temporary-copy", [CXX98CompatBindToTemporaryCopy]>; def SelfAssignmentField : DiagGroup<"self-assign-field">; def SelfAssignment : DiagGroup<"self-assign", [SelfAssignmentField]>; +def SelfMove : DiagGroup<"self-move">; def SemiBeforeMethodBody : DiagGroup<"semicolon-before-method-body">; def Sentinel : DiagGroup<"sentinel">; def MissingMethodReturnType : DiagGroup<"missing-method-return-type">; @@ -586,6 +587,7 @@ def Most : DiagGroup<"most", [ Reorder, ReturnType, SelfAssignment, + SelfMove, SizeofArrayArgument, SizeofArrayDecay, StringPlusInt, diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index cfe7b99b93..4bf83b6aed 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -4701,6 +4701,9 @@ def warn_addition_in_bitshift : Warning< def warn_self_assignment : Warning< "explicitly assigning value of variable of type %0 to itself">, InGroup, DefaultIgnore; +def warn_self_move : Warning< + "explicitly moving variable of type %0 to itself">, + InGroup, DefaultIgnore; def warn_string_plus_int : Warning< "adding %0 to a string does not append to the string">, diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 68bad16ff3..c912520983 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -9380,6 +9380,90 @@ static inline UnaryOperatorKind ConvertTokenKindToUnaryOpcode( return Opc; } +/// DiagnoseSelfMove - Emits a warning if a value is moved to itself. +static void DiagnoseSelfMove(Sema &S, const Expr *LHSExpr, const Expr *RHSExpr, + SourceLocation OpLoc) { + if (!S.ActiveTemplateInstantiations.empty()) + return; + + // Strip parens and casts away. + LHSExpr = LHSExpr->IgnoreParenImpCasts(); + RHSExpr = RHSExpr->IgnoreParenImpCasts(); + + // Check for a call expression + const CallExpr *CE = dyn_cast(RHSExpr); + if (!CE || CE->getNumArgs() != 1) + return; + + // Check for a call to std::move + const FunctionDecl *FD = CE->getDirectCallee(); + if (!FD || !FD->isInStdNamespace() || !FD->getIdentifier() || + !FD->getIdentifier()->isStr("move")) + return; + + // Get argument from std::move + RHSExpr = CE->getArg(0); + + const DeclRefExpr *LHSDeclRef = dyn_cast(LHSExpr); + const DeclRefExpr *RHSDeclRef = dyn_cast(RHSExpr); + + // Two DeclRefExpr's, check that the decls are the same. + if (LHSDeclRef && RHSDeclRef) { + if (!LHSDeclRef->getDecl() || !RHSDeclRef->getDecl()) + return; + if (LHSDeclRef->getDecl()->getCanonicalDecl() != + RHSDeclRef->getDecl()->getCanonicalDecl()) + return; + + S.Diag(OpLoc, diag::warn_self_move) << LHSExpr->getType() + << LHSExpr->getSourceRange() + << RHSExpr->getSourceRange(); + return; + } + + // Member variables require a different approach to check for self moves. + // MemberExpr's are the same if every nested MemberExpr refers to the same + // Decl and that the base Expr's are DeclRefExpr's with the same Decl or + // the base Expr's are CXXThisExpr's. + const Expr *LHSBase = LHSExpr; + const Expr *RHSBase = RHSExpr; + const MemberExpr *LHSME = dyn_cast(LHSExpr); + const MemberExpr *RHSME = dyn_cast(RHSExpr); + if (!LHSME || !RHSME) + return; + + while (LHSME && RHSME) { + if (LHSME->getMemberDecl()->getCanonicalDecl() != + RHSME->getMemberDecl()->getCanonicalDecl()) + return; + + LHSBase = LHSME->getBase(); + RHSBase = RHSME->getBase(); + LHSME = dyn_cast(LHSBase); + RHSME = dyn_cast(RHSBase); + } + + LHSDeclRef = dyn_cast(LHSBase); + RHSDeclRef = dyn_cast(RHSBase); + if (LHSDeclRef && RHSDeclRef) { + if (!LHSDeclRef->getDecl() || !RHSDeclRef->getDecl()) + return; + if (LHSDeclRef->getDecl()->getCanonicalDecl() != + RHSDeclRef->getDecl()->getCanonicalDecl()) + return; + + S.Diag(OpLoc, diag::warn_self_move) << LHSExpr->getType() + << LHSExpr->getSourceRange() + << RHSExpr->getSourceRange(); + return; + } + + if (isa(LHSBase) && isa(RHSBase)) + S.Diag(OpLoc, diag::warn_self_move) << LHSExpr->getType() + << LHSExpr->getSourceRange() + << RHSExpr->getSourceRange(); +} + /// DiagnoseSelfAssignment - Emits a warning if a value is assigned to itself. /// This warning is only emitted for builtin assignment operations. It is also /// suppressed in the event of macro expansions. @@ -9507,8 +9591,10 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, VK = LHS.get()->getValueKind(); OK = LHS.get()->getObjectKind(); } - if (!ResultTy.isNull()) + if (!ResultTy.isNull()) { DiagnoseSelfAssignment(*this, LHS.get(), RHS.get(), OpLoc); + DiagnoseSelfMove(*this, LHS.get(), RHS.get(), OpLoc); + } RecordModifiableNonNullParam(*this, LHS.get()); break; case BO_PtrMemD: diff --git a/test/SemaCXX/warn-self-move.cpp b/test/SemaCXX/warn-self-move.cpp new file mode 100644 index 0000000000..ae2dab1de1 --- /dev/null +++ b/test/SemaCXX/warn-self-move.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -fsyntax-only -Wself-move -std=c++11 -verify %s + +// definitions for std::move +namespace std { +inline namespace foo { +template struct remove_reference { typedef T type; }; +template struct remove_reference { typedef T type; }; +template struct remove_reference { typedef T type; }; + +template typename remove_reference::type &&move(T &&t); +} +} + +void int_test() { + int x = 5; + x = std::move(x); // expected-warning{{explicitly moving}} + (x) = std::move(x); // expected-warning{{explicitly moving}} + + using std::move; + x = move(x); // expected-warning{{explicitly moving}} +} + +int global; +void global_int_test() { + global = std::move(global); // expected-warning{{explicitly moving}} + (global) = std::move(global); // expected-warning{{explicitly moving}} + + using std::move; + global = move(global); // expected-warning{{explicitly moving}} +} + +class field_test { + int x; + field_test(field_test&& other) { + x = std::move(x); // expected-warning{{explicitly moving}} + x = std::move(other.x); + other.x = std::move(x); + other.x = std::move(other.x); // expected-warning{{explicitly moving}} + } +}; -- 2.40.0