[PATCH] D143194: [clang][analyzer] Make messages of StdCLibraryFunctionsChecker user-friendly

[Kristóf Umann via Phabricator via cfe-commits]

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