[PATCH] D114887: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

I think I'm in favor of this change. However, Im also slightly in favor of reverting stuff instead of rushing things. Let's hope it will resolve the issue of the previous offending commit.

Accepted.



================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2220
+                                  const SymbolRef Sym) {
+  SVal Val = SVB.makeSymbolVal(Sym);
+  SVal SimplifiedVal = SVB.simplifySVal(State, Val);
----------------
Couldnt we just create an SVal from a SymExpr? Why do we need the SVB foe this?


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2222
+  SVal SimplifiedVal = SVB.simplifySVal(State, Val);
+  // Do the simplification until we can.
+  while (SimplifiedVal != Val) {
----------------
Nit: It doesnt seem to be grammatically correct.


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2218
+LLVM_NODISCARD
+static SVal simplifyUntilFixpoint(SValBuilder &SVB, ProgramStateRef State,
+                                  const SymbolRef Sym) {
----------------
martong wrote:
> Perhaps this should be done in `SimpleSValbuilder::simplifySVal`, I think this should be the responsibility of the SValBuilder compoment.
My thought was the same.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114887/new/

https://reviews.llvm.org/D114887