martong added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2314-2315
+ if (SymbolRef SimplifiedSym = simplify(St, Sym))
+ Sym = SimplifiedSym;
+
----------------
vsavchenko wrote:
> I don't like the idea of duplicating it into every `assume` method. This way
> we drastically increase our chances to forget it (like you did with
> `assumeSymGE` and `assumeSymLE`).
> I think the better place for it is in `RangedConstraintManager::assumeSymRel`
> and neighboring methods, though still not perfect.
> I don't really get why we get not simplified symbol to begin with.
>
`assumeSymRel` is not enough, because e.g. `assumeSymGE` is called also e.g.
from `assumeSymUnsupported`. Perhaps we could change the signature of
`assumeSymEQ/NE/GT/GE/LT/LE` to take an auxiliary `Simplifier` wrapper object
instead of `SymbolRef`?
```
ProgramStateRef assumeSymNE(ProgramStateRef State, Simplifier S,
const llvm::APSInt &V,
const llvm::APSInt &Adjustment);
```
And for the Simplifier something like:
```
struct Simplifier {
SymbolRef SimplifiedSym = nullptr;
Simplifier(SymbolRef Sym) : SimplifiedSym(simplify(Sym)) {}
};
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104844/new/
https://reviews.llvm.org/D104844
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
