On Wed, May 30, 2012 at 5:06 PM, Anna Zaks <[email protected]> wrote: > > On May 30, 2012, at 4:22 PM, Matt Beaumont-Gay wrote: > >> On Wed, May 30, 2012 at 4:14 PM, Anna Zaks <[email protected]> wrote: >>> Author: zaks >>> Date: Wed May 30 18:14:52 2012 >>> New Revision: 157722 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=157722&view=rev >>> Log: >>> Change wording of 'memcpy' type mismatch warning and remove fixit. >>> >>> As per comments following r157659. >>> >>> Removed: >>> cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof-fixit.cpp >>> Modified: >>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >>> cfe/trunk/lib/Sema/SemaChecking.cpp >>> cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp >>> >>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=157722&r1=157721&r2=157722&view=diff >>> ============================================================================== >>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) >>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed May 30 >>> 18:14:52 2012 >>> @@ -335,10 +335,14 @@ >>> 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">, >> >> I'm not thrilled with this new wording. These functions operate on >> bytes, not objects. > > Is "values" better than "objects"? > > 'memcpy' is usually used to copy over a number of values of the given type. > Especially in the case the waring triggers (sizeof is used to compute the > size): > - memcpy(to, from, sizeof(to)); > + memcpy(to, from, sizeof(*to)*N);
I just looked over the cleanups I did when we turned this warning on in our codebase. Many of the buggy memcpy calls needed a count multiplier, but on the other hand, almost none of the buggy memset calls did. In general, I don't think we need to be any more clever or explicit in terms of diagnostic wording here, nor do I think we need a fixit (which we can't get right with particularly high confidence). Just pointing out the problem is hugely valuable, and it doesn't feel like guessing at what the programmer meant is going to add a lot more value. -Matt _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
