steakhal added a comment.

I believe, `zaks.anna` and `vsavchenko` are no longer involved in the project.
I think it makes sense to have the code owners NoQ and xazax.hun as reviewers, 
and I also tend to review quite a lot nowadays.
And we usually use the `[analyzer]` tag instead of `[StaticAnalyzer]` for the 
patches.
It's useful to use the right tags to trigger the right herald scripts to get 
the right circle notified.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:569-573
+void NullabilityChecker::checkBeginFunction(CheckerContext &C) const {
+  const LocationContext *LCtx = C.getLocationContext();
+  auto AbstractCall = AnyCall::forDecl(LCtx->getDecl());
+  if (!AbstractCall)
+    return;
----------------
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.


================
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();
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153017

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

Reply via email to