martong marked 2 inline comments as done.
martong added inline comments.

================
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:
> 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()`.
Yes, absolutely, good idea. It makes sense to provide another member for the 
`Summary` that could specifically describe the function specific assumptions 
(or violations). However, before we would be able to go through all functions 
manually to create these specific messages we need a generic solution to have 
something that is more descriptive than the current solution.


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

Reply via email to