ASDenysPetrov added inline comments.
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:293-296
+ SymbolRef Sym = Operand;
+ while (isa<SymbolCast>(Sym))
+ Sym = cast<SymbolCast>(Sym)->Operand;
+ return Sym;
----------------
vsavchenko wrote:
> ASDenysPetrov wrote:
> > vsavchenko wrote:
> > > ASDenysPetrov wrote:
> > > > vsavchenko wrote:
> > > > >
> > > > Do you think the recursive call is better than the loop? But, I guess,
> > > > I see your point. You option could be safer if we had another
> > > > implementation of the virtual method. Or you think such alike cast
> > > > symbol is possible in the future? Well, for now `ignoreCasts` doesn't
> > > > make sense to any other `Expr` successors.
> > > Oh, wait, why is it even virtual? I don't think that it should be
> > > virtual.
> > > Are similar functions in `Expr` virtual?
> > > And I think that this implementation should live in `SymExpr` directly.
> > > Then it would look like:
> > > ```
> > > if (const SymbolCast *ThisAsCast = dyn_cast<SymbolCast>(this)) {
> > > return ThisAsCast->ignoreCasts();
> > > }
> > > return this;
> > > ```
> > Yes, `SymExpr` is an abstract class. And because of limitations and
> > dependency of inheritance we are not able to know the implementaion of
> > `SymbolCast`. Unfortunately, this is not a CRTP.
> Re-read my comment, please.
> Oh, wait, why is it even virtual?
`ignoreCasts` is a virtual function because I haven't found any other way to
implement it better.
> I don't think that it should be virtual.
Unfortunately, this is not a CRTP to avoid dynamic linking.
> Are similar functions in Expr virtual?
`SymExpr` is an abstract class. I'm not sure about similarity but `SymExpr` has
such virtual methods:
- computeComplexity
- getType
- getOriginRegion
> And I think that this implementation should live in SymExpr directly.
It's impossible due to `SymExpr` implementation design. `SymExpr` knows nothing
about implementation details of `SymbolCast` to invoke `ignoreCasts()`.
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
