ASDenysPetrov added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1203
+ if (!Opts.ShouldSupportSymbolicIntegerCasts)
+ return VisitSymExpr(Sym);
+
----------------
vsavchenko wrote:
> 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.
> were transparent.
Not exactly. There are still some cases when symbols are not equal to there
roots(aka Operands). Such cases were handled by `VisitSymExpr` which uses
`infer(Sym->getType());` instead of getOperand`. So this needs a sort of think
twice. Also I see a problem with `EquivalenceClass`'es. Consider next:
```
int x, y;
if(x == y)
if ((char)x == 2)
if(y == 259)
// Here we shall update `(char)x` and find this branch infeasible.
```
Also such cases like:
```
if(x == (short)y)
// What we should do(store) with(in) `EquivalenceClass`es.
```
Currently, I have an obscure vision of the solution.
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
