martong marked 2 inline comments as done.
martong added inline comments.

================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2104
+      const llvm::APSInt &SV = CI->getValue();
+      const RangeSet *ClassConstraint = getConstraint(State, Class);
+      // We have found a contradiction.
----------------
ASDenysPetrov wrote:
> We usually use `R` or `RS`. It gets readability better IMO.
There are many other cases in this file where we spell out ClassConstraint, 
e.g. in `mergeImpl` we have `NewClassConstraint`. Also, I think the name 
`ClassConstraint` is telling way more than `RS`, so I'd like to keep this name.


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2107-2109
+        return nullptr;
+    } else {
+      SimplifiedMemberSym = SimplifiedMemberVal.getAsSymbol();
----------------
ASDenysPetrov wrote:
> What prevents you to not put this stuff inside `ento::simplify`. I think it 
> should perfectly fit in there.
`ento::simplify` is at a different abstraction level, it works on a `SymbolRef` 
and the `SVal` associated to it; and it uses the `SValBuilder` to achieve the 
desired constant folding. We have absolutely no knowledge about the equivalence 
classes there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110913

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to