steakhal added reviewers: Szelethus, steakhal.
steakhal added a comment.

tldr; static-analyzer fixits are not completely implemented. We don't even have 
tests for it, which is a shame.
When I passed the `apply-fixits`, it modified the input source file - as I 
expected.
Then I tried the `-analyzer-output=text` and suddenly it inserted the fixit 2 
times xD, which is less than ideal and we should fix this.
And I'm expecting many more bugs with this feature.

There is a `clang/test/Analysis/check-analyzer-fixit.py` script which could be 
invoked by a RUN line like this:

  // RUN: %check-analyzer-fixit %s %t -analyzer-checker=core

At least, the comment of that script says so.

---

I'm quite skeptical about inserting such fixits in general - regarding 
path-sensitive analysis.
For fixits, we should be confident that some property holds, but the static 
analyzer might conclude erroneously that a given pointer is null resulting in a 
**bad** fixit.
Thus, relying on this might be dangerous.
You can read more about false-positive cases regarding null pointers in the 
`clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def`, namely 
`suppress-null-return-paths`, `avoid-suppressing-null-argument-paths`, 
`suppress-inlined-defensive-checks`. These options were introduced for 
mitigating such false-positive null pointer deref reports.

Let me invite @Szelethus for his expertise in the null pointer checker.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123352/new/

https://reviews.llvm.org/D123352

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to