From c7f101d7ac7ea1cedaf1915cff124c5bd2f8f9dc Mon Sep 17 00:00:00 2001 From: Paul Hoad Date: Wed, 13 Mar 2019 08:15:03 +0000 Subject: [PATCH] Revert "[clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present" This reverts commit b358cbb9b78389e20f7be36e1a98e26515c3ecce. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@356030 91177308-0d34-0410-b5e6-96231b3b80d8 --- docs/ClangFormatStyleOptions.rst | 41 +------------ include/clang/Format/Format.h | 32 +--------- lib/Format/Format.cpp | 26 ++------ lib/Format/UnwrappedLineFormatter.cpp | 10 ++-- unittests/Format/FormatTest.cpp | 75 ++++-------------------- unittests/Format/FormatTestSelective.cpp | 2 +- 6 files changed, 24 insertions(+), 162 deletions(-) diff --git a/docs/ClangFormatStyleOptions.rst b/docs/ClangFormatStyleOptions.rst index d4a56d3bb3..582b9d938c 100644 --- a/docs/ClangFormatStyleOptions.rst +++ b/docs/ClangFormatStyleOptions.rst @@ -365,47 +365,10 @@ the configuration (without a prefix: ``Auto``). }; 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 diff --git a/include/clang/Format/Format.h b/include/clang/Format/Format.h index 4d34a559c8..94cceb30dc 100644 --- a/include/clang/Format/Format.h +++ b/include/clang/Format/Format.h @@ -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. diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp index ce67b82e10..a56fed9dee 100644 --- a/lib/Format/Format.cpp +++ b/lib/Format/Format.cpp @@ -106,18 +106,6 @@ template <> struct ScalarEnumerationTraits { } }; -template <> struct ScalarEnumerationTraits { - 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 { static void enumeration(IO &IO, FormatStyle::BinPackStyle &Value) { IO.enumCase(Value, "Auto", FormatStyle::BPS_Auto); @@ -643,7 +631,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) { 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::LanguageKind Language) { 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::LanguageKind Language) { 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::LanguageKind Language) { 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::LanguageKind Language) { }; 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; diff --git a/lib/Format/UnwrappedLineFormatter.cpp b/lib/Format/UnwrappedLineFormatter.cpp index f14e3851c8..a62755840d 100644 --- a/lib/Format/UnwrappedLineFormatter.cpp +++ b/lib/Format/UnwrappedLineFormatter.cpp @@ -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; } diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index 7fcff46f58..650636b763 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -439,8 +439,7 @@ TEST_F(FormatTest, FormatIfWithoutCompoundStatement) { 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, FormatIfWithoutCompoundStatement) { 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, FormatShortBracedStatements) { 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, FormatShortBracedStatements) { "};", 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, FormatShortBracedStatements) { "}", AllowSimpleBracedStatements); - AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = - FormatStyle::SIS_WithoutElse; + AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = true; AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true; AllowSimpleBracedStatements.BraceWrapping.AfterControlStatement = true; @@ -664,8 +625,7 @@ TEST_F(FormatTest, FormatShortBracedStatements) { "}", 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, FormatShortBracedStatements) { 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, GraciouslyHandleIncorrectPreprocessorConditions) { 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, FormatHashIfExpressions) { 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, ParsesConfigurationBools) { 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(); diff --git a/unittests/Format/FormatTestSelective.cpp b/unittests/Format/FormatTestSelective.cpp index f031a3dee5..b89c04755f 100644 --- a/unittests/Format/FormatTestSelective.cpp +++ b/unittests/Format/FormatTestSelective.cpp @@ -98,7 +98,7 @@ TEST_F(FormatTestSelective, ReformatsMovedLines) { } 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)); -- 2.40.0