[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

This is starting to look great, thanks!



================
Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:96
+
+          Out << '\'' << Name << "' always returns "
+              << (Value ? "true" : "false");
----------------
Let's omit the word "always" here, as we know that there are exceptions from this rule. This may look bad if both `Parser::Error() always returns true` and `Parser::Error() returns false` appear in the same report.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:120
+            << (Value ? "true" : "false")
+            << " according to the LLVM coding standard, but it returns "
+            << (Value ? "false" : "true");
----------------
LLVM coding standard is a fairly specific document: https://llvm.org/docs/CodingStandards.html . It doesn't seem to say anything about parsers.

Let's make this much softer: `Parser::Error() returns false` and that's it.

Also given that this note always applies to inlined calls, let's move this logic to `checkEndFunction()`. I.e., we emit the "false" note in `checkEndFunction` but we emit the "true" note in `checkPostCall`.


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

https://reviews.llvm.org/D63915