donat.nagy added a comment. In D156312#4589251 <https://reviews.llvm.org/D156312#4589251>, @steakhal wrote:
> I don't think we should do anything about it unless it's frequent enough. > Try to come up with a heuristic to be able to measure how often this happens, > if you really care. > Once you have a semi-working heuristic for determining if that bugpath > suffers from this, you can as well use it for marking the bugreport invalid > if that's the case to lean towards suppressing those FPs. Minor clarification: these are not FPs, these are true positives with a bad error message. I was annoyed when I found this surprising bug on the almost-ready checker that I was working on; but I wouldn't say that this is an especially severe issue. In D156312#4589251 <https://reviews.llvm.org/D156312#4589251>, @steakhal wrote: > I don't think it's completely necessary to fix those FPs to land this. I > think of that as an improvement, on top of this one. > I hope I clarified my standpoint. I agree, I'll create a new revision which mentions this checker in the release notes, and I hope that I'll be able to merge that. Later I'll try to return to this question with either an engine improvement or a heuristic mitigation. ================ Comment at: clang/test/Analysis/bitwise-shift-sanity-checks.c:70-74 + if (right < 0) + return 0; + return left << (right + 32); + // expected - warning@-1 {{Left shift overflows the capacity of 'int'}} + // expected-warning@-2 {{Right operand is negative in left shift}} ---------------- steakhal wrote: > Let's be explicit about the actual assumed value range, and use > `clang_analyzer_value()`. > I also recommend using an explicit FIXME for calling out what should be > there, instead of abusing a non-matching `expected` pattern. I know I used > that in the past, and probably seen it from me. I feel ashamed that I did > that. I know I did that to have cleaner diffs, once fixed, but I honestly > think it does more harm than good. I already tried calling `clang_analyzer_value()` at that point when I was investigating, but unfortunately it won't say anything useful because the actual range corresponding to the symbolic expression is only calculated when the `evalBinOp()` calls examine it. I'll change the "expected - warning" to a FIXME. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156312/new/ https://reviews.llvm.org/D156312 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
