r356030 - Revert "[clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present"

Paul Hoad via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 13 01:15:03 PDT 2019


Author: paulhoad
Date: Wed Mar 13 01:15:03 2019
New Revision: 356030

URL: http://llvm.org/viewvc/llvm-project?rev=356030&view=rev
Log:
Revert "[clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present"

This reverts commit b358cbb9b78389e20f7be36e1a98e26515c3ecce.

Modified:
    cfe/trunk/docs/ClangFormatStyleOptions.rst
    cfe/trunk/include/clang/Format/Format.h
    cfe/trunk/lib/Format/Format.cpp
    cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
    cfe/trunk/unittests/Format/FormatTest.cpp
    cfe/trunk/unittests/Format/FormatTestSelective.cpp

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=356030&r1=356029&r2=356030&view=diff
==============================================================================
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Wed Mar 13 01:15:03 2019
@@ -365,47 +365,10 @@ the configuration (without a prefix: ``A
       };
       void f() { bar(); }
 
-**AllowShortIfStatementsOnASingleLine** (``ShortIfStyle``)
-  Dependent on the value, ``if (a) return 0;`` can be put on a
-  single line.
 
-  Possible values:
 
-  * ``SIS_Never`` (in configuration: ``Never``)
-    Do not allow short if functions.
-
-    .. code-block:: c++
-
-       if (a)
-         return;
-       else
-         return;
-
-  * ``SIS_WithoutElse`` (in configuration: ``WithoutElse``)
-    Allow short if functions on the same line, as long as else
-    is not a compound statement.
-
-    .. code-block:: c++
-
-       if (a) return;
-       else
-         return;
-
-       if (a)
-         return;
-       else {
-         return;
-       }
-
-  * ``SIS_Always`` (in configuration: ``Always``)
-    Allow short if statements even if the else is a compound statement.
-
-    .. code-block:: c++
-
-       if (a) return;
-       else {
-          return;
-       }
+**AllowShortIfStatementsOnASingleLine** (``bool``)
+  If ``true``, ``if (a) return;`` can be put on a single line.
 
 **AllowShortLoopsOnASingleLine** (``bool``)
   If ``true``, ``while (true) continue;`` can be put on a single

Modified: cfe/trunk/include/clang/Format/Format.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=356030&r1=356029&r2=356030&view=diff
==============================================================================
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Wed Mar 13 01:15:03 2019
@@ -241,38 +241,8 @@ struct FormatStyle {
   /// single line.
   ShortFunctionStyle AllowShortFunctionsOnASingleLine;
 
-  /// Different styles for handling short if lines
-  enum ShortIfStyle {
-    /// Never put short ifs on the same line.
-    /// \code
-    ///   if (a)
-    ///     return ;
-    ///   else {
-    ///     return;
-    ///   }
-    /// \endcode
-    SIS_Never,
-    /// Without else put short ifs on the same line only if
-    /// the else is not a compound statement.
-    /// \code
-    ///   if (a) return;
-    ///   else
-    ///     return;
-    /// \endcode
-    SIS_WithoutElse,
-    /// Always put short ifs on the same line if
-    /// the else is not a compound statement or not.
-    /// \code
-    ///   if (a) return;
-    ///   else {
-    ///     return;
-    ///   }
-    /// \endcode
-    SIS_Always,
-  };
-
   /// If ``true``, ``if (a) return;`` can be put on a single line.
-  ShortIfStyle AllowShortIfStatementsOnASingleLine;
+  bool AllowShortIfStatementsOnASingleLine;
 
   /// If ``true``, ``while (true) continue;`` can be put on a single
   /// line.

Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=356030&r1=356029&r2=356030&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Mar 13 01:15:03 2019
@@ -106,18 +106,6 @@ template <> struct ScalarEnumerationTrai
   }
 };
 
-template <> struct ScalarEnumerationTraits<FormatStyle::ShortIfStyle> {
-  static void enumeration(IO &IO, FormatStyle::ShortIfStyle &Value) {
-    IO.enumCase(Value, "Never", FormatStyle::SIS_Never);
-    IO.enumCase(Value, "Always", FormatStyle::SIS_Always);
-    IO.enumCase(Value, "WithoutElse", FormatStyle::SIS_WithoutElse);
-
-    // For backward compatibility.
-    IO.enumCase(Value, "false", FormatStyle::SIS_Never);
-    IO.enumCase(Value, "true", FormatStyle::SIS_WithoutElse);
-  }
-};
-
 template <> struct ScalarEnumerationTraits<FormatStyle::BinPackStyle> {
   static void enumeration(IO &IO, FormatStyle::BinPackStyle &Value) {
     IO.enumCase(Value, "Auto", FormatStyle::BPS_Auto);
@@ -643,7 +631,7 @@ FormatStyle getLLVMStyle(FormatStyle::La
   LLVMStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
   LLVMStyle.AllowShortBlocksOnASingleLine = false;
   LLVMStyle.AllowShortCaseLabelsOnASingleLine = false;
-  LLVMStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
+  LLVMStyle.AllowShortIfStatementsOnASingleLine = false;
   LLVMStyle.AllowShortLoopsOnASingleLine = false;
   LLVMStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
   LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
@@ -749,8 +737,7 @@ FormatStyle getGoogleStyle(FormatStyle::
 
   GoogleStyle.AccessModifierOffset = -1;
   GoogleStyle.AlignEscapedNewlines = FormatStyle::ENAS_Left;
-  GoogleStyle.AllowShortIfStatementsOnASingleLine =
-      FormatStyle::SIS_WithoutElse;
+  GoogleStyle.AllowShortIfStatementsOnASingleLine = true;
   GoogleStyle.AllowShortLoopsOnASingleLine = true;
   GoogleStyle.AlwaysBreakBeforeMultilineStrings = true;
   GoogleStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
@@ -817,7 +804,7 @@ FormatStyle getGoogleStyle(FormatStyle::
     GoogleStyle.AlignOperands = false;
     GoogleStyle.AlignTrailingComments = false;
     GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
-    GoogleStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
+    GoogleStyle.AllowShortIfStatementsOnASingleLine = false;
     GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
     GoogleStyle.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment;
     GoogleStyle.ColumnLimit = 100;
@@ -859,8 +846,7 @@ FormatStyle getGoogleStyle(FormatStyle::
 FormatStyle getChromiumStyle(FormatStyle::LanguageKind Language) {
   FormatStyle ChromiumStyle = getGoogleStyle(Language);
   if (Language == FormatStyle::LK_Java) {
-    ChromiumStyle.AllowShortIfStatementsOnASingleLine =
-        FormatStyle::SIS_WithoutElse;
+    ChromiumStyle.AllowShortIfStatementsOnASingleLine = true;
     ChromiumStyle.BreakAfterJavaFieldAnnotations = true;
     ChromiumStyle.ContinuationIndentWidth = 8;
     ChromiumStyle.IndentWidth = 4;
@@ -873,12 +859,12 @@ FormatStyle getChromiumStyle(FormatStyle
     };
     ChromiumStyle.SortIncludes = true;
   } else if (Language == FormatStyle::LK_JavaScript) {
-    ChromiumStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
+    ChromiumStyle.AllowShortIfStatementsOnASingleLine = false;
     ChromiumStyle.AllowShortLoopsOnASingleLine = false;
   } else {
     ChromiumStyle.AllowAllParametersOfDeclarationOnNextLine = false;
     ChromiumStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
-    ChromiumStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
+    ChromiumStyle.AllowShortIfStatementsOnASingleLine = false;
     ChromiumStyle.AllowShortLoopsOnASingleLine = false;
     ChromiumStyle.BinPackParameters = false;
     ChromiumStyle.DerivePointerAlignment = false;

Modified: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp?rev=356030&r1=356029&r2=356030&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp Wed Mar 13 01:15:03 2019
@@ -413,12 +413,10 @@ private:
     if (I[1]->First->isOneOf(tok::semi, tok::kw_if, tok::kw_for, tok::kw_while,
                              TT_LineComment))
       return 0;
-    // Only inline simple if's (no nested if or else), unless specified
-    if (Style.AllowShortIfStatementsOnASingleLine != FormatStyle::SIS_Always) {
-      if (I + 2 != E && Line.startsWith(tok::kw_if) &&
-          I[2]->First->is(tok::kw_else))
-        return 0;
-    }
+    // Only inline simple if's (no nested if or else).
+    if (I + 2 != E && Line.startsWith(tok::kw_if) &&
+        I[2]->First->is(tok::kw_else))
+      return 0;
     return 1;
   }
 

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=356030&r1=356029&r2=356030&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Mar 13 01:15:03 2019
@@ -439,8 +439,7 @@ TEST_F(FormatTest, FormatIfWithoutCompou
 
   FormatStyle AllowsMergedIf = getLLVMStyle();
   AllowsMergedIf.AlignEscapedNewlines = FormatStyle::ENAS_Left;
-  AllowsMergedIf.AllowShortIfStatementsOnASingleLine =
-      FormatStyle::SIS_WithoutElse;
+  AllowsMergedIf.AllowShortIfStatementsOnASingleLine = true;
   verifyFormat("if (a)\n"
                "  // comment\n"
                "  f();",
@@ -488,41 +487,6 @@ TEST_F(FormatTest, FormatIfWithoutCompou
   verifyFormat("if (a)\n  return;", AllowsMergedIf);
 }
 
-TEST_F(FormatTest, FormatIfWithoutCompoundStatementButElseWith) {
-  FormatStyle AllowsMergedIf = getLLVMStyle();
-  AllowsMergedIf.AlignEscapedNewlines = FormatStyle::ENAS_Left;
-  AllowsMergedIf.AllowShortIfStatementsOnASingleLine =
-      FormatStyle::SIS_WithoutElse;
-  verifyFormat("if (a)\n"
-               "  f();\n"
-               "else {\n"
-               "  g();\n"
-               "}",
-               AllowsMergedIf);
-  verifyFormat("if (a)\n"
-               "  f();\n"
-               "else\n"
-               "  g();\n",
-               AllowsMergedIf);
-
-  AllowsMergedIf.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Always;
-
-  verifyFormat("if (a) f();\n"
-               "else {\n"
-               "  g();\n"
-               "}",
-               AllowsMergedIf);
-  verifyFormat("if (a) f();\n"
-               "else {\n"
-               "  if (a) f();\n"
-               "  else {\n"
-               "    g();\n"
-               "  }\n"
-               "  g();\n"
-               "}",
-               AllowsMergedIf);
-}
-
 TEST_F(FormatTest, FormatLoopsWithoutCompoundStatement) {
   FormatStyle AllowsMergedLoops = getLLVMStyle();
   AllowsMergedLoops.AllowShortLoopsOnASingleLine = true;
@@ -551,8 +515,7 @@ TEST_F(FormatTest, FormatShortBracedStat
   AllowSimpleBracedStatements.ColumnLimit = 40;
   AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = true;
 
-  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine =
-      FormatStyle::SIS_WithoutElse;
+  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = true;
   AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;
 
   AllowSimpleBracedStatements.BreakBeforeBraces = FormatStyle::BS_Custom;
@@ -600,8 +563,7 @@ TEST_F(FormatTest, FormatShortBracedStat
                "};",
                AllowSimpleBracedStatements);
 
-  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine =
-      FormatStyle::SIS_Never;
+  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = false;
   verifyFormat("if (true) {}", AllowSimpleBracedStatements);
   verifyFormat("if (true) {\n"
                "  f();\n"
@@ -626,8 +588,7 @@ TEST_F(FormatTest, FormatShortBracedStat
                "}",
                AllowSimpleBracedStatements);
 
-  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine =
-      FormatStyle::SIS_WithoutElse;
+  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = true;
   AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;
   AllowSimpleBracedStatements.BraceWrapping.AfterControlStatement = true;
 
@@ -664,8 +625,7 @@ TEST_F(FormatTest, FormatShortBracedStat
                "}",
                AllowSimpleBracedStatements);
 
-  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine =
-      FormatStyle::SIS_Never;
+  AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = false;
   verifyFormat("if (true) {}", AllowSimpleBracedStatements);
   verifyFormat("if (true)\n"
                "{\n"
@@ -699,7 +659,7 @@ TEST_F(FormatTest, FormatShortBracedStat
 TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
   FormatStyle Style = getLLVMStyleWithColumns(60);
   Style.AllowShortBlocksOnASingleLine = true;
-  Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
+  Style.AllowShortIfStatementsOnASingleLine = true;
   Style.BreakBeforeBraces = FormatStyle::BS_Allman;
   EXPECT_EQ("#define A                                                  \\\n"
             "  if (HANDLEwernufrnuLwrmviferuvnierv)                     \\\n"
@@ -3197,7 +3157,7 @@ TEST_F(FormatTest, GraciouslyHandleIncor
 
 TEST_F(FormatTest, FormatsJoinedLinesOnSubsequentRuns) {
   FormatStyle SingleLine = getLLVMStyle();
-  SingleLine.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
+  SingleLine.AllowShortIfStatementsOnASingleLine = true;
   verifyFormat("#if 0\n"
                "#elif 1\n"
                "#endif\n"
@@ -8040,8 +8000,7 @@ TEST_F(FormatTest, FormatHashIfExpressio
 
 TEST_F(FormatTest, MergeHandlingInTheFaceOfPreprocessorDirectives) {
   FormatStyle AllowsMergedIf = getGoogleStyle();
-  AllowsMergedIf.AllowShortIfStatementsOnASingleLine =
-      FormatStyle::SIS_WithoutElse;
+  AllowsMergedIf.AllowShortIfStatementsOnASingleLine = true;
   verifyFormat("void f() { f(); }\n#error E", AllowsMergedIf);
   verifyFormat("if (true) return 42;\n#error E", AllowsMergedIf);
   verifyFormat("if (true)\n#error E\n  return 42;", AllowsMergedIf);
@@ -10466,8 +10425,7 @@ TEST_F(FormatTest, AllmanBraceBreaking)
   AllmanBraceStyle.ColumnLimit = 80;
 
   FormatStyle BreakBeforeBraceShortIfs = AllmanBraceStyle;
-  BreakBeforeBraceShortIfs.AllowShortIfStatementsOnASingleLine =
-      FormatStyle::SIS_WithoutElse;
+  BreakBeforeBraceShortIfs.AllowShortIfStatementsOnASingleLine = true;
   BreakBeforeBraceShortIfs.AllowShortLoopsOnASingleLine = true;
   verifyFormat("void f(bool b)\n"
                "{\n"
@@ -10929,6 +10887,7 @@ TEST_F(FormatTest, ParsesConfigurationBo
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
   CHECK_PARSE_BOOL(AllowShortBlocksOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
+  CHECK_PARSE_BOOL(AllowShortIfStatementsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);
@@ -11191,20 +11150,6 @@ TEST_F(FormatTest, ParsesConfiguration)
   CHECK_PARSE("NamespaceIndentation: All", NamespaceIndentation,
               FormatStyle::NI_All);
 
-  Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Always;
-  CHECK_PARSE("AllowShortIfStatementsOnASingleLine: Never",
-              AllowShortIfStatementsOnASingleLine, FormatStyle::SIS_Never);
-  CHECK_PARSE("AllowShortIfStatementsOnASingleLine: WithoutElse",
-              AllowShortIfStatementsOnASingleLine,
-              FormatStyle::SIS_WithoutElse);
-  CHECK_PARSE("AllowShortIfStatementsOnASingleLine: Always",
-              AllowShortIfStatementsOnASingleLine, FormatStyle::SIS_Always);
-  CHECK_PARSE("AllowShortIfStatementsOnASingleLine: false",
-              AllowShortIfStatementsOnASingleLine, FormatStyle::SIS_Never);
-  CHECK_PARSE("AllowShortIfStatementsOnASingleLine: true",
-              AllowShortIfStatementsOnASingleLine,
-              FormatStyle::SIS_WithoutElse);
-
   // FIXME: This is required because parsing a configuration simply overwrites
   // the first N elements of the list instead of resetting it.
   Style.ForEachMacros.clear();

Modified: cfe/trunk/unittests/Format/FormatTestSelective.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestSelective.cpp?rev=356030&r1=356029&r2=356030&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTestSelective.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestSelective.cpp Wed Mar 13 01:15:03 2019
@@ -98,7 +98,7 @@ TEST_F(FormatTestSelective, ReformatsMov
 }
 
 TEST_F(FormatTestSelective, FormatsIfWithoutCompoundStatement) {
-  Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
+  Style.AllowShortIfStatementsOnASingleLine = true;
   EXPECT_EQ("if (a) return;", format("if(a)\nreturn;", 7, 1));
   EXPECT_EQ("if (a) return; // comment",
             format("if(a)\nreturn; // comment", 20, 1));




More information about the cfe-commits mailing list