[PATCH] D48561: [Analyzer] Moved RangeConstraintManager to header. NFC.

George Karpenkov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 27 20:04:22 PDT 2018


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





More information about the cfe-commits mailing list