r225581 - Add a new warning, -Wself-move, to Clang.
Jordan Rose
jordan_rose at apple.com
Mon Jan 12 13:52:34 PST 2015
Out of curiosity, why is this a separate warning group and not just part of -Wself-assign? Or at least a subgroup?
Jordan
> On Jan 9, 2015, at 22:04, Richard Trieu <rtrieu at google.com> wrote:
>
> Author: rtrieu
> Date: Sat Jan 10 00:04:18 2015
> New Revision: 225581
>
> URL: http://llvm.org/viewvc/llvm-project?rev=225581&view=rev
> Log:
> 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.
>
>
> Added:
> cfe/trunk/test/SemaCXX/warn-self-move.cpp
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/Sema/SemaExpr.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=225581&r1=225580&r2=225581&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Sat Jan 10 00:04:18 2015
> @@ -294,6 +294,7 @@ def BindToTemporaryCopy : DiagGroup<"bin
> [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,
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=225581&r1=225580&r2=225581&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sat Jan 10 00:04:18 2015
> @@ -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<SelfAssignment>, DefaultIgnore;
> +def warn_self_move : Warning<
> + "explicitly moving variable of type %0 to itself">,
> + InGroup<SelfMove>, DefaultIgnore;
>
> def warn_string_plus_int : Warning<
> "adding %0 to a string does not append to the string">,
>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=225581&r1=225580&r2=225581&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Sat Jan 10 00:04:18 2015
> @@ -9380,6 +9380,90 @@ static inline UnaryOperatorKind ConvertT
> 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<CallExpr>(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<DeclRefExpr>(LHSExpr);
> + const DeclRefExpr *RHSDeclRef = dyn_cast<DeclRefExpr>(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<MemberExpr>(LHSExpr);
> + const MemberExpr *RHSME = dyn_cast<MemberExpr>(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<MemberExpr>(LHSBase);
> + RHSME = dyn_cast<MemberExpr>(RHSBase);
> + }
> +
> + LHSDeclRef = dyn_cast<DeclRefExpr>(LHSBase);
> + RHSDeclRef = dyn_cast<DeclRefExpr>(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<CXXThisExpr>(LHSBase) && isa<CXXThisExpr>(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(Sour
> 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:
>
> Added: cfe/trunk/test/SemaCXX/warn-self-move.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-self-move.cpp?rev=225581&view=auto
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/warn-self-move.cpp (added)
> +++ cfe/trunk/test/SemaCXX/warn-self-move.cpp Sat Jan 10 00:04:18 2015
> @@ -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 <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);
> +}
> +}
> +
> +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}}
> + }
> +};
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list