]> granicus.if.org Git - clang/commitdiff
Add a new warning, -Wself-move, to Clang.
authorRichard Trieu <rtrieu@google.com>
Sat, 10 Jan 2015 06:04:18 +0000 (06:04 +0000)
committerRichard Trieu <rtrieu@google.com>
Sat, 10 Jan 2015 06:04:18 +0000 (06:04 +0000)
-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
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaExpr.cpp
test/SemaCXX/warn-self-move.cpp [new file with mode: 0644]

index d4610e0a3ff1a23ad1f6fb129b878f39ad934d75..75d40b17a67c482d3145a587e95bcc51ffc2e3d5 100644 (file)
@@ -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,
index cfe7b99b932185d0321189e6b2de8fb32b761a1f..4bf83b6aede31afe16ec378355f7090642b6a8b7 100644 (file)
@@ -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">,
index 68bad16ff38cf671d85e2e6c5ace19d35841d67f..c9125209839e17851a4e1a9289b28e94353f9503 100644 (file)
@@ -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<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(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 (file)
index 0000000..ae2dab1
--- /dev/null
@@ -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}}
+  }
+};