[PATCH] D153017: [analyzer] Fix false negative when using a nullable parameter directly without binding to a variable

[Balázs Benics via Phabricator via cfe-commits]

steakhal added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:569-594
+void NullabilityChecker::checkBeginFunction(CheckerContext &C) const {
+  const LocationContext *LCtx = C.getLocationContext();
+  auto AbstractCall = AnyCall::forDecl(LCtx->getDecl());
+  if (!AbstractCall)
+    return;
+
+  ProgramStateRef State = C.getState();
----------------
steakhal wrote:
> steakhal wrote:
> > Uh, the diffing here looks terrible.
> > What you probably want: Fold the `State`s, and if you are done, transition - but only if we have any parameters.
> > We need to have a single `addTransition()` call if we want a single execution path modeled in the graph. We probably don't want one path on which the first parameter's annotation is known; and a separate one where only the second, etc.
> Shouldn't we only do this for the analysis entrypoints only? (aka. top-level functions)
> I assume this checker already did some modeling of the attributes, hence we have the warnings in the tests.
Please note that each iteration of the loop will overwrite the state you made in the previous iteration this way.
Its most likely not what you want.
Because C.getState() will always return the same state no matter the context here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153017