[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

Manuel Klimek via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 28 09:23:44 PDT 2019


klimek added inline comments.


================
Comment at: lib/Format/WhitespaceManager.cpp:520
+
+  AlignMacroSequence(StartOfSequence, EndOfSequence, MinColumn, MaxColumn,
+                     FoundMatchOnLine, AlignMacrosMatches, Changes);
----------------
Why are we calling AlignMacroSequence(0, 0, ...) here?


================
Comment at: lib/Format/WhitespaceManager.cpp:524
+  unsigned I = 0;
+  for (unsigned E = Changes.size(); I != E; ++I) {
+    if (Changes[I].NewlinesBefore != 0) {
----------------
I still think the code in this loop either needs significantly more comments to explain what's going on, or needs to be changed to be more straight forward:
What I don't understand is why we're calling AlignMacroSequence potentially multiple times, and especially what things like the if in line 541 are for.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D28462/new/

https://reviews.llvm.org/D28462





More information about the llvm-commits mailing list