[PATCH] D105692: [analyzer][solver][NFC] Introduce ConstraintAssignor

Gabor Marton via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 9 08:20:54 PDT 2021


martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.

Generally it looks okay to me. However, I have a question:
In `SimpleSValBuilder::evalBinOpNN` we do infer values to symbols. E.g. if we have an expression `y / x` and `y` is known to be `0` then we assign `0` to the division expression. On one hand, this logic is independent from whichever solver implementation we use. On the other hand, we do the value inferring based on the fact that `y` is a constrainted to be in `[0, 0]` so in this sense this infer logic should be in the solver.
What do you think, perhaps another round of refactoring should move that logic into the solver?



================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1388
+//===----------------------------------------------------------------------===//
+//                            New constraint handler
+//===----------------------------------------------------------------------===//
----------------
`New` now, but it might be old after a while.


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1394
+///
+/// It is designed to have one derived class, but generally it cna have more.
+/// Derived class can control which types we handle by defining methods of the
----------------
typo, `can`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105692



More information about the cfe-commits mailing list