[PATCH] D90691: [analyzer] Add new checker for unchecked return value.

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

Szelethus added reviewers: NoQ, vsavchenko.
Szelethus added a comment.

Looks like a neat checker! I guess the only question is the function matching, and I don't dislike it in its current state. @martong, do you have any thoughts on this?

On another note, have you checked this on some projects yet? I expect that we might need to tone down on some of the functions if this ends up being too noisy.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/UncheckedReturnValueChecker.cpp:66-67
+        continue;
+      if (!FD->isInStdNamespace() && !isInNoNamespace(FD))
+        continue;
+
----------------
I suppose we need this to compensate for the fact that we can't reliably match on standard C functions.  Unfortunately, using `CallDescription` wouldn't necessarily solve this: D81745.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/UncheckedReturnValueChecker.cpp:85
+private:
+  llvm::StringMap<int> FunctionsToCheck = {
+      {"aligned_alloc", 2}, {"asctime_s", 3},     {"at_quick_exit", 1},
----------------
balazske wrote:
> baloghadamsoftware wrote:
> > Hmm, why `StringMap<>`? Why not `CallDescriptionMap<>`?
> `CallDescriptionMap` is only usable with `CallEvent` that is not used in this checker. Or it can be extended with lookup from `FunctionDecl`?
Good point. @martong, you have used `FunctionDecl` based checking, do you have any thoughts here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90691