Change wording of 'memcpy' type mismatch warning and remove fixit.

As per comments following r157659.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@157722 91177308-0d34-0410-b5e6-96231b3b80d8

4 files changed, 29 insertions, 55 deletions

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 206cbe1056..32576a3de6 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -335,10 +335,14 @@ def warn_dyn_class_memaccess : Warning<
 def note_bad_memaccess_silence : Note<
   "explicitly cast the pointer to silence this warning">;
 def warn_sizeof_pointer_expr_memaccess : Warning<
-  "argument to 'sizeof' in '%0' call is the same expression as the "
-  "%select{destination|source}1; did you mean to "
-  "%select{dereference it|remove the addressof|provide an explicit length}2?">,
+  "'%0' call operates on objects of type %1 while the size is based on a " 
+  "different type %2">, 
   InGroup<DiagGroup<"sizeof-pointer-memaccess">>;
+def warn_sizeof_pointer_expr_memaccess_note : Note<
+  "did you mean to %select{dereference the argument to 'sizeof' (and multiply "
+  "it by the number of elements)|remove the addressof in the argument to "
+  "'sizeof' (and multiply it by the number of elements)|provide an explicit "
+  "length}0?">;
 def warn_sizeof_pointer_type_memaccess : Warning<
   "argument to 'sizeof' in %0 call is the same pointer type %1 as the "
   "%select{destination|source}2; expected %3 or an explicit length">,
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 004e9948f2..e35f45b89d 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -2755,24 +2755,14 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call,
           // TODO: For strncpy() and friends, this could suggest sizeof(dst)
           //       over sizeof(src) as well.
           unsigned ActionIdx = 0; // Default is to suggest dereferencing.
-          FixItHint Fixit = FixItHint(); // Default hint.
           StringRef ReadableName = FnName->getName();
 
-          if (isa<DeclRefExpr>(SizeOfArg))
-            Fixit = FixItHint::CreateInsertion(SizeOfArg->getLocStart(), "*");
-
           if (const UnaryOperator *UnaryOp = dyn_cast<UnaryOperator>(Dest))
-            if (UnaryOp->getOpcode() == UO_AddrOf) {
-              Fixit = FixItHint::CreateRemoval(
-                      CharSourceRange::getTokenRange(SizeOfArg->getLocStart(),
-                                                     SizeOfArg->getLocStart()));
+            if (UnaryOp->getOpcode() == UO_AddrOf)
               ActionIdx = 1; // If its an address-of operator, just remove it.
-            }
           if (Context.getTypeSize(PointeeTy) == Context.getCharWidth())
             ActionIdx = 2; // If the pointee's size is sizeof(char),
                            // suggest an explicit length.
-          unsigned DestSrcSelect =
-            (BId == Builtin::BIstrndup ? 1 : ArgIdx);
 
           // If the function is defined as a builtin macro, do not show macro
           // expansion.
@@ -2790,14 +2780,18 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call,
                              SM.getSpellingLoc(SSR.getEnd()));
           }
 
-          DiagRuntimeBehavior(SL, Dest,
+          DiagRuntimeBehavior(SL, SizeOfArg,
                               PDiag(diag::warn_sizeof_pointer_expr_memaccess)
                                 << ReadableName
-                                << DestSrcSelect
-                                << ActionIdx
+                                << PointeeTy
+                                << DestTy
                                 << DSR
-                                << SSR
-                                << Fixit);
+                                << SSR);
+          DiagRuntimeBehavior(SL, SizeOfArg,
+                         PDiag(diag::warn_sizeof_pointer_expr_memaccess_note)
+                                << ActionIdx
+                                << SSR);
+
           break;
         }
       }
diff --git a/test/SemaCXX/warn-memset-bad-sizeof-fixit.cpp b/test/SemaCXX/warn-memset-bad-sizeof-fixit.cpp
deleted file mode 100644
index f76f497393..0000000000
--- a/test/SemaCXX/warn-memset-bad-sizeof-fixit.cpp
+++ /dev/null
@@ -1,24 +0,0 @@
-// RUN: cp %s %t
-// RUN: not %clang_cc1 -fixit -Werror -x c++ -std=c++98 %t
-// RUN: %clang_cc1 -fsyntax-only -Werror -x c++ -std=c++98 %t
-// RUN: cp %s %t
-// RUN: not %clang_cc1 -DUSE_BUILTINS -fixit -Werror -x c++ -std=c++98 %t
-// RUN: %clang_cc1 -DUSE_BUILTINS -fsyntax-only -Werror -x c++ -std=c++98 %t
-
-extern "C" void *memcpy(void *s1, const void *s2, unsigned n);
-
-#ifdef USE_BUILTINS
-# define BUILTIN(f) __builtin_ ## f
-#else
-# define BUILTIN(f) f
-#endif
-
-#define memcpy BUILTIN(memcpy)
-
-int testFixits(int *to, int *from) {
-  memcpy(to, from, sizeof(to)); // \
-         // expected-warning {{argument to 'sizeof' in 'memcpy' call is the same expression as the destination; did you mean to dereference it?}}
-  memcpy(0, &from, sizeof(&from)); // \
-         // expected-warning {{argument to 'sizeof' in 'memcpy' call is the same expression as the source; did you mean to remove the addressof?}}
-  return 0;
-}
diff --git a/test/SemaCXX/warn-memset-bad-sizeof.cpp b/test/SemaCXX/warn-memset-bad-sizeof.cpp
index e0d28da3d5..e388634e88 100644
--- a/test/SemaCXX/warn-memset-bad-sizeof.cpp
+++ b/test/SemaCXX/warn-memset-bad-sizeof.cpp
@@ -35,27 +35,27 @@ void f(Mat m, const Foo& const_foo, char *buffer) {
 
   /* Should warn */
   memset(&s, 0, sizeof(&s));  // \
-      // expected-warning {{argument to 'sizeof' in 'memset' call is the same expression as the destination}}
+      // expected-warning {{'memset' call operates on objects of type 'S' while the size is based on a different type 'S *'}} expected-note{{did you mean to remove the addressof in the argument to 'sizeof' (and multiply it by the number of elements)?}}
   memset(ps, 0, sizeof(ps));  // \
-      // expected-warning {{argument to 'sizeof' in 'memset' call is the same expression as the destination}}
+      // expected-warning {{'memset' call operates on objects of type 'S' while the size is based on a different type 'S *'}} expected-note{{did you mean to dereference the argument to 'sizeof' (and multiply it by the number of elements)?}}
   memset(ps2, 0, sizeof(ps2));  // \
-      // expected-warning {{argument to 'sizeof' in 'memset' call is the same expression as the destination}}
+      // expected-warning {{'memset' call operates on objects of type 'S' while the size is based on a different type 'PS' (aka 'S *')}} expected-note{{did you mean to dereference the argument to 'sizeof' (and multiply it by the number of elements)?}}
   memset(ps2, 0, sizeof(typeof(ps2)));  // \
       // expected-warning {{argument to 'sizeof' in 'memset' call is the same pointer type}}
   memset(ps2, 0, sizeof(PS));  // \
       // expected-warning {{argument to 'sizeof' in 'memset' call is the same pointer type}}
   memset(heap_buffer, 0, sizeof(heap_buffer));  // \
-      // expected-warning {{argument to 'sizeof' in 'memset' call is the same expression as the destination}}
+      // expected-warning {{'memset' call operates on objects of type 'char' while the size is based on a different type 'char *'}} expected-note{{did you mean to provide an explicit length?}}
 
   memcpy(&s, 0, sizeof(&s));  // \
-      // expected-warning {{argument to 'sizeof' in 'memcpy' call is the same expression as the destination}}
+      // expected-warning {{'memcpy' call operates on objects of type 'S' while the size is based on a different type 'S *'}} expected-note{{did you mean to remove the addressof in the argument to 'sizeof' (and multiply it by the number of elements)?}}
   memcpy(0, &s, sizeof(&s));  // \
-      // expected-warning {{argument to 'sizeof' in 'memcpy' call is the same expression as the source}}
+      // expected-warning {{'memcpy' call operates on objects of type 'S' while the size is based on a different type 'S *'}} expected-note{{did you mean to remove the addressof in the argument to 'sizeof' (and multiply it by the number of elements)?}}
 
   memmove(ps, 0, sizeof(ps));  // \
-      // expected-warning {{argument to 'sizeof' in 'memmove' call is the same expression as the destination}}
+      // expected-warning {{'memmove' call operates on objects of type 'S' while the size is based on a different type 'S *'}} expected-note{{did you mean to dereference the argument to 'sizeof' (and multiply it by the number of elements)?}}
   memcmp(ps, 0, sizeof(ps));  // \
-      // expected-warning {{argument to 'sizeof' in 'memcmp' call is the same expression as the destination}}
+      // expected-warning {{'memcmp' call operates on objects of type 'S' while the size is based on a different type 'S *'}} expected-note{{did you mean to dereference the argument to 'sizeof' (and multiply it by the number of elements)?}}
 
   /* Shouldn't warn */
   memset((void*)&s, 0, sizeof(&s));
@@ -132,14 +132,14 @@ void strcpy_and_friends() {
   const char* BAR = "<- this, too";
 
   strncmp(FOO, BAR, sizeof(FOO)); // \
-      // expected-warning {{argument to 'sizeof' in 'strncmp' call is the same expression as the destination}}
+      // expected-warning {{'strncmp' call operates on objects of type 'const char' while the size is based on a different type 'const char *'}} expected-note{{did you mean to provide an explicit length?}}
   strncasecmp(FOO, BAR, sizeof(FOO));  // \
-      // expected-warning {{argument to 'sizeof' in 'strncasecmp' call is the same expression as the destination}}
+      // expected-warning {{'strncasecmp' call operates on objects of type 'const char' while the size is based on a different type 'const char *'}} expected-note{{did you mean to provide an explicit length?}}
 
   char buff[80];
 
   strncpy(buff, BAR, sizeof(BAR)); // \
-      // expected-warning {{argument to 'sizeof' in 'strncpy' call is the same expression as the source}}
+      // expected-warning {{'strncpy' call operates on objects of type 'const char' while the size is based on a different type 'const char *'}} expected-note{{did you mean to provide an explicit length?}}
   strndup(FOO, sizeof(FOO)); // \
-      // expected-warning {{argument to 'sizeof' in 'strndup' call is the same expression as the source}}
+      // expected-warning {{'strndup' call operates on objects of type 'const char' while the size is based on a different type 'const char *'}} expected-note{{did you mean to provide an explicit length?}}
 }