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

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

Szelethus added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:288-290
+  SymbolRef Sym = RetVal.getAsSymbol();
+  stateNotNull = stateNotNull->set<StreamMap>(Sym, StreamState::getOpened());
+  stateNull = stateNull->set<StreamMap>(Sym, StreamState::getOpenFailed());
----------------
Aha, so we're no longer checking whether `Sym` is null, because why would we, we literally made created it as a symbol a couple lines up. Is it worth `assert()`ing though?


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:296-297
 
-ProgramStateRef StreamChecker::CheckNullStream(SVal SV, ProgramStateRef state,
-                                    CheckerContext &C) const {
+Optional<ProgramStateRef>
+StreamChecker::CheckNullStream(SVal SV, CheckerContext &C) const {
   Optional<DefinedSVal> DV = SV.getAs<DefinedSVal>();
----------------
Why is there a need to use an `Optional`? Why not just return a `nullptr`? As I saw it, each time we check both whether the optional has a value //and// whether the state within it is null.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:319-320
 
-ProgramStateRef StreamChecker::CheckDoubleClose(const CallExpr *CE,
-                                               ProgramStateRef state,
-                                               CheckerContext &C) const {
-  SymbolRef Sym = C.getSVal(CE->getArg(0)).getAsSymbol();
+Optional<ProgramStateRef>
+StreamChecker::CheckDoubleClose(const CallEvent &Call,
+                                CheckerContext &C) const {
----------------
And here too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67706