Szelethus added a comment.
I think you forgot to remove `/* */` and clang formatting before uploading the
patch.
================
Comment at: lib/StaticAnalyzer/Core/LoopUnrolling.cpp:214-216
+ const Expr *BoundExpr = CondOp->getLHS()->IgnoreParenImpCasts();
+ if (BoundExpr == Matches[0].getNodeAs<Expr>("boundVarOperand"))
+ BoundExpr = CondOp->getRHS()->IgnoreParenImpCasts();
----------------
Szelethus wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > Alright, I stood on top of this for longer than I'd like to admit, what's
> > > happening here? Like, `CondExpr`, with the given testfile, should be `i <
> > > n`, right? Then `BoundExpr` is supposed to be `i`, and if that is equal
> > > to `Matches[0].getNodeAs<Expr>("boundVarOperand")` (which I guess is
> > > supposed to `n`), which I'm not sure how it could ever happen, we assign
> > > the RHS of the assignment to `BoundExpr`?
> > >
> > > What does this do? Why is it needed? Which 2 test cases cover these lines?
> > I think this is about `i < n` vs. `n > i`.
> If that is the case, I'd be more comfortable seeing a test for it (there
> doesn't seem to be any) before landing this.
I'd appreciate a few lines of comment, given that there's evidence that it
might not be obvious what's happening here :^)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63279/new/
https://reviews.llvm.org/D63279
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
