[PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

[Daniel Jasper via Phabricator via cfe-commits]

djasper added inline comments.


================
Comment at: lib/Format/WhitespaceManager.cpp:190
 
+struct TokenTypeAndLevel {
+  TokenType Type;
----------------
I don't think you need this struct now. Just use the FormatToken itself, it should have all of this information.


================
Comment at: lib/Format/WhitespaceManager.cpp:195
+
+// Upon entry, IndentLevel is only set on the first token of the line.
+// Here we propagate IndentLevel to every token on the line.
----------------
This is no longer true. IndentLevel should be set correctly for every token.


================
Comment at: lib/Format/WhitespaceManager.cpp:288
+// If this function encounters a scope level less than the initial level,
+// it exits.
+// There is a non-obvious subtlety in the recursive behavior: Even though we
----------------
nit: s/exits/returns/

(or maybe even "returns the current position")


================
Comment at: lib/Format/WhitespaceManager.cpp:359
       ++CommasBeforeMatch;
-    } else if (Changes[i].Tok->isOneOf(tok::r_brace, tok::r_paren,
-                                       tok::r_square)) {
-      --NestingLevel;
-    } else if (Changes[i].Tok->isOneOf(tok::l_brace, tok::l_paren,
-                                       tok::l_square)) {
-      // We want sequences to skip over child scopes if possible, but not the
-      // other way around.
-      NestingLevelOfLastMatch = std::min(NestingLevelOfLastMatch, NestingLevel);
-      ++NestingLevel;
+    } else if (Changes[i].NestingAndIndentLevel != NestingAndIndentLevel) {
+      // Call AlignTokens recursively, skipping over this scope block.
----------------
The "!=" is a bit confusing here. ">" would do the same thing, right (because "<" is already handled above)?


================
Comment at: lib/Format/WhitespaceManager.h:130
+    // used to add tabs for indentation.
+    std::pair<unsigned, unsigned> NestingAndIndentLevel;
+
----------------
I think you can get rid of this field now and just directly use the values stored in the tokens.


https://reviews.llvm.org/D21279