[cfe-commits] [PATCH] Allow RefactoringTool to write to memory instead of always to disk

Manuel Klimek klimek at google.com
Thu Jan 10 06:53:02 PST 2013


  How about instead we pull the RefactoringTool apart into 3 methods:
  int runAndSave(FrontendActionFactory); // what run() does now
  int run(FrontendActionFactory); // just run, no apply or save - we can still get the replacements afterwards
  bool applyAllReplacements(Rewriter& Rewrite); // just applies, does not save

  That should give you all the power needed to put stuff into an extra rewriter and do anything you want with the results.


================
Comment at: include/clang/Rewrite/Core/Rewriter.h:61
@@ +60,3 @@
+  /// this function is the responsibility of the caller.
+  const char *getRewriteResult() const;
+
----------------
I think this is unneeded. Instead, we can use a raw_string_ostream and call write() on it.

================
Comment at: include/clang/Tooling/Refactoring.h:125
@@ -120,3 +124,3 @@
 /// processed.
-class RefactoringTool {
+class RefactoringTool : public ClangTool {
 public:
----------------
I'm opposed to this change :) If you want this, you'll need to sell me on the benefit - the default for me is "no inheritance".

================
Comment at: lib/Tooling/Refactoring.cpp:142
@@ +141,3 @@
+// saveRewrittenFiles() and getResults(), without cluttering Refactoring.h's
+// header with includes that are really only needed by the implementation.
+class RewriteHelper {
----------------
I think not putting something into the header is not a good reason to pull out an abstraction. I also generally don't like abstractions called "Helper" (I understand that you don't intend this to be an abstraction, but my point is that if there's not abstraction, I don't think it makes sense to pull one out just to reduce stuff in the header).


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



More information about the cfe-commits mailing list