tripleCC added inline comments.
================
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:
> 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.
Thanking for your reviewing. You are correct, I added an `inTopFrame()`
condition here. It only makes sense for top-level functions.
================
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();
----------------
tripleCC wrote:
> 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.
> Thanking for your reviewing. You are correct, I added an `inTopFrame()`
> condition here. It only makes sense for top-level functions.
I made a rookie mistake here. I should call the `addTransition` function
outside the for loop.
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
