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

[Daniel Jasper via cfe-commits]

djasper added a comment.

Sorry :(... Review is easy enough.. Feel free to ping me more often in the future.


================
Comment at: lib/Format/WhitespaceManager.cpp:95
@@ -97,2 +94,3 @@
   std::sort(Changes.begin(), Changes.end(), Change::IsBeforeInFile(SourceMgr));
+  calculateScopeLevel();
   calculateLineBreakInformation();
----------------
bmharper wrote:
> berenm wrote:
> > Maybe we could spare the computation if we aren't going to align anything?
> > 
> > Is it better for clarity to always compute additional information? @djasper what's the Clang way to do?
> > 
> That's a good point. One certainly could elide that if alignment was turned off. I think so long as it was mentioned in the comments of the ScopeLevel member variable, it would be OK to do so. However, I'll also just defer this decision to @djasper.
Yeah, just avoid unnecessary work.

================
Comment at: lib/Format/WhitespaceManager.h:173
@@ -165,1 +172,3 @@
+    // down to -1.
+    int ScopeLevel;
   };
----------------
NestingLevel does include braces, generally. However, there are two types:
- Braced initializers: These should just work as is.
- Braces that open blocks: Here, child lines are created and so the tokens within a block restart from NestingLevel 0. However, taking that NestingLevel in combination with the Level of the AnnotatedLine should work.

I think reusing that is highly preferable over implementing yet another parentheses counting.


Repository:
  rL LLVM

https://reviews.llvm.org/D21279