[PATCH] D93839: [clang-format] PR48594 BraceWrapping: SplitEmptyRecord ignored for templates

[Marek Kurdej via Phabricator via cfe-commits]

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