martong planned changes to this revision.
martong marked 4 inline comments as done.
martong 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; }));
}
----------------
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.
================
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}} \
----------------
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.
================
Comment at: clang/test/Analysis/std-c-library-functions-arg-constraints.c:42
int ret = isalnum(x);
+ // bugpath-note@-1{{Applied constraint: The 1st arg should be within the
range [[-1, -1], [0, 255]]}}
(void)ret;
----------------
NoQ wrote:
> This isn't part of this patch but what do you think about `{-1} U [0, 255]`?
> Or, you know, `[-1, 255]`.
Yeah, good idea, `{-1} U [0, 255]` would be indeed nicer and shorter. However,
`[-1, 255]` is hard(er) so I am going to start with the former in a follow up
patch.
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
