[PATCH] D49112: [Sema] Implement -Wmemset-transposed-args

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:662
+def note_suspicious_sizeof_memset_silence : Note<
+  "%select{parenthesize the third argument|cast the second argument to 'int'}0 to silence">;
+
----------------
aaron.ballman wrote:
> erik.pilkington wrote:
> > Quuxplusone wrote:
> > > If it were my codebase, I'd rather see a cast to `(unsigned char)` than a cast to `(int)`. (The second argument to memset is supposed to be a single byte.) Why did you pick `(int)` specifically?
> > I chose `int` because that's the actual type of the second parameter to `memset`, it just gets casted down to `unsigned char` internally. FWIW, either type will suppress the warning. I'm fine with recommending `unsigned char` if you have a strong preference for it.
> My preference is for `int` as well.
Personally I have a strong preference for "any 8-bit type" — the important thing is that it needs to indicate to the reader that it's intended to be the single-byte value that `memset` expects. I even want my compiler to diagnose `memset(x, 0x1234, z)` as a bug!
But I'm not inclined to fight hard, because this is all about the mechanism of *suppressing* of the warning, and I don't imagine we'll see too many people trying to suppress it.


https://reviews.llvm.org/D49112