[PATCH] D122285: [analyzer] Add path note tags to standard library function summaries.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:879
+            // knowing in advance which branch is taken.
+            return (Node->succ_size() > 1) ? Note.str() : "";
+          },
----------------
NoQ wrote:
> balazske wrote:
> > balazske wrote:
> > > Can other checkers not add successor nodes (that make this check not always correct)?
> > Is there a reason against add the note without the word "Assuming" instead of no note?
> > Can other checkers not add successor nodes (that make this check not always correct)?
> 
> They'll be chained to the newly created node, not to the same predecessor. Two checkers adding transitions on the same post-call won't (and shouldn't) suddenly cause a state split.
> 
> > Is there a reason against add the note without the word "Assuming" instead of no note?
> 
> We generally never have event notes in such cases.
> 
> We sometimes have control flow notes such as "`aking true branch`" (arrow pointing to the branch in plist).
> 
> Then, separately from that, we have tracking notes such as "an interesting value appeared in this variable from this assignment, which also makes the assigned value interesting" (eg. "`null pointer value assigned to 'p'`") - we could have a similar note "an interesting value was returned from that function because that other value had a certain range, which also makes that other value interesting" - which we should definitely consider.
> 
> But simply saying "this function call was completely predictable and we don't even care what the value is in the first place" doesn't sound useful.
*`Taking`


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

https://reviews.llvm.org/D122285