martong marked 3 inline comments as done.
martong added a comment.
Thanks for the review Valeriy!
================
Comment at:
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:338-339
}
+ llvm_unreachable("The constraint must be either a concrete value or "
+ "encoded in an argument.");
}();
----------------
vsavchenko wrote:
> Just a thought here, maybe we should assert `SizeArgN` instead then?
Absolutely, good point.
================
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:
> 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.
================
Comment at: clang/test/Analysis/std-c-library-functions-arg-constraints.c:33
// report-warning{{Function argument constraint is not satisfied}} \
+ // report-note{{}} \
// bugpath-warning{{Function argument constraint is not satisfied}} \
----------------
vsavchenko wrote:
> What's up with these?
The new explanation of the constraints is added as an extra 'note' tag, which
is displayed alongside the warning.
In these tests I don't want to test for the note, that would make these tests
overly specified. For testing the notes we have a separate, newly added test
file `std-c-library-functions-arg-constraints-notes.cpp`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101060/new/
https://reviews.llvm.org/D101060
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
