[cfe-commits] [PATCH] Foundation for Transform registration infrastructure

[Manuel Klimek]

  I'm not sure such a heavy-weight design is needed. I haven't looked at llvm's Pass in detail, but I assume it solves a more generic problem? I especially don't see the need for a tag interface (Transform) which is basically never used.

  I think we can use a much more "Transformation"-centric design here.

  I'd vote for adding a few transformations and then extracting what we see is common, instead of coming up with a design up front where I'm not sure how it might or might not fit our use cases...

  I'm all for adding structure early and stealing good design from other places, but in this specific case my prediction is we'll end up with something much simpler.


================
Comment at: cpp11-migrate/TransformRegistry.h:21
@@ +20,3 @@
+class TransformRegistry {
+  TransformRegistryImpl *Impl;
+
----------------
Adding my usual rant that I strongly prefer the implementation details to be at the bottom of the class, instead of at the top, where it's the first thing a user of the class sees.

================
Comment at: cpp11-migrate/TransformRegistry.h:31
@@ +30,3 @@
+  /// Register a Transform with the registry.
+  void registerTransform(const TransformInfo &TI);
+
----------------
Adding another rant (feel free to ignore) about TLAs. I'd name this Info, and the parameters in the other methods Listener.

================
Comment at: cpp11-migrate/TransformRegistry.cpp:23
@@ +22,3 @@
+
+class TransformRegistryImpl {
+public:
----------------
Why is there a need for this Impl class instead of just instantiating the TransformRegistry?


http://llvm-reviews.chandlerc.com/D221