curdeius requested changes to this revision. curdeius added inline comments. This revision now requires changes to proceed.
================ Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:364 + + if (TheLine->Last->is(tok::l_brace) && I != AnnotatedLines.begin() && + I[-1]->Last && Style.BraceWrapping.SplitEmptyRecord) { ---------------- Could you please add a comment about this block of code? Something's like "Don't merge blocks when SplitEmptyRecords is enabled." or whatever you judge more useful/correct. ================ Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:370 + Previous = Previous->getPreviousNonComment(); + if (Previous && Previous->is(tok::greater)) + return 0; ---------------- You might want to factor out the check of Previous from this and the next if. ================ Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:375 + Previous->getPreviousNonComment(); + if (PreviousPrevious && Previous->is(tok::identifier) && + PreviousPrevious->isOneOf(tok::kw_class, tok::kw_struct)) ---------------- You should check the type of Previous before computing PreviousPrevious to avoid the latter when unnecessary. ================ Comment at: clang/unittests/Format/FormatTest.cpp:9952 + "{\n" + " int x;\n" + "};", ---------------- What about testing specializations with empty body? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93839/new/ https://reviews.llvm.org/D93839 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits