Szelethus requested changes to this revision. Szelethus added inline comments. This revision now requires changes to proceed.
================ 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(); ---------------- 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. ================ Comment at: lib/StaticAnalyzer/Core/LoopUnrolling.cpp:221-230 + if (BoundNumVal.isUnknown()) { + if (const auto *BoundDeclRefExpr = dyn_cast<DeclRefExpr>(BoundExpr)) { + // FIXME: Add other declarations such as Objective-C fields + if (const auto *BoundVarDecl = + dyn_cast<VarDecl>(BoundDeclRefExpr->getDecl())) { + BoundNumVal = State->getSVal( + State->getLValue(BoundVarDecl, Pred->getLocationContext())); ---------------- NoQ wrote: > Szelethus wrote: > > I don't see obvious test case for which `BoundNumVal` would be unknown, am > > I wrong? > We need an `ExprInspection` method that reliably produces an `UnknownVal`, > because there's no truly valid reason to produce `UnknownVal` apart from > "something is unimplemented". For this too. Repository: rC Clang 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