vsavchenko added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1203 + if (!Opts.ShouldSupportSymbolicIntegerCasts) + return VisitSymExpr(Sym); + ---------------- ASDenysPetrov wrote: > vsavchenko wrote: > > ASDenysPetrov wrote: > > > vsavchenko wrote: > > > > Why do you use `VisitSymExpr` here? You want to interrupt all `Visits > > > > or... I'm not sure I fully understand. > > > Here we want to delegate the reasoning to another handler as we don't > > > support non-integral cast yet. > > You are not delegating it here. `Visit` includes a runtime dispatch that > > calls a correct `VisitTYPE` method. Here you call `VisitSymExpr` directly, > > which is one of the `VisitTYPE` methods. No dispatch will happen, and > > since we use `VisitSymExpr` as the last resort (it's the most base class, > > if we got there, we don't actually support the expression), you interrupt > > the `Visit` and go directly to "the last resort". > > > > See the problem now? > OK. I reject this idea before. If we call `Visit` inside `VisitSymbolCast`, > we will go into recursive loop, because it will return us back to > `VisitSymbolCast` as we have passed `Sym` as is. (This is theoretically, I > didn't check in practice.) Or I'm missing smth? > I choosed `VisitSymExpr` here because all kinds of `SymbolCast` were > previously handled here. So I decided to pass all unsupproted forms of casts > there. Did I suggest to `Visit(Sym)`? Of course it is going to end up in a loop! Why isn't it `Visit(Sym->getOperand())` here? Before we started producing casts, casts were transparent. This logic would fit perfectly with that. ================ Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1216 + if (!T->isIntegralOrEnumerationType()) + return VisitSymExpr(Sym); + ---------------- vsavchenko wrote: > Same goes here. And here, since we couldn't really reason about it, we usually return `infer(T)`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103096/new/ https://reviews.llvm.org/D103096 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
