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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
