[PATCH] D101060: [Analyzer][StdLibraryFunctionsChecker] Describe arg constraints

Fri Apr 23 06:19:13 PDT 2021

martong marked an inline comment as done.
martong added inline comments.

================
Comment at: clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp:32
+    __buf_size_arg_constraint_concrete(buf); // \
+    // expected-note{{The size of the 0th arg should be equal to or less than the value of 10}} \
+    // expected-warning{{}}
----------------
vsavchenko wrote:
> martong wrote:
> > vsavchenko wrote:
> > > Oof, I do understand that we are devs and enumerate things starting from 0. But this is supposed to be human-readable and humans start counting from 1.
> > I've been thinking a lot about this and I see your point. On the other hand, we report warnings to other developers/programmers who are all used to start the indexing from 0, they may find it odd to start from 1. 
> > 
> > Alas, the string `0th` makes it obvious that we are talking about the first argument, however the string `1st` is ambiguous, even if we start the indexing from 0 or from 1. In this sense, starting from 0 makes less confusion.
> I know that we are talking to developers, but no developers say that this is a 0th argument. And IMO the vast majority of developers would think of the argument at index 0 when they read '1st' because most of people are not compiler engineers and don't think of the list of arguments as an array. 
> But that is all opinions after all. What is most important is that clang already reports a ton of warnings pointing to a specific argument/parameter by its ordinal number.  Simply grep `DiagnosticsSemaKinds.td` for `ordinal` and see the examples in tests. As you can guess, they all use ordinals starting from **1st**.
Fair enough. It would be inconsistent to start from 0 if the Sema warnings already start from 1. I've changed it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101060