[PATCH] D18551: Added Fixer implementation and fix() interface in clang-format for removing redundant code.

[Manuel Klimek via cfe-commits]

klimek added inline comments.

================
Comment at: include/clang/Format/Format.h:769
@@ -768,1 +768,3 @@
 
+/// \brief Returns the replacements corresponding to applying \p Replaces and
+/// cleaning up the code after that.
----------------
cleanupAroundReplacements sounds good.

================
Comment at: lib/Format/Format.cpp:1466
@@ -1464,1 +1465,3 @@
+  template <typename T>
+  tooling::Replacements process(T Callable, bool *IncompleteFormat) {
     tooling::Replacements Result;
----------------
Nit: I think it's idiomatic to call this a "Callback".

================
Comment at: lib/Format/Format.cpp:2143
@@ +2142,3 @@
+
+  CodeProcessor Processor(Expanded, SourceMgr, ID, Ranges);
+  auto Callable =
----------------
This is unexpected: I'd not have expected the CodeProcessor to be used to keep state apart from what's used during a Process() run. I think we'll need a different name.

================
Comment at: lib/Format/Format.cpp:2176
@@ +2175,3 @@
+                   bool *IncompleteFormat) -> tooling::Replacements {
+    Cleaner Cleaner(Processor);
+    return Cleaner.runCleanup(Tokens, AnnotatedLines);
----------------
The way the Processor is passed into the callbacks so they can call stuff on the processor again makes me think this is really tightly coupled and should use inheritance.

Alternatively, we might want to try to break up the CodeProcessor into a part that just runs the callback and keeps the state for running the callback around, and an interface that is used by the callbacks - I'll need to look more closely into this though.


http://reviews.llvm.org/D18551