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

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

Quuxplusone added inline comments.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:7984
+  if (isa<IntegerLiteral>(ThirdArg) &&
+      cast<IntegerLiteral>(ThirdArg)->getValue() == 0) {
+    WarningKind = 0;
----------------
> Suppress the diagnostic in cases like `memset(ptr, 0xff, PADDING)`, when `PADDING` is a macro defined to be `0`.

Would it suffice to treat a literal `0xFF` in the exact same way you treat a literal `0`? That is,
```
if (SecondArg is integer literal 0 or 255 || ThirdArg involves sizeof) {
    do nothing
} else if (ThirdArg is integer literal 0 or 255 || SecondArg involves sizeof) {
    diagnose it
}
```
You should add a test case proving that `memset(x, 0, 0)` doesn't get diagnosed (assuming the two `0`s come from two different macro expansions, for example).
My sketch above would fail to diagnose `memset(x, 0, 255)`. I don't know if this is a feature or a bug. :)  You probably ought to have a test case for that, either way, just to demonstrate the expected behavior.


https://reviews.llvm.org/D49112