[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:207
+
+    if (i != Start) {
+      if (Changes[i].NestingAndIndentLevel >
----------------
Merge the two ifs into a single one?


================
Comment at: lib/Format/WhitespaceManager.cpp:318
+  for (unsigned e = Changes.size();
+       i != e && Changes[i].NestingAndIndentLevel >= NestingAndIndentLevel;
+       ++i) {
----------------
I'd probably move the second condition into an early exit inside the loop:

  if (Changes[i].NestingAndIndentLevel >= NestingAndIndentLevel)
    break;



================
Comment at: lib/Format/WhitespaceManager.cpp:394
               },
-              Changes);
+              Changes, 0);
 }
----------------
Use a comment to describe the literal, i.e.:

  /*StartAt=*/0



================
Comment at: lib/Format/WhitespaceManager.h:134
+    // A combination of nesting level and indent level.
+    // The nesting level of this token, i.e. the number of surrounding (),
+    // [], {} or <>. Note that the value stored here is slightly different
----------------
This comment seems outdated.


================
Comment at: lib/Format/WhitespaceManager.h:217
+  /// declaration name.
+  static bool IsStartOfDeclName(const FormatToken &Tok);
+
----------------
This function should be a local function in the .cpp file. Also, can you describe in more detail what this does, i.e. what kind of declarations are covered here? Also, it is a bit confusing to have a function and a member of WhitespaceManager::Change with the exact same name.


https://reviews.llvm.org/D21279