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

[Daniel Jasper via cfe-commits]

djasper added inline comments.

================
Comment at: lib/Format/Format.cpp:1654
@@ +1653,3 @@
+    UnwrappedLines.push_back(SmallVector<UnwrappedLine, 16>());
+  }
+
----------------
That I don't understand. Almost all fixers (cleaning up commas, constructor initializers, etc.) will only ever look at a single line.

The matchers that do make multiline fixes (empty namespaces, empty public/private sections) only look at complete lines, i.e. only ever use AnnotatedLine::startsWith(...) and don't iterate over the tokens, AFAICT.

I really think that giving the option to iterate over all tokens of multiple lines does more harm than good. Among several other things, error recovery is made harder. It never makes sense for the ctor-initializer fixer to leave its line (or else you'll just ignore some nice error recovery already implemented in the UnwrappedLineParser). 

================
Comment at: lib/Format/Format.cpp:1693
@@ +1692,3 @@
+    // FIXME: we need better way to determine wether to delete this token.
+    if (!AnnotatedLines[CurrentLine]->Affected && !Forced) return;
+    Tok->Deleted = true;
----------------
I have a comment here (and in some other places), but I think this will be different anyway if we move to line-based fixers. So I am holding off with those comments for now.

================
Comment at: lib/Format/Format.cpp:1718
@@ +1717,3 @@
+  void skipParens() {
+    assert(CurrentToken->is(tok::l_paren));
+    nextToken();
----------------
Just go to CurrentToken->MatchingParen!?

================
Comment at: lib/Format/Format.cpp:1824
@@ +1823,3 @@
+  // Returns true if the namespace is empty .
+  bool checkEmptyNamespace() {
+    assert(CurrentToken->is(tok::kw_namespace));
----------------
I am happy to show you how the implementation here gets much simpler if you only iterate over AnnotatedLines.

================
Comment at: lib/Format/Format.cpp:1916
@@ +1915,3 @@
+  FormatToken *CurrentToken;
+  // Redundant tokens to be deleted.
+  std::set<FormatToken *, FormatTokenLess> RedundantTokens;
----------------
I think "redundant" is the wrong word here. How about DeletedTokens?


http://reviews.llvm.org/D18551