enyquist marked 8 inline comments as done.
enyquist added inline comments.
================
Comment at: lib/Format/WhitespaceManager.cpp:287
SmallVector<WhitespaceManager::Change, 16>
&Changes,
+ bool ConsiderScope, bool ConsiderCommas,
unsigned StartAt) {
----------------
djasper wrote:
> I don't find it intuitive what "consider" means in this case. Can you add a
> comment? Are the two parameters ever set independently? If not, I'd prefer to
> keep one parameter for now as we have test coverage only for that.
Currently, they are never set independently, no. I will combine them and add an
explanatory comment.
================
Comment at: lib/Format/WhitespaceManager.cpp:413
+
+ while (Param && !Param->is(tok::l_paren)) {
+ if (!Param->is(tok::identifier) && !Param->is(tok::comma))
----------------
djasper wrote:
> I think you should be able to use Current.MatchingParen here.
Hmm, I couldn't make this work... I just replaced this line with
while (Param && Param != Current->MatchingParen)
But it must not be doing what I think it's doing, since it made some tests fail.
Mind you, my C-brain might be taking over here, please let me know if I'm using
MatchingParen incorrectly
================
Comment at: unittests/Format/FormatTest.cpp:7733
- verifyFormat("auto lambda = []() {\n"
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
----------------
djasper wrote:
> Why this change? This seems to be tested above.
I wanted to ensure this option didn't break other alignment options while I was
developing-- this is a remnant. I will remove it.
================
Comment at: unittests/Format/FormatTest.cpp:8010
Alignment.AlignConsecutiveAssignments = true;
- verifyFormat("auto lambda = []() {\n"
+ verifyFormat("#define a 5\n"
+ "#define foo(x, y) (x + y)\n"
----------------
djasper wrote:
> Same here?
Yes.
Repository:
rL LLVM
https://reviews.llvm.org/D28462
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
