george.karpenkov reopened this revision. george.karpenkov added a comment. This revision is now accepted and ready to land.
After thinking about this change a bit longer, I think it does not make sense. Albeit poorly named, the previous design had a purpose: `RangedConstraintManager` is a public interface, and `RangeConstraintManager` is a private implementation. Exposing both in the header does not make sense. For exposing the factory could you just move the factory and it's getter? Another solution is just merging the two classes entirely, but that's more heavyweight, and would force exposing private functions in a header (but those could be just moved to static C functions). @NoQ further comments? ================ Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:120 + LLVM_DUMP_METHOD void dump() const { print(llvm::errs()); } + ---------------- Adding this `dump` method resulted in a bot breakage on our side (during the linking process). I'm not 100% sure why, but since this belongs in a separate review anyway, could we drop those lines? Repository: rC Clang https://reviews.llvm.org/D48561 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits