[PATCH] D67706: [clang][analyzer] Using CallDescription in StreamChecker.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:90-106
+  FnCheck identifyCall(const CallEvent &Call, CheckerContext &C,
+                       const CallExpr *CE) const;
+
+  void evalFopen(CheckerContext &C, const CallExpr *CE) const;
+  void evalTmpfile(CheckerContext &C, const CallExpr *CE) const;
+  void evalFclose(CheckerContext &C, const CallExpr *CE) const;
+  void evalFread(CheckerContext &C, const CallExpr *CE) const;
----------------
balazske wrote:
> NoQ wrote:
> > For most purposes it's more convenient to pass `CallEvent` around. The only moment when you actually need the `CallExpr` is when you're doing the binding for the return value, which happens once, so it's fine to call `.getOriginExpr()` directly at this point.
> The CallExpr is used at many places to get arguments and other data. There is a `CallEvent::getArgSVal` that can be used instead but at some places CallExpr is used in other ways. I do not see much benefit of change the passed `CallExpr` to `CallEvent`.
> at some places CallExpr is used in other ways

Can we add more convenient getters to `CallEvent` to help with those? 'Cause `CallEvent` has strictly more information than `CallExpr` (combining syntactic information with path-sensitive information), it's all about convenience.

I also see that `CallExpr` is stored in `StreamState` (which is something you can't do with a `CallEvent` because it's short-lived) but i suspect that it's actually never read from there and i doubt that it was the right solution anyway. I.e., no other checker needs that and i don't have a reason to believe that `StreamChecker` is special.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67706