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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
