Extend the self move warning to record types.
authorRichard Trieu <rtrieu@google.com>
Tue, 13 Jan 2015 02:32:02 +0000 (02:32 +0000)
committerRichard Trieu <rtrieu@google.com>
Tue, 13 Jan 2015 02:32:02 +0000 (02:32 +0000)
Move the logic for checking self moves into SemaChecking and add that function
to Sema since it is now used in multiple places.

llvm-svn: 225756

clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/SemaCXX/warn-self-move.cpp

index 95fc68f..2ba74b1 100644 (file)
@@ -3326,6 +3326,10 @@ public:
   void DiagnoseEmptyLoopBody(const Stmt *S,
                              const Stmt *PossibleBody);
 
+  /// Warn if a value is moved to itself.
+  void DiagnoseSelfMove(const Expr *LHSExpr, const Expr *RHSExpr,
+                        SourceLocation OpLoc);
+
   ParsingDeclState PushParsingDeclaration(sema::DelayedDiagnosticPool &pool) {
     return DelayedDiagnostics.push(pool);
   }
index d14549c..9fc2bec 100644 (file)
@@ -8325,6 +8325,96 @@ void Sema::DiagnoseEmptyLoopBody(const Stmt *S,
   }
 }
 
+//===--- CHECK: Warn on self move with std::move. -------------------------===//
+
+/// DiagnoseSelfMove - Emits a warning if a value is moved to itself.
+void Sema::DiagnoseSelfMove(const Expr *LHSExpr, const Expr *RHSExpr,
+                             SourceLocation OpLoc) {
+
+  if (Diags.isIgnored(diag::warn_sizeof_pointer_expr_memaccess, OpLoc))
+    return;
+
+  if (!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;
+
+    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;
+
+    Diag(OpLoc, diag::warn_self_move) << LHSExpr->getType()
+                                        << LHSExpr->getSourceRange()
+                                        << RHSExpr->getSourceRange();
+    return;
+  }
+
+  if (isa<CXXThisExpr>(LHSBase) && isa<CXXThisExpr>(RHSBase))
+    Diag(OpLoc, diag::warn_self_move) << LHSExpr->getType()
+                                        << LHSExpr->getSourceRange()
+                                        << RHSExpr->getSourceRange();
+}
+
 //===--- Layout compatibility ----------------------------------------------//
 
 namespace {
index cd953ab..59efbc1 100644 (file)
@@ -9380,90 +9380,6 @@ 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.
@@ -9593,7 +9509,7 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
     }
     if (!ResultTy.isNull()) {
       DiagnoseSelfAssignment(*this, LHS.get(), RHS.get(), OpLoc);
-      DiagnoseSelfMove(*this, LHS.get(), RHS.get(), OpLoc);
+      DiagnoseSelfMove(LHS.get(), RHS.get(), OpLoc);
     }
     RecordModifiableNonNullParam(*this, LHS.get());
     break;
index 8341c00..9195ee5 100644 (file)
@@ -11199,7 +11199,12 @@ Sema::CreateOverloadedBinOp(SourceLocation OpLoc,
         // Cut off the implicit 'this'.
         if (isa<CXXMethodDecl>(FnDecl))
           ArgsArray = ArgsArray.slice(1);
-        checkCall(FnDecl, ArgsArray, 0, isa<CXXMethodDecl>(FnDecl), OpLoc, 
+
+        // Check for a self move.
+        if (Op == OO_Equal)
+          DiagnoseSelfMove(Args[0], Args[1], OpLoc);
+
+        checkCall(FnDecl, ArgsArray, 0, isa<CXXMethodDecl>(FnDecl), OpLoc,
                   TheCall->getSourceRange(), VariadicDoesNotApply);
 
         return MaybeBindToTemporary(TheCall);
index ae2dab1..23778c1 100644 (file)
@@ -38,3 +38,18 @@ class field_test {
     other.x = std::move(other.x);  // expected-warning{{explicitly moving}}
   }
 };
+
+struct A {};
+struct B { A a; };
+struct C { C() {}; ~C() {} };
+void struct_test() {
+  A a;
+  a = std::move(a);  // expected-warning{{explicitly moving}}
+
+  B b;
+  b = std::move(b);  // expected-warning{{explicitly moving}}
+  b.a = std::move(b.a);  // expected-warning{{explicitly moving}}
+
+  C c;
+  c = std::move(c);  // expected-warning{{explicitly moving}}
+}