[PATCH] D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:836-837
+          NewState, NewNode,
+          C.getNoteTag([Msg](PathSensitiveBugReport &BR,
+                             llvm::raw_ostream &OS) { OS << Msg; }));
     }
----------------
martong wrote:
> steakhal wrote:
> > This way each and every applied constraint will be displayed even if the given argument does not constitute to the bug condition.
> > I recommend you branching within the lambda, on the interestingness of the given argument constraint.
> Okay, good point, thanks for the feedback! I am planning to change to this direction.
Excellent catch @steakhal!

I think you can always emit the note but only mark it as //unprunable// when the argument is interesting. This way it'd work identically to our normal "Assuming..." notes.


================
Comment at: clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp:16
+int test_note(int x, int y) {
+    __single_val_1(x);  // expected-note{{Applied constraint: The 1st arg should be within the range [1, 1]}}
+    return y / (1 - x); // expected-warning{{Division by zero}} \
----------------
martong wrote:
> NoQ wrote:
> > This has to be a user-friendly message.
> > * "Constraints" is compiler jargon.
> > * We cannot afford shortening "argument" to "arg".
> > * Generally, the less machine-generated it looks the better (":" is definitely robotic).
> Okay, thanks for your comment. I can make it to be more similar to the other notes we already have. What about this?
> ```
> Assuming the 1st argument is within the range [1, 1]
> ```
> 
> > We cannot afford shortening "argument" to "arg".
> I'd like to address this in another following patch if you don't mind.
This sounds good for a generic message. I still think that most of the time these messages should be part of the summary. Eg., 
```
Assuming the 1st argument is within range [33, 47] U [58, 64] U [91, 96] U [123, 125]
```
ideally should be rephrased as
```
Assuming the argument is a punctuation character
```
in the summary of `ispunct()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101526