erik.pilkington added inline comments.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:7984
+ if (isa<IntegerLiteral>(ThirdArg) &&
+ cast<IntegerLiteral>(ThirdArg)->getValue() == 0) {
+ WarningKind = 0;
----------------
Quuxplusone wrote:
> > 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.
I definitely don't think that ThirdArg being 255 should lead to a diagnostic,
regardless of what SecondArg is. I'm fine with omitting a diagnostic in
`memset(ptr, 255, 0)`, but if the user explicitly spelled out `0` then I would
probably prefer to diagnose. FWIW, for the case I found where we emitted a
false-positive the second argument was 0xd9 or something.
https://reviews.llvm.org/D49112
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
