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

Reply via email to