[PATCH] D103317: [Analyzer][engine][solver] Simplify complex constraints
Gabor Marton via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 12 04:04:17 PST 2021
martong added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1155-1156
- // TODO: Support SymbolCast. Support IntSymExpr when/if we actually
- // start producing them.
----------------
ASDenysPetrov wrote:
> ASDenysPetrov wrote:
> > vsavchenko wrote:
> > > ASDenysPetrov wrote:
> > > > vsavchenko wrote:
> > > > > Do we actually produce them?
> > > > You can met SymbolCast here if `evalCast` produced it.
> > > > See `SValBuilder::evalCastSubKind(nonloc::SymbolVal...)`:
> > > > ```
> > > > if (!Loc::isLocType(CastTy))
> > > > if (!IsUnknownOriginalType || !CastTy->isFloatingType() ||
> > > > T->isFloatingType())
> > > > return makeNonLoc(SE, T, CastTy);
> > > > ```
> > > > Also my patch will produce a lot of integral cast symbols D103096. You are welcome to examine it.
> > > In my comment I meant `IntSymExpr`
> > Oh, anyway :-)
> I think we should remove **ConcreteInt** and and as a consequence **IntSymExpr**, **SymIntExpr** in the nearest future. SymbolVal should be reworked to store a QualType, a RangeSet and an expression pointer itself. IMO SymSymExpr and SymbolVal is sufficient to make any calculations. It may help generalize a lot of them. **ConcreteInt** is just a special case for SymbolVal. More than that **PointerToMember** can be removed as well, because we would get an info being a pointer to member from QualType in this way.
> Do we actually produce them?
Yes we produce `IntSymExpr`s.
================
Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1170-1172
+ SVal LHS = getConst(S->getLHS());
+ if (LHS.isUndef())
+ LHS = Visit(S->getLHS());
----------------
vsavchenko wrote:
> It seems like a very common pattern now:
> ```
> SVal Const = getConst(Expr);
> if (Const.isUndef())
> return Visit(Expr);
> return Const;
> ```
>
> At least we can make a function out of this, so we don't repeat this pattern over and over.
> But I was thinking more radically to actually replace `Visit` with this, what do you think?
Good idea!
================
Comment at: clang/test/Analysis/simplify-complex-constraints.cpp:25-27
+ if (x + (y + z) != 0)
+ return 0;
+ if (y + z != 0)
----------------
vsavchenko wrote:
> Since it's not really specific to addition, maybe we can have tests for other operators?
Ok, I've added a new test, and modified a bit this test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103317/new/
https://reviews.llvm.org/D103317
More information about the cfe-commits
mailing list