[PATCH] Modularize assistant mode

Sean Silva silvas at purdue.edu
Mon Oct 14 08:48:11 PDT 2013


  LGTM with a couple tiny changes. Doug, could you take a look at this?

  Btw, does this need user-facing documentation yet? (that can be a separate commit if so)


================
Comment at: modularize/ModuleAssistant.cpp:272
@@ +271,3 @@
+
+} // end anonymous namespace.
+
----------------
The rationale in <http://llvm.org/docs/CodingStandards.html#anonymous-namespaces> is basically to improve locality of reference. This anonymous namespace should surround just the declaration of class Module, and the free functions that currently are in the anonymous namespace (but will not be after that change) should be marked static.

================
Comment at: modularize/ModuleAssistant.cpp:266-267
@@ +265,4 @@
+
+  // The tool_output_file destructor writes the file from its buffer
+  // and closes the stream.
+
----------------
I don't think you need to explicitly say this.


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



More information about the cfe-commits mailing list