VelocityRa added inline comments.
================ Comment at: lib/Format/WhitespaceManager.cpp:520 + + AlignMacroSequence(StartOfSequence, EndOfSequence, MinColumn, MaxColumn, + FoundMatchOnLine, AlignMacrosMatches, Changes); ---------------- klimek wrote: > Why are we calling AlignMacroSequence(0, 0, ...) here? Wasn't needed apparently. ================ Comment at: lib/Format/WhitespaceManager.cpp:524 + unsigned I = 0; + for (unsigned E = Changes.size(); I != E; ++I) { + if (Changes[I].NewlinesBefore != 0) { ---------------- klimek wrote: > 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. That `if` and surrounding logic also exists and is copied from `AlignTokens` and it's not documented there. Anyway, apparently the extra `AlignMacroSequence` in this loop could be removed. I tried iteratively removing/tweaking other snippets like the `if` and tests failed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D28462/new/ https://reviews.llvm.org/D28462 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits