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
  • [PATCH] D9383... MyDeveloperDay via Phabricator via cfe-commits
    • [PATCH] ... Johel Ernesto Guerrero Peña via Phabricator via cfe-commits
    • [PATCH] ... Björn Schäpers via Phabricator via cfe-commits
    • [PATCH] ... Johel Ernesto Guerrero Peña via Phabricator via cfe-commits
    • [PATCH] ... MyDeveloperDay via Phabricator via cfe-commits
    • [PATCH] ... MyDeveloperDay via Phabricator via cfe-commits
    • [PATCH] ... Marek Kurdej via Phabricator via cfe-commits
    • [PATCH] ... MyDeveloperDay via Phabricator via cfe-commits
    • [PATCH] ... Marek Kurdej via Phabricator via cfe-commits
    • [PATCH] ... MyDeveloperDay via Phabricator via cfe-commits

Reply via email to