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;
----------------
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, 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121120

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

Reply via email to