<div dir="ltr">There's been several requests for warnings with std::move. I was planning a creating a new warning group for all of them once a few more get implemented.</div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jan 12, 2015 at 1:52 PM, Jordan Rose <span dir="ltr"><<a href="mailto:jordan_rose@apple.com" target="_blank">jordan_rose@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Out of curiosity, why is this a separate warning group and not just part of -Wself-assign? Or at least a subgroup?<br>
<span class="HOEnZb"><font color="#888888"><br>
Jordan<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
> On Jan 9, 2015, at 22:04, Richard Trieu <<a href="mailto:rtrieu@google.com">rtrieu@google.com</a>> wrote:<br>
><br>
> Author: rtrieu<br>
> Date: Sat Jan 10 00:04:18 2015<br>
> New Revision: 225581<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=225581&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=225581&view=rev</a><br>
> Log:<br>
> Add a new warning, -Wself-move, to Clang.<br>
><br>
> -Wself-move is similiar to -Wself-assign. This warning is triggered when<br>
> a value is attempted to be moved to itself. See r221008 for a bug that<br>
> would have been caught with this warning.<br>
><br>
><br>
> Added:<br>
> cfe/trunk/test/SemaCXX/warn-self-move.cpp<br>
> Modified:<br>
> cfe/trunk/include/clang/Basic/DiagnosticGroups.td<br>
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
> cfe/trunk/lib/Sema/SemaExpr.cpp<br>
><br>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=225581&r1=225580&r2=225581&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=225581&r1=225580&r2=225581&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)<br>
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Sat Jan 10 00:04:18 2015<br>
> @@ -294,6 +294,7 @@ def BindToTemporaryCopy : DiagGroup<"bin<br>
> [CXX98CompatBindToTemporaryCopy]>;<br>
> def SelfAssignmentField : DiagGroup<"self-assign-field">;<br>
> def SelfAssignment : DiagGroup<"self-assign", [SelfAssignmentField]>;<br>
> +def SelfMove : DiagGroup<"self-move">;<br>
> def SemiBeforeMethodBody : DiagGroup<"semicolon-before-method-body">;<br>
> def Sentinel : DiagGroup<"sentinel">;<br>
> def MissingMethodReturnType : DiagGroup<"missing-method-return-type">;<br>
> @@ -586,6 +587,7 @@ def Most : DiagGroup<"most", [<br>
> Reorder,<br>
> ReturnType,<br>
> SelfAssignment,<br>
> + SelfMove,<br>
> SizeofArrayArgument,<br>
> SizeofArrayDecay,<br>
> StringPlusInt,<br>
><br>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=225581&r1=225580&r2=225581&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=225581&r1=225580&r2=225581&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sat Jan 10 00:04:18 2015<br>
> @@ -4701,6 +4701,9 @@ def warn_addition_in_bitshift : Warning<<br>
> def warn_self_assignment : Warning<<br>
> "explicitly assigning value of variable of type %0 to itself">,<br>
> InGroup<SelfAssignment>, DefaultIgnore;<br>
> +def warn_self_move : Warning<<br>
> + "explicitly moving variable of type %0 to itself">,<br>
> + InGroup<SelfMove>, DefaultIgnore;<br>
><br>
> def warn_string_plus_int : Warning<<br>
> "adding %0 to a string does not append to the string">,<br>
><br>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=225581&r1=225580&r2=225581&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=225581&r1=225580&r2=225581&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)<br>
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Sat Jan 10 00:04:18 2015<br>
> @@ -9380,6 +9380,90 @@ static inline UnaryOperatorKind ConvertT<br>
> return Opc;<br>
> }<br>
><br>
> +/// DiagnoseSelfMove - Emits a warning if a value is moved to itself.<br>
> +static void DiagnoseSelfMove(Sema &S, const Expr *LHSExpr, const Expr *RHSExpr,<br>
> + SourceLocation OpLoc) {<br>
> + if (!S.ActiveTemplateInstantiations.empty())<br>
> + return;<br>
> +<br>
> + // Strip parens and casts away.<br>
> + LHSExpr = LHSExpr->IgnoreParenImpCasts();<br>
> + RHSExpr = RHSExpr->IgnoreParenImpCasts();<br>
> +<br>
> + // Check for a call expression<br>
> + const CallExpr *CE = dyn_cast<CallExpr>(RHSExpr);<br>
> + if (!CE || CE->getNumArgs() != 1)<br>
> + return;<br>
> +<br>
> + // Check for a call to std::move<br>
> + const FunctionDecl *FD = CE->getDirectCallee();<br>
> + if (!FD || !FD->isInStdNamespace() || !FD->getIdentifier() ||<br>
> + !FD->getIdentifier()->isStr("move"))<br>
> + return;<br>
> +<br>
> + // Get argument from std::move<br>
> + RHSExpr = CE->getArg(0);<br>
> +<br>
> + const DeclRefExpr *LHSDeclRef = dyn_cast<DeclRefExpr>(LHSExpr);<br>
> + const DeclRefExpr *RHSDeclRef = dyn_cast<DeclRefExpr>(RHSExpr);<br>
> +<br>
> + // Two DeclRefExpr's, check that the decls are the same.<br>
> + if (LHSDeclRef && RHSDeclRef) {<br>
> + if (!LHSDeclRef->getDecl() || !RHSDeclRef->getDecl())<br>
> + return;<br>
> + if (LHSDeclRef->getDecl()->getCanonicalDecl() !=<br>
> + RHSDeclRef->getDecl()->getCanonicalDecl())<br>
> + return;<br>
> +<br>
> + S.Diag(OpLoc, diag::warn_self_move) << LHSExpr->getType()<br>
> + << LHSExpr->getSourceRange()<br>
> + << RHSExpr->getSourceRange();<br>
> + return;<br>
> + }<br>
> +<br>
> + // Member variables require a different approach to check for self moves.<br>
> + // MemberExpr's are the same if every nested MemberExpr refers to the same<br>
> + // Decl and that the base Expr's are DeclRefExpr's with the same Decl or<br>
> + // the base Expr's are CXXThisExpr's.<br>
> + const Expr *LHSBase = LHSExpr;<br>
> + const Expr *RHSBase = RHSExpr;<br>
> + const MemberExpr *LHSME = dyn_cast<MemberExpr>(LHSExpr);<br>
> + const MemberExpr *RHSME = dyn_cast<MemberExpr>(RHSExpr);<br>
> + if (!LHSME || !RHSME)<br>
> + return;<br>
> +<br>
> + while (LHSME && RHSME) {<br>
> + if (LHSME->getMemberDecl()->getCanonicalDecl() !=<br>
> + RHSME->getMemberDecl()->getCanonicalDecl())<br>
> + return;<br>
> +<br>
> + LHSBase = LHSME->getBase();<br>
> + RHSBase = RHSME->getBase();<br>
> + LHSME = dyn_cast<MemberExpr>(LHSBase);<br>
> + RHSME = dyn_cast<MemberExpr>(RHSBase);<br>
> + }<br>
> +<br>
> + LHSDeclRef = dyn_cast<DeclRefExpr>(LHSBase);<br>
> + RHSDeclRef = dyn_cast<DeclRefExpr>(RHSBase);<br>
> + if (LHSDeclRef && RHSDeclRef) {<br>
> + if (!LHSDeclRef->getDecl() || !RHSDeclRef->getDecl())<br>
> + return;<br>
> + if (LHSDeclRef->getDecl()->getCanonicalDecl() !=<br>
> + RHSDeclRef->getDecl()->getCanonicalDecl())<br>
> + return;<br>
> +<br>
> + S.Diag(OpLoc, diag::warn_self_move) << LHSExpr->getType()<br>
> + << LHSExpr->getSourceRange()<br>
> + << RHSExpr->getSourceRange();<br>
> + return;<br>
> + }<br>
> +<br>
> + if (isa<CXXThisExpr>(LHSBase) && isa<CXXThisExpr>(RHSBase))<br>
> + S.Diag(OpLoc, diag::warn_self_move) << LHSExpr->getType()<br>
> + << LHSExpr->getSourceRange()<br>
> + << RHSExpr->getSourceRange();<br>
> +}<br>
> +<br>
> /// DiagnoseSelfAssignment - Emits a warning if a value is assigned to itself.<br>
> /// This warning is only emitted for builtin assignment operations. It is also<br>
> /// suppressed in the event of macro expansions.<br>
> @@ -9507,8 +9591,10 @@ ExprResult Sema::CreateBuiltinBinOp(Sour<br>
> VK = LHS.get()->getValueKind();<br>
> OK = LHS.get()->getObjectKind();<br>
> }<br>
> - if (!ResultTy.isNull())<br>
> + if (!ResultTy.isNull()) {<br>
> DiagnoseSelfAssignment(*this, LHS.get(), RHS.get(), OpLoc);<br>
> + DiagnoseSelfMove(*this, LHS.get(), RHS.get(), OpLoc);<br>
> + }<br>
> RecordModifiableNonNullParam(*this, LHS.get());<br>
> break;<br>
> case BO_PtrMemD:<br>
><br>
> Added: cfe/trunk/test/SemaCXX/warn-self-move.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-self-move.cpp?rev=225581&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-self-move.cpp?rev=225581&view=auto</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/SemaCXX/warn-self-move.cpp (added)<br>
> +++ cfe/trunk/test/SemaCXX/warn-self-move.cpp Sat Jan 10 00:04:18 2015<br>
> @@ -0,0 +1,40 @@<br>
> +// RUN: %clang_cc1 -fsyntax-only -Wself-move -std=c++11 -verify %s<br>
> +<br>
> +// definitions for std::move<br>
> +namespace std {<br>
> +inline namespace foo {<br>
> +template <class T> struct remove_reference { typedef T type; };<br>
> +template <class T> struct remove_reference<T&> { typedef T type; };<br>
> +template <class T> struct remove_reference<T&&> { typedef T type; };<br>
> +<br>
> +template <class T> typename remove_reference<T>::type &&move(T &&t);<br>
> +}<br>
> +}<br>
> +<br>
> +void int_test() {<br>
> + int x = 5;<br>
> + x = std::move(x); // expected-warning{{explicitly moving}}<br>
> + (x) = std::move(x); // expected-warning{{explicitly moving}}<br>
> +<br>
> + using std::move;<br>
> + x = move(x); // expected-warning{{explicitly moving}}<br>
> +}<br>
> +<br>
> +int global;<br>
> +void global_int_test() {<br>
> + global = std::move(global); // expected-warning{{explicitly moving}}<br>
> + (global) = std::move(global); // expected-warning{{explicitly moving}}<br>
> +<br>
> + using std::move;<br>
> + global = move(global); // expected-warning{{explicitly moving}}<br>
> +}<br>
> +<br>
> +class field_test {<br>
> + int x;<br>
> + field_test(field_test&& other) {<br>
> + x = std::move(x); // expected-warning{{explicitly moving}}<br>
> + x = std::move(other.x);<br>
> + other.x = std::move(x);<br>
> + other.x = std::move(other.x); // expected-warning{{explicitly moving}}<br>
> + }<br>
> +};<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br>
</div></div></blockquote></div><br></div>