[PATCH] D93938: [clang-format] Fixed AfterEnum handling

[Ally Tiritoglu via Phabricator via cfe-commits]

atirit updated this revision to Diff 330860.
atirit added a comment.

Fixed remote build


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93938/new/

https://reviews.llvm.org/D93938

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1344,13 +1344,68 @@
   Style.AllowShortEnumsOnASingleLine = true;
   verifyFormat("enum { A, B, C } ShortEnum1, ShortEnum2;", Style);
   Style.AllowShortEnumsOnASingleLine = false;
+  verifyFormat("enum {\n"
+               "  A,\n"
+               "  B,\n"
+               "  C\n"
+               "} ShortEnum1, ShortEnum2;",
+               Style);
+}
+
+TEST_F(FormatTest, AfterEnum) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+
+  Style.AllowShortEnumsOnASingleLine = true;
+  Style.BraceWrapping.AfterEnum = true;
+  verifyFormat("enum { A, B, C } Test1;", Style);
+  verifyFormat("enum\n"
+               "{\n"
+               "  A,\n"
+               "  B, // foo\n"
+               "  C\n"
+               "} Test2;",
+               Style);
+  Style.BraceWrapping.AfterEnum = false;
+  verifyFormat("enum { A, B, C } Test3;", Style);
+  verifyFormat("enum {\n"
+               "  A,\n"
+               "  B, // foo\n"
+               "  C\n"
+               "} Test4;",
+               Style);
+
+  Style.AllowShortEnumsOnASingleLine = false;
+  Style.BraceWrapping.AfterEnum = true;
   verifyFormat("enum\n"
                "{\n"
                "  A,\n"
                "  B,\n"
                "  C\n"
-               "} ShortEnum1, ShortEnum2;",
+               "} Test5;",
+               Style);
+  Style.BraceWrapping.AfterEnum = false;
+  verifyFormat("enum {\n"
+               "  A,\n"
+               "  B,\n"
+               "  C\n"
+               "} Test6;",
+               Style);
+}
+
+TEST_F(FormatTest, AfterEnumShortEnums) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+
+  Style.AllowShortEnumsOnASingleLine = true;
+  Style.BraceWrapping.AfterEnum = true;
+  verifyFormat("enum\n"
+               "{\n"
+               "  A,\n"
+               "  B,\n"
+               "} ShortEnum;",
                Style);
+  verifyFormat("enum { A, B } ShortEnum;", Style);
 }
 
 TEST_F(FormatTest, ShortCaseLabels) {
@@ -13276,6 +13331,7 @@
 TEST_F(FormatTest, AllmanBraceBreaking) {
   FormatStyle AllmanBraceStyle = getLLVMStyle();
   AllmanBraceStyle.BreakBeforeBraces = FormatStyle::BS_Allman;
+  AllmanBraceStyle.AllowShortEnumsOnASingleLine = false;
 
   EXPECT_EQ("namespace a\n"
             "{\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -694,6 +694,8 @@
     return Style.BraceWrapping.AfterUnion;
   if (InitialToken.is(tok::kw_struct))
     return Style.BraceWrapping.AfterStruct;
+  if (InitialToken.is(tok::kw_enum))
+    return Style.BraceWrapping.AfterEnum;
   return false;
 }
 
@@ -2482,8 +2484,9 @@
     return true;
   }
 
-  if (!Style.AllowShortEnumsOnASingleLine)
+  if (!Style.AllowShortEnumsOnASingleLine && Style.BraceWrapping.AfterEnum)
     addUnwrappedLine();
+
   // Parse enum body.
   nextToken();
   if (!Style.AllowShortEnumsOnASingleLine) {
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3662,12 +3662,37 @@
     return true;
   }
 
-  if (isAllmanBrace(Left) || isAllmanBrace(Right))
-    return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
-           (Line.startsWith(tok::kw_typedef, tok::kw_enum) &&
-            Style.BraceWrapping.AfterEnum) ||
-           (Line.startsWith(tok::kw_class) && Style.BraceWrapping.AfterClass) ||
-           (Line.startsWith(tok::kw_struct) && Style.BraceWrapping.AfterStruct);
+  if (isAllmanBrace(Left) || isAllmanBrace(Right)) {
+    // The tokens that could force an enum to not be on a single line are a
+    // trailing comment and a trailing comma on the last case. This checks for
+    // those.
+    auto isAllowedByShortEnums = [&]() {
+      if (!Style.AllowShortEnumsOnASingleLine ||
+          (strlen(Right.TokenText.data()) + Right.OriginalColumn) >
+              Style.ColumnLimit)
+        return true;
+
+      const FormatToken *breakingSearchToken = &Right;
+      while ((breakingSearchToken = breakingSearchToken->Next)) {
+        bool hasBreakingComma = breakingSearchToken->is(tok::comma) &&
+                                breakingSearchToken->Next->is(tok::r_brace);
+        if (breakingSearchToken->isTrailingComment() || hasBreakingComma) {
+          return true;
+        }
+      }
+
+      return false;
+    };
+    bool isAllowedByAfterEnum =
+        (Line.startsWith(tok::kw_enum) ||
+         Line.startsWith(tok::kw_typedef, tok::kw_enum)) &&
+        Style.BraceWrapping.AfterEnum;
+    return (Line.startsWith(tok::kw_class) && Style.BraceWrapping.AfterClass) ||
+           (Line.startsWith(tok::kw_struct) &&
+            Style.BraceWrapping.AfterStruct) ||
+           (isAllowedByAfterEnum && isAllowedByShortEnums());
+  }
+
   if (Left.is(TT_ObjCBlockLBrace) &&
       Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Never)
     return true;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits