xazax.hun added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp:84
+ if (!BlockToOutputState ||
+ BlockToOutputState->size() <= Context->getCFG().getExit().getBlockID())
+ return;
----------------
xazax.hun wrote:
> ymandel wrote:
> > xazax.hun wrote:
> > > xazax.hun wrote:
> > > > Could the size of the vector ever be wrong? Should this be an assert
> > > > instead?
> > > Whoops, after the update this comment is out of place, now it supposed to
> > > be on line 60.
> > Based on my reading, it is a rare, but possible condition. Basically, we
> > need code where the exit block is unreachable, which I believe can happen
> > in weird cases like:
> >
> > ```
> > while(true) {...}
> > ```
> > https://godbolt.org/z/rfEnfaWTv -- notice the lack of predecessors for the
> > exit block.
> >
> > See the code here, which follows the ordering of the blocks and doesn't
> > force blocks to be processed:
> >
> > https://github.com/llvm/llvm-project/blob/main/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp#L337-L364
> Interesting. Since we already have optionals in the vector, I assumed we will
> always have matching size. I think we might want to change this so there is
> only one way for the analysis to not provide a state for a basic block to
> make this a bit less confusing,
Actually, in the linked code I see ` BlockStates.resize(CFCtx.getCFG().size(),
llvm::None);`. So I would expect the size to be always right with possibly some
`None`s for the nodes that were not processed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121120/new/
https://reviews.llvm.org/D121120
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
