[PATCH] clang-apply-replacements: Add code-formatting post processing step

Edwin Vane edwin.vane at intel.com
Thu Sep 26 06:17:23 PDT 2013



================
Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:289
@@ +288,3 @@
+
+void calculateChangedRanges(
+    const std::vector<clang::tooling::Replacement> &Replaces,
----------------
Daniel Jasper wrote:
> I think a lot of the complexity stems from this function accepting non-empty ChangedRanges. I'd prefer to simply impose a restriction until this functionality is actually needed. Or do you already have a use case?
The contents of `claculateChangedRanges()` and `reformatRanges()` are actually copied from clang-modernize. The implementations there are going away soon.

So for non-empty ChangedRanges, this actually //can// be simplified. clang-modernize builds up changed ranges after every transform and reformats once at the very end. But since clang-apply-replacements doesn't have this need I will simplify things to assume empty vector upon input. Thanks for seeing this. When copy-pasting I often forget to revisit the original design requirements.

================
Comment at: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp:117
@@ +116,3 @@
+/// code.
+static void formatReplacements(const Rewriter &AppliedReplacements,
+                               const FileToReplacementsMap &GroupedReplacements,
----------------
Daniel Jasper wrote:
> I think this interface is too complex and has too many assumptions. I think it would get significantly easier if it operated on a single file. Does anything speak against moving the "if (DoFormat)" below into the loop.
> 
> Also, while you can of course try to reuse rewriters ans SourceManagers, I think this makes not only the interface more complex, but also limits the possibilities for executing this in multiple threads. Why not just hand in the applied replacements and a string with the code?
Ok. I'll do replacement application, reformatting, and file writing all in the body of a single loop.


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

BRANCH
  mv-reformat

ARCANIST PROJECT
  clang-tools-extra



More information about the cfe-commits mailing list