[PATCH] D149447: [clang][analyzer] Improve documentation of StdCLibraryFunctionArgs checker (NFC)

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus requested changes to this revision.
Szelethus added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang/docs/analyzer/checkers.rst:2457
 If the user disables the checker then the argument violation warning is
 suppressed. However, the assumption about the argument is still modeled. This
 is because exploring an execution path that already contains undefined behavior
----------------
This makes sense to me, and likely all of us knowledgable about symbolic execution (and clang in particular), but make little sense to end users. Could you add an example to ease reading?

"For instance, if the argument to <fn> must be in between 0 and 255. If the value of the argument is unknown, the analyzer will assume that it is in this interval, even if warnings for this checker are disabled. Similarly, if a function mustn't be called with a null pointer but it is, analysis will stop on that execution path (similarly to a division by zero), with or without a warning."


================
Comment at: clang/docs/analyzer/checkers.rst:2459
 is because exploring an execution path that already contains undefined behavior
-is not valuable.
+is not valuable. This applies to all the restrictions that are listed below.
 
----------------
Restriction is not a bad word, but lets ease into it a bit.

"You can think of this checker as defining restrictions (pre- and postconditions) on standard library functions. Preconditions are checked, and when they are violated, a warning is emitted. Post conditions are added to the analysis, e.g. that the return value must be no greater than 255."


================
Comment at: clang/docs/analyzer/checkers.rst:2490-2523
+**List of checked functions**
+
+``fgetc``, ``fread``, ``fwrite``, ``getc``, ``getchar``, ``getdelim``,
+``getenv``, ``getline``, ``isalnum``, ``isalpha``, ``isascii``, ``isblank``,
+``isdigit``, ``isgraph``, ``islower``, ``isprint``, ``ispunct``, ``isspace``,
+``isupper``, ``isxdigit``, ``read``, ``toascii``, ``tolower``, ``toupper``,
+``write``
----------------
We should create an option or something the //actual// list of functions we model. This is the prime example of unsustainable documenting.


================
Comment at: clang/docs/analyzer/checkers.rst:2542
+modeling (and emit diagnostics) of additional functions that are defined in the
+POSIX standard. This option is disabled by default. Note that this option
+belongs to a separate built-in checker ``apiModeling.StdCLibraryFunctions`` and
----------------
Isn't this something that we either do or do not enable by default? My memory betrays me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149447