gedare updated this revision to Diff 543721.
gedare added a comment.
This revision is now accepted and ready to land.

Change option name from NonEmpty to NonNullStatement. Fix space before semi.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154550

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/ConfigParseTest.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1380,7 +1380,8 @@
 
 TEST_F(FormatTest, FormatLoopsWithoutCompoundStatement) {
   FormatStyle AllowsMergedLoops = getLLVMStyle();
-  AllowsMergedLoops.AllowShortLoopsOnASingleLine = true;
+  AllowsMergedLoops.AllowShortLoopsOnASingleLine =
+      FormatStyle::SWFLS_NonNullStatement;
   verifyFormat("while (true) continue;", AllowsMergedLoops);
   verifyFormat("for (;;) continue;", AllowsMergedLoops);
   verifyFormat("for (int &v : vec) v *= 2;", AllowsMergedLoops);
@@ -1436,6 +1437,66 @@
                "  while (true);\n"
                "}",
                AllowsMergedLoops);
+  AllowsMergedLoops.AllowShortLoopsOnASingleLine = FormatStyle::SWFLS_All;
+  verifyFormat("while (true) continue;", AllowsMergedLoops);
+  verifyFormat("for (;;) continue;", AllowsMergedLoops);
+  verifyFormat("for (int &v : vec) v *= 2;", AllowsMergedLoops);
+  verifyFormat("BOOST_FOREACH (int &v, vec) v *= 2;", AllowsMergedLoops);
+  verifyFormat("while (true);",
+               AllowsMergedLoops);
+  verifyFormat("for (;;);",
+               AllowsMergedLoops);
+  verifyFormat("for (;;)\n"
+               "  for (;;) continue;",
+               AllowsMergedLoops);
+  verifyFormat("for (;;)\n"
+               "  for (;;);",
+               AllowsMergedLoops);
+  verifyFormat("for (;;)\n"
+               "  while (true);",
+               AllowsMergedLoops);
+  verifyFormat("while (true)\n"
+               "  for (;;) continue;",
+               AllowsMergedLoops);
+  verifyFormat("while (true)\n"
+               "  for (;;);",
+               AllowsMergedLoops);
+  verifyFormat("BOOST_FOREACH (int &v, vec)\n"
+               "  for (;;);",
+               AllowsMergedLoops);
+  verifyFormat("for (;;)\n"
+               "  BOOST_FOREACH (int &v, vec);",
+               AllowsMergedLoops);
+  verifyFormat("for (;;) // Can't merge this\n"
+               "  ;",
+               AllowsMergedLoops);
+  verifyFormat("for (;;) /* still don't merge */\n"
+               "  ;",
+               AllowsMergedLoops);
+  verifyFormat("do a++;\n"
+               "while (true);",
+               AllowsMergedLoops);
+  verifyFormat("do /* Don't merge */\n"
+               "  a++;\n"
+               "while (true);",
+               AllowsMergedLoops);
+  verifyFormat("do // Don't merge\n"
+               "  a++;\n"
+               "while (true);",
+               AllowsMergedLoops);
+  verifyFormat("do\n"
+               "  // Don't merge\n"
+               "  a++;\n"
+               "while (true);",
+               AllowsMergedLoops);
+  // Without braces labels are interpreted differently.
+  verifyFormat("{\n"
+               "  do\n"
+               "  label:\n"
+               "    a++;\n"
+               "  while (true);\n"
+               "}",
+               AllowsMergedLoops);
 }
 
 TEST_F(FormatTest, FormatShortBracedStatements) {
@@ -1443,7 +1504,8 @@
   EXPECT_EQ(AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine, false);
   EXPECT_EQ(AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine,
             FormatStyle::SIS_Never);
-  EXPECT_EQ(AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine, false);
+  EXPECT_EQ(AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine,
+            FormatStyle::SWFLS_Never);
   EXPECT_EQ(AllowSimpleBracedStatements.BraceWrapping.AfterFunction, false);
   verifyFormat("for (;;) {\n"
                "  f();\n"
@@ -1488,7 +1550,8 @@
   AllowSimpleBracedStatements.ColumnLimit = 40;
   AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine =
       FormatStyle::SBS_Always;
-  AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;
+  AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine =
+      FormatStyle::SWFLS_NonNullStatement;
   AllowSimpleBracedStatements.BreakBeforeBraces = FormatStyle::BS_Custom;
   AllowSimpleBracedStatements.BraceWrapping.AfterFunction = true;
   AllowSimpleBracedStatements.BraceWrapping.SplitEmptyRecord = false;
@@ -1605,7 +1668,8 @@
                "}",
                AllowSimpleBracedStatements);
 
-  AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = false;
+  AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine =
+      FormatStyle::SWFLS_Never;
   verifyFormat("while (true) {}", AllowSimpleBracedStatements);
   verifyFormat("while (true) {\n"
                "  f();\n"
@@ -1624,7 +1688,8 @@
 
   AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine =
       FormatStyle::SIS_WithoutElse;
-  AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;
+  AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine =
+      FormatStyle::SWFLS_NonNullStatement;
   AllowSimpleBracedStatements.BraceWrapping.AfterControlStatement =
       FormatStyle::BWACS_Always;
 
@@ -1738,7 +1803,8 @@
                "}",
                AllowSimpleBracedStatements);
 
-  AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = false;
+  AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine =
+    FormatStyle::SWFLS_Never;
   verifyFormat("while (true) {}", AllowSimpleBracedStatements);
   verifyFormat("while (true)\n"
                "{\n"
@@ -2292,7 +2358,7 @@
 TEST_F(FormatTest, ForEachLoops) {
   FormatStyle Style = getLLVMStyle();
   EXPECT_EQ(Style.AllowShortBlocksOnASingleLine, FormatStyle::SBS_Never);
-  EXPECT_EQ(Style.AllowShortLoopsOnASingleLine, false);
+  EXPECT_EQ(Style.AllowShortLoopsOnASingleLine, FormatStyle::SWFLS_Never);
   verifyFormat("void f() {\n"
                "  for (;;) {\n"
                "  }\n"
@@ -2321,7 +2387,7 @@
 
   FormatStyle ShortBlocks = getLLVMStyle();
   ShortBlocks.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Always;
-  EXPECT_EQ(ShortBlocks.AllowShortLoopsOnASingleLine, false);
+  EXPECT_EQ(ShortBlocks.AllowShortLoopsOnASingleLine, FormatStyle::SWFLS_Never);
   verifyFormat("void f() {\n"
                "  for (;;)\n"
                "    int j = 1;\n"
@@ -2337,7 +2403,7 @@
                ShortBlocks);
 
   FormatStyle ShortLoops = getLLVMStyle();
-  ShortLoops.AllowShortLoopsOnASingleLine = true;
+  ShortLoops.AllowShortLoopsOnASingleLine = FormatStyle::SWFLS_NonNullStatement;
   EXPECT_EQ(ShortLoops.AllowShortBlocksOnASingleLine, FormatStyle::SBS_Never);
   verifyFormat("void f() {\n"
                "  for (;;) int j = 1;\n"
@@ -2353,7 +2419,8 @@
 
   FormatStyle ShortBlocksAndLoops = getLLVMStyle();
   ShortBlocksAndLoops.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Always;
-  ShortBlocksAndLoops.AllowShortLoopsOnASingleLine = true;
+  ShortBlocksAndLoops.AllowShortLoopsOnASingleLine =
+      FormatStyle::SWFLS_NonNullStatement;
   verifyFormat("void f() {\n"
                "  for (;;) int j = 1;\n"
                "  Q_FOREACH (int &v, vec) int j = 1;\n"
@@ -16507,7 +16574,8 @@
   FormatStyle SpaceForeachMacros = getLLVMStyle();
   EXPECT_EQ(SpaceForeachMacros.AllowShortBlocksOnASingleLine,
             FormatStyle::SBS_Never);
-  EXPECT_EQ(SpaceForeachMacros.AllowShortLoopsOnASingleLine, false);
+  EXPECT_EQ(SpaceForeachMacros.AllowShortLoopsOnASingleLine,
+            FormatStyle::SWFLS_Never);
   SpaceForeachMacros.SpaceBeforeParens = FormatStyle::SBPO_Custom;
   SpaceForeachMacros.SpaceBeforeParensOptions.AfterForeachMacros = true;
   verifyFormat("for (;;) {\n"
@@ -19822,7 +19890,8 @@
   FormatStyle BreakBeforeBraceShortIfs = AllmanBraceStyle;
   BreakBeforeBraceShortIfs.AllowShortIfStatementsOnASingleLine =
       FormatStyle::SIS_WithoutElse;
-  BreakBeforeBraceShortIfs.AllowShortLoopsOnASingleLine = true;
+  BreakBeforeBraceShortIfs.AllowShortLoopsOnASingleLine =
+      FormatStyle::SWFLS_NonNullStatement;
   verifyFormat("void f(bool b)\n"
                "{\n"
                "  if (b)\n"
@@ -20240,7 +20309,8 @@
   FormatStyle BreakBeforeBraceShortIfs = WhitesmithsBraceStyle;
   BreakBeforeBraceShortIfs.AllowShortIfStatementsOnASingleLine =
       FormatStyle::SIS_OnlyFirstIf;
-  BreakBeforeBraceShortIfs.AllowShortLoopsOnASingleLine = true;
+  BreakBeforeBraceShortIfs.AllowShortLoopsOnASingleLine =
+      FormatStyle::SWFLS_NonNullStatement;
   verifyFormat("void f(bool b)\n"
                "  {\n"
                "  if (b)\n"
Index: clang/unittests/Format/ConfigParseTest.cpp
===================================================================
--- clang/unittests/Format/ConfigParseTest.cpp
+++ clang/unittests/Format/ConfigParseTest.cpp
@@ -148,7 +148,6 @@
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortEnumsOnASingleLine);
-  CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
@@ -553,6 +552,21 @@
   CHECK_PARSE("AllowShortLambdasOnASingleLine: true",
               AllowShortLambdasOnASingleLine, FormatStyle::SLS_All);
 
+  Style.AllowShortLoopsOnASingleLine = FormatStyle::SWFLS_All;
+  CHECK_PARSE("AllowShortLoopsOnASingleLine: Never",
+              AllowShortLoopsOnASingleLine, FormatStyle::SWFLS_Never);
+  CHECK_PARSE("AllowShortLoopsOnASingleLine: NonNullStatement",
+              AllowShortLoopsOnASingleLine,
+              FormatStyle::SWFLS_NonNullStatement);
+  CHECK_PARSE("AllowShortLoopsOnASingleLine: All",
+              AllowShortLoopsOnASingleLine, FormatStyle::SWFLS_All);
+  // For backward compatibility:
+  CHECK_PARSE("AllowShortLoopsOnASingleLine: false",
+              AllowShortLoopsOnASingleLine, FormatStyle::SWFLS_Never);
+  CHECK_PARSE("AllowShortLoopsOnASingleLine: true",
+              AllowShortLoopsOnASingleLine,
+              FormatStyle::SWFLS_NonNullStatement);
+
   Style.SpaceAroundPointerQualifiers = FormatStyle::SAPQ_Both;
   CHECK_PARSE("SpaceAroundPointerQualifiers: Default",
               SpaceAroundPointerQualifiers, FormatStyle::SAPQ_Default);
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -619,8 +619,16 @@
       return 0;
     if (1 + I[1]->Last->TotalLength > Limit)
       return 0;
+    if (I[1]->First->is(tok::semi)) {
+      if (Line.First->isOneOf(tok::kw_for, tok::kw_while, TT_ForEachMacro) &&
+          Style.AllowShortLoopsOnASingleLine == FormatStyle::SWFLS_All) { 
+        I[1]->First->SpacesRequiredBefore = 0;
+        return 1;
+      }
+      return 0;
+    }
     // Don't merge with loops, ifs, a single semicolon or a line comment.
-    if (I[1]->First->isOneOf(tok::semi, tok::kw_if, tok::kw_for, tok::kw_while,
+    if (I[1]->First->isOneOf(tok::kw_if, tok::kw_for, tok::kw_while,
                              TT_ForEachMacro, TT_LineComment)) {
       return 0;
     }
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -590,6 +590,18 @@
   }
 };
 
+template <> struct ScalarEnumerationTraits<FormatStyle::ShortWhileForLoopStyle> {
+  static void enumeration(IO &IO, FormatStyle::ShortWhileForLoopStyle &Value) {
+    IO.enumCase(Value, "Never", FormatStyle::SWFLS_Never);
+    IO.enumCase(Value, "NonNullStatement", FormatStyle::SWFLS_NonNullStatement);
+    IO.enumCase(Value, "All", FormatStyle::SWFLS_All);
+
+    // For backward compatibility.
+    IO.enumCase(Value, "false", FormatStyle::SWFLS_Never);
+    IO.enumCase(Value, "true", FormatStyle::SWFLS_NonNullStatement);
+  }
+};
+
 template <> struct ScalarEnumerationTraits<FormatStyle::SortIncludesOptions> {
   static void enumeration(IO &IO, FormatStyle::SortIncludesOptions &Value) {
     IO.enumCase(Value, "Never", FormatStyle::SI_Never);
@@ -1332,7 +1344,7 @@
   LLVMStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
   LLVMStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
   LLVMStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_All;
-  LLVMStyle.AllowShortLoopsOnASingleLine = false;
+  LLVMStyle.AllowShortLoopsOnASingleLine = FormatStyle::SWFLS_Never;
   LLVMStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
   LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
@@ -1521,7 +1533,7 @@
   GoogleStyle.AlignEscapedNewlines = FormatStyle::ENAS_Left;
   GoogleStyle.AllowShortIfStatementsOnASingleLine =
       FormatStyle::SIS_WithoutElse;
-  GoogleStyle.AllowShortLoopsOnASingleLine = true;
+  GoogleStyle.AllowShortLoopsOnASingleLine = FormatStyle::SWFLS_NonNullStatement;
   GoogleStyle.AlwaysBreakBeforeMultilineStrings = true;
   GoogleStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
   GoogleStyle.DerivePointerAlignment = true;
@@ -1694,12 +1706,12 @@
     ChromiumStyle.SortIncludes = FormatStyle::SI_CaseSensitive;
   } else if (Language == FormatStyle::LK_JavaScript) {
     ChromiumStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
-    ChromiumStyle.AllowShortLoopsOnASingleLine = false;
+    ChromiumStyle.AllowShortLoopsOnASingleLine = FormatStyle::SWFLS_Never;
   } else {
     ChromiumStyle.AllowAllParametersOfDeclarationOnNextLine = false;
     ChromiumStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
     ChromiumStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
-    ChromiumStyle.AllowShortLoopsOnASingleLine = false;
+    ChromiumStyle.AllowShortLoopsOnASingleLine = FormatStyle::SWFLS_Never;
     ChromiumStyle.BinPackParameters = false;
     ChromiumStyle.DerivePointerAlignment = false;
     if (Language == FormatStyle::LK_ObjC)
@@ -1797,7 +1809,7 @@
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
   Style.AllowShortCaseLabelsOnASingleLine = false;
   Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
-  Style.AllowShortLoopsOnASingleLine = false;
+  Style.AllowShortLoopsOnASingleLine = FormatStyle::SWFLS_Never;
   Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
   Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
   return Style;
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -710,11 +710,31 @@
   /// \version 9
   ShortLambdaStyle AllowShortLambdasOnASingleLine;
 
-  /// If ``true``, ``while (true) continue;`` can be put on a single
+  /// Different styles for merging short while and for loops.
+  enum ShortWhileForLoopStyle : int8_t {
+    /// Never merge loops into a single line.
+    SWFLS_Never,
+    /// Only merge loops that are not terminated by an isolated null statement
+    /// (semicolon as the body).
+    /// \code
+    ///   while (true) continue;
+    ///   while (true)
+    ///       ;
+    /// \endcode
+    SWFLS_NonNullStatement,
+    /// Merge all loops fitting on a single line.
+    /// \code
+    ///   while (true) continue;
+    ///   while (true);
+    /// \endcode
+    SWFLS_All,
+  };
+
+  /// Dependent on the value, ``while (true);`` can be put on a single
   /// line.
   /// \version 3.7
-  bool AllowShortLoopsOnASingleLine;
-
+  ShortWhileForLoopStyle AllowShortLoopsOnASingleLine;
+ 
   /// Different ways to break after the function definition return type.
   /// This option is **deprecated** and is retained for backwards compatibility.
   enum DefinitionReturnTypeBreakingStyle : int8_t {
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -804,6 +804,9 @@
 - Add ``BracedInitializerIndentWidth`` which can be used to configure
   the indentation level of the contents of braced init lists.
 - Add ``KeepEmptyLinesAtEOF`` to keep empty lines at end of file.
+- Add style options ``Never``, ``NonNullStatement``, and ``All`` to control the
+  behavior of ``AllowShortLoopsOnASingleLine`` when handling loops terminated
+  by a semicolon. These options only affect while and for loops.
 
 libclang
 --------
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1259,10 +1259,35 @@
 
 .. _AllowShortLoopsOnASingleLine:
 
-**AllowShortLoopsOnASingleLine** (``Boolean``) :versionbadge:`clang-format 3.7` :ref:`¶ <AllowShortLoopsOnASingleLine>`
-  If ``true``, ``while (true) continue;`` can be put on a single
+**AllowShortLoopsOnASingleLine** (``ShortWhileForLoopStyle``) :versionbadge:`clang-format 3.7` :ref:`¶ <AllowShortLoopsOnASingleLine>`
+  Dependent on the value, ``while (true);`` can be put on a single
   line.
 
+  Possible values:
+
+  * ``SWFLS_Never`` (in configuration: ``Never``)
+    Never merge loops into a single line.
+
+  * ``SWFLS_NonNullStatement`` (in configuration: ``NonNullStatement``)
+    Only merge loops that are not terminated by an isolated null statement
+    (semicolon as the body).
+
+    .. code-block:: c++
+
+      while (true) continue;
+      while (true)
+          ;
+
+  * ``SWFLS_All`` (in configuration: ``All``)
+    Merge all loops fitting on a single line.
+
+    .. code-block:: c++
+
+      while (true) continue;
+      while (true);
+
+
+
 .. _AlwaysBreakAfterDefinitionReturnType:
 
 **AlwaysBreakAfterDefinitionReturnType** (``DefinitionReturnTypeBreakingStyle``) :versionbadge:`clang-format 3.7` :ref:`¶ <AlwaysBreakAfterDefinitionReturnType>`
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to