[PATCH] D117568: [Analyzer] Add docs to StdCLibraryFunctionArgsChecker

Gabor Marton via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 19 07:13:33 PST 2022


martong added inline comments.


================
Comment at: clang/docs/analyzer/checkers.rst:2361
+    (void)ret;
+    clang_analyzer_eval(EOF <= x && x <= 255); // this reports TRUE
+  }
----------------
NoQ wrote:
> I recommend against using `clang_analyzer_eval` in user docs. Users aren't expected to know what it is.
Ok, I've changed to have an infeasible branch condition and below that an unreported div zero warning demonstrates the same.


================
Comment at: clang/docs/analyzer/checkers.rst:2366
+suppressed. However, the assumption about the argument is still modeled (otherwise we
+would be further analyzing an illformed program).
+
----------------
NoQ wrote:
> Nitpick: the program doesn't become ill-formed just because the user has turned off the checker. Maybe it's better to say that exploring an execution path that already contains undefined behavior is not valuable, or something along those lines(?)
Ok, I've changed the wording.


================
Comment at: clang/docs/analyzer/checkers.rst:2371
+diagnostics) for functions that are defined in the POSIX standard. This option
+is disabled by default.
+
----------------
dkrupp wrote:
> I think it would be useful for the user to see one example per constraint type that this checker supports.
> RangeConstraint (was covered), ComparisonConstraint, ValueConstraint, Not null  Constraint, BufferSize constraint etc.
> 
> 
> It would be also nice to add a section "Limitations".
> 
> Describe there well known false positive cases or limitations in the bug diagnostics that limits understandability.
> Essentially the most important well known cases why this checker is alpha.
> 
> This section would be useful for users to understand and help identifying cases that are known false positives and for the developers to know how to improve this checker. I remember many cases when we had to test multiple times "why a checker is in alpha", because we forgot about it. I think it is best to document it.
Ok, I've added a few paragraphs to describe these things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117568



More information about the cfe-commits mailing list