[PATCH] Implement correct indentation across preprocessor branches.

[Daniel Jasper]

================
Comment at: lib/Format/Format.cpp:1252
@@ +1251,3 @@
+    assert(!UnwrappedLines.empty());
+    UnwrappedLines.rbegin()->push_back(TheLine);
+  }
----------------
I would use back(), but probably irrelevant ..

================
Comment at: lib/Format/FormatToken.h:78
@@ -77,1 +77,3 @@
 
+enum FormatState {
+  FS_Unformatted,
----------------
I don't like this name as it is too closely related to LineState and ParenState. We'll eventually confuse LineState (which actually is a FormatState with this). Maybe FormatDecision?

================
Comment at: lib/Format/FormatToken.h:347
@@ -339,1 +346,3 @@
 
+  /// \brief Stores the formatting decision for the token once it was made.
+  FormatState State;
----------------
Ha, so you agree that it is a decision (didn't see this before making the name suggestion above) :-)

================
Comment at: lib/Format/FormatToken.h:350
@@ +349,3 @@
+
+  /// \brief If true, this token has been fully formatted (indented and
+  /// potentially re-formatted inside), and we do not allow further formatting
----------------
nit: If \c true, ...

================
Comment at: lib/Format/UnwrappedLineParser.cpp:228
@@ +227,3 @@
+    while (!PPLevelBranchIndex.empty() &&
+           *PPLevelBranchIndex.rbegin() + 1 == *PPLevelBranchCount.rbegin()) {
+      PPLevelBranchIndex.resize(PPLevelBranchIndex.size() - 1);
----------------
Use PPLevelBranchIndex.back() here and below.

================
Comment at: lib/Format/UnwrappedLineParser.cpp:194
@@ +193,3 @@
+void UnwrappedLineParser::reset() {
+  PPBranchLevel = -1;
+  Line.reset(new UnwrappedLine);
----------------
Seems weird to initialize this to 0 in the constructor and to -1 here

================
Comment at: lib/Format/UnwrappedLineParser.cpp:223
@@ +222,3 @@
+         I != E; ++I) {
+      Callback.consumeUnwrappedLine(*I);
+    }
----------------
This seems like a really weird interface now (iterating over an array, adding each line and then calling finish). Would it make sense to change to a single method call where we can pass in an array? Then on the other side you could save the logic to add an empty array for the next set of lines in finishRun(), which is not really intuitive..


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