Szelethus added a comment. A small nit, otherwise LGTM.
In D143194#4112538 <https://reviews.llvm.org/D143194#4112538>, @balazske wrote: > Probably it is not always useful to explain why the argument is wrong. In > cases when we can find out that the value is exactly between two consecutive > valid ranges we can display a note, or when the exact value is known. > Otherwise it may end up in something like "the value should be between 1 and > 4 or between 6 and 10, but it is less than 1 or exactly 5 or greater than 10". You're totally right. I'm not sure how to approach tricky cases like that either. > The "good" cases are (when more information is available): "the value is less > than 1", "the value is 5", "the value is greater than 10". If two bad ranges > are known it may be OK too: "the value is less than 1 or it is exactly 5". > The explanation may be possible to implement by test assumes for the negated > ranges. Maybe we can start out with the easier cases, but we should check whether there is a checker that solves a different problem, and should try to aim at a somewhat general, reusable solution. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:95 + QualType ArgT, BasicValueFactory &BVF, + DescString &Out); + /// Append textual description of a numeric range out of [RMin,RMax] to the ---------------- Using a `raw_ostream` as a parameter sounds more elegant than a `SmallString` with a precise stack buffer length. Not to mention that you could call this function with `llvm::errs()` for easy debugging. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:100 + QualType ArgT, BasicValueFactory &BVF, + DescString &Out); ---------------- Here as well. ================ Comment at: clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp:51-79 +int __single_val_0(int); // [0, 0] int __single_val_1(int); // [1, 1] int __range_1_2(int); // [1, 2] -int __range_1_2__4_5(int); // [1, 2], [4, 5] -void test_range(int x) { - __single_val_1(2); // \ - // expected-note{{The 1st argument should be within the range [1, 1]}} \ - // expected-warning{{}} -} -// Do more specific check against the range strings. +int __range_m1_1(int); // [-1, 1] +int __range_m2_m1(int); // [-2, -1] +int __range_m10_10(int); // [-10, 10] +int __range_m1_inf(int); // [-1, inf] ---------------- Please clang-format the changes in this test file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143194/new/ https://reviews.llvm.org/D143194 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
