=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>,
=?utf-8?q?Tomáš?= Slanina <[email protected]>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/[email protected]>


owenca wrote:

> Please don't click `Update branch` or merge `main` branch.

It's hard for me to review your patch if you keep doing that. I want to compare 
your most recent changes with my suggestions and expect to see something like 
the following:
```diff
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -487,7 +487,8 @@ private:
             const FormatToken *PreviousPrevious =
                 Previous->getPreviousNonComment();
             if (PreviousPrevious &&
-                PreviousPrevious->isOneOf(tok::kw_class, tok::kw_struct)) {
+                PreviousPrevious->isOneOf(tok::kw_class, tok::kw_struct,
+                                          tok::kw_union)) {
               return 0;
             }
           }
@@ -513,7 +514,7 @@ private:
         return tryMergeRecord(I, E, Limit);
       } else if (TheLine->InPPDirective ||
                  !TheLine->First->isOneOf(tok::kw_class, tok::kw_enum,
-                                          tok::kw_struct)) {
+                                          tok::kw_struct, tok::kw_union)) {
         // Try to merge a block with left brace unwrapped that wasn't yet
         // covered.
         ShouldMerge = !Style.BraceWrapping.AfterFunction ||
@@ -586,35 +587,34 @@ private:
     const auto *Line = I[0];
     const auto *NextLine = I[1];
 
-    auto GetRelevantAfterOption = [&](const FormatToken *Tok) {
-      switch (Tok->getType()) {
-      case TT_StructLBrace:
-        return Style.BraceWrapping.AfterStruct;
-      case TT_ClassLBrace:
-        return Style.BraceWrapping.AfterClass;
-      case TT_UnionLBrace:
-        return Style.BraceWrapping.AfterUnion;
-      default:
-        return false;
+    // Current line begins both record and block, brace was not wrapped.
+    if (Line->Last->isOneOf(TT_ClassLBrace, TT_StructLBrace, TT_UnionLBrace)) {
+      auto WrapLBrace = [&](TokenType LBraceType) {
+        switch (LBraceType) {
+        case TT_ClassLBrace:
+          return Style.BraceWrapping.AfterClass;
+        case TT_StructLBrace:
+          return Style.BraceWrapping.AfterStruct;
+        case TT_UnionLBrace:
+          return Style.BraceWrapping.AfterUnion;
+        default:
+          return false;
+        };
       };
-    };
 
-    // Current line begins both record and block, brace was not wrapped.
-    if (Line->Last->isOneOf(TT_StructLBrace, TT_ClassLBrace, TT_UnionLBrace)) {
       auto TryMergeShortRecord = [&] {
         switch (Style.AllowShortRecordOnASingleLine) {
         case FormatStyle::SRS_Never:
           return false;
-        case FormatStyle::SRS_EmptyIfAttached:
-        case FormatStyle::SRS_Empty:
-          return NextLine->First->is(tok::r_brace);
         case FormatStyle::SRS_Always:
           return true;
+        default:
+          return NextLine->First->is(tok::r_brace);
         }
       };
 
       if (Style.AllowShortRecordOnASingleLine != FormatStyle::SRS_Never &&
-          (!GetRelevantAfterOption(Line->Last) ||
+          (!WrapLBrace(Line->Last->getType()) ||
            (!Style.BraceWrapping.SplitEmptyRecord && TryMergeShortRecord()))) {
         return tryMergeSimpleBlock(I, E, Limit);
       }
@@ -622,27 +622,19 @@ private:
 
     // Cases where the l_brace was wrapped.
     // Current line begins record, next line block.
-    if (NextLine->First->isOneOf(TT_StructLBrace, TT_ClassLBrace,
+    if (NextLine->First->isOneOf(TT_ClassLBrace, TT_StructLBrace,
                                  TT_UnionLBrace)) {
-      if (I + 2 == E)
+      if (I + 2 == E || I[2]->First->is(tok::r_brace) ||
+          Style.AllowShortRecordOnASingleLine != FormatStyle::SRS_Always) {
         return 0;
-
-      const bool IsEmptyBlock = I[2]->First->is(tok::r_brace);
-
-      if (!IsEmptyBlock &&
-          Style.AllowShortRecordOnASingleLine == FormatStyle::SRS_Always) {
-        return tryMergeSimpleBlock(I, E, Limit);
       }
-    }
 
-    if (I == AnnotatedLines.begin())
-      return 0;
-
-    const auto *PreviousLine = I[-1];
+      return tryMergeSimpleBlock(I, E, Limit);
+    }
 
     // Previous line begins record, current line block.
-    if (PreviousLine->First->isOneOf(tok::kw_struct, tok::kw_class,
-                                     tok::kw_union)) {
+    if (I != AnnotatedLines.begin() &&
+        I[-1]->First->isOneOf(tok::kw_class, tok::kw_struct, tok::kw_union)) {
       const bool IsEmptyBlock =
           Line->Last->is(tok::l_brace) && NextLine->First->is(tok::r_brace);
```
and no changes to other files.

https://github.com/llvm/llvm-project/pull/154580
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to