r225581 - Add a new warning, -Wself-move, to Clang.
Richard Trieu
rtrieu at google.com
Mon Jan 12 15:56:03 PST 2015
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.
On Mon, Jan 12, 2015 at 1:52 PM, Jordan Rose <jordan_rose at apple.com> wrote:
> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150112/ca6e63c2/attachment.html>
More information about the cfe-commits
mailing list