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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to