steakhal added a comment. Great work!
================ Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1591-1595 template <class ClassOrSymbol> LLVM_NODISCARD static ProgramStateRef - assign(ProgramStateRef State, SValBuilder &Builder, RangeSet::Factory &F, - ClassOrSymbol CoS, RangeSet NewConstraint) { + assign(ProgramStateRef State, RangeConstraintManager *RCM, + SValBuilder &Builder, RangeSet::Factory &F, ClassOrSymbol CoS, + RangeSet NewConstraint) { ---------------- Hm, why don't we acquire `RCM`, `Builder`, `F` in the constructor? I'm expecting all of them to remain the same for all the `assign()` calls. ================ Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1603-1604 + template <typename SymT> + bool handleRem(const SymT *Sym, RangeSet Constraint) { + // a % b != 0 implies that a != 0. ---------------- Why is this not a `const` member function? ================ Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1605 + bool handleRem(const SymT *Sym, RangeSet Constraint) { + // a % b != 0 implies that a != 0. + if (Sym->getOpcode() != BO_Rem) ---------------- I think you should also implement `a % b == 0 implies that a == 0`. ================ Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1628 private: - ConstraintAssignor(ProgramStateRef State, SValBuilder &Builder, - RangeSet::Factory &F) - : State(State), Builder(Builder), RangeFactory(F) {} + ConstraintAssignor(ProgramStateRef State, RangeConstraintManager *RCM, + SValBuilder &Builder, RangeSet::Factory &F) ---------------- IMO we should pass a reference here, just like the rest of the parameters. ================ Comment at: clang/test/Analysis/constraint-assignor.c:18 + clang_analyzer_warnIfReached(); // no-warning + (void)x; // keep the constraints alive. +} ---------------- It's still mindboggling that we need to do this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110357/new/ https://reviews.llvm.org/D110357 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits