[clang] 8d93d7f - [clang-format] Add options to AllowShortIfStatementsOnASingleLine to apply to "else if" and "else".
Marek Kurdej via cfe-commits
cfe-commits at lists.llvm.org
Mon May 3 09:11:31 PDT 2021
Author: Marek Kurdej
Date: 2021-05-03T18:11:25+02:00
New Revision: 8d93d7ffedebc5f18dee22ba954d38a1d2d0affa
URL: https://github.com/llvm/llvm-project/commit/8d93d7ffedebc5f18dee22ba954d38a1d2d0affa
DIFF: https://github.com/llvm/llvm-project/commit/8d93d7ffedebc5f18dee22ba954d38a1d2d0affa.diff
LOG: [clang-format] Add options to AllowShortIfStatementsOnASingleLine to apply to "else if" and "else".
This fixes the bug http://llvm.org/pr50019.
Reviewed By: MyDeveloperDay
Differential Revision: https://reviews.llvm.org/D100727
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/unittests/Format/FormatTest.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d178097ac8e67..fe3b3257c6e46 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1,4 +1,4 @@
-========================================
+q========================================
Clang 13.0.0 (In-Progress) Release Notes
========================================
@@ -239,6 +239,10 @@ clang-format
- Option ``SpacesInAngles`` has been improved, it now accepts ``Leave`` value
that allows to keep spaces where they are already present.
+- Option ``AllowShortIfStatementsOnASingleLine`` has been improved, it now
+ accepts ``AllIfsAndElse`` value that allows to put "else if" and "else" short
+ statements on a single line. (Fixes https://llvm.org/PR50019.)
+
libclang
--------
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 6c49497a56996..d13c6ff4d9334 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -620,37 +620,74 @@ struct FormatStyle {
/// single line.
ShortFunctionStyle AllowShortFunctionsOnASingleLine;
- /// Different styles for handling short if lines
+ /// Different styles for handling short if statements.
enum ShortIfStyle : unsigned char {
/// Never put short ifs on the same line.
/// \code
/// if (a)
- /// return ;
+ /// return;
+ ///
+ /// if (b)
+ /// return;
+ /// else
+ /// return;
+ ///
+ /// if (c)
+ /// return;
/// else {
/// return;
/// }
/// \endcode
SIS_Never,
- /// Without else put short ifs on the same line only if
- /// the else is not a compound statement.
+ /// Put short ifs on the same line only if there is no else statement.
/// \code
/// if (a) return;
+ ///
+ /// if (b)
+ /// return;
/// else
/// return;
+ ///
+ /// if (c)
+ /// return;
+ /// else {
+ /// return;
+ /// }
/// \endcode
SIS_WithoutElse,
- /// Always put short ifs on the same line if
- /// the else is not a compound statement or not.
+ /// Put short ifs, but not else ifs nor else statements, on the same line.
/// \code
/// if (a) return;
+ ///
+ /// if (b) return;
+ /// else if (b)
+ /// return;
+ /// else
+ /// return;
+ ///
+ /// if (c) return;
+ /// else {
+ /// return;
+ /// }
+ /// \endcode
+ SIS_OnlyFirstIf,
+ /// Always put short ifs, else ifs and else statements on the same
+ /// line.
+ /// \code
+ /// if (a) return;
+ ///
+ /// if (b) return;
+ /// else return;
+ ///
+ /// if (c) return;
/// else {
/// return;
/// }
/// \endcode
- SIS_Always,
+ SIS_AllIfsAndElse,
};
- /// If ``true``, ``if (a) return;`` can be put on a single line.
+ /// Dependent on the value, ``if (a) return;`` can be put on a single line.
ShortIfStyle AllowShortIfStatementsOnASingleLine;
/// Different styles for merging short lambdas containing at most one
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 7a80af24ab48c..ba7b03de8b3d3 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -146,10 +146,12 @@ template <> struct ScalarEnumerationTraits<FormatStyle::AlignConsecutiveStyle> {
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);
+ IO.enumCase(Value, "OnlyFirstIf", FormatStyle::SIS_OnlyFirstIf);
+ IO.enumCase(Value, "AllIfsAndElse", FormatStyle::SIS_AllIfsAndElse);
// For backward compatibility.
+ IO.enumCase(Value, "Always", FormatStyle::SIS_OnlyFirstIf);
IO.enumCase(Value, "false", FormatStyle::SIS_Never);
IO.enumCase(Value, "true", FormatStyle::SIS_WithoutElse);
}
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index e38d90d58362e..8d1694fe4b442 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -421,7 +421,16 @@ class LineJoiner {
}
return MergedLines;
}
- if (TheLine->First->is(tok::kw_if)) {
+ auto IsElseLine = [&First = TheLine->First]() -> bool {
+ if (First->is(tok::kw_else))
+ return true;
+
+ return First->is(tok::r_brace) && First->Next &&
+ First->Next->is(tok::kw_else);
+ };
+ if (TheLine->First->is(tok::kw_if) ||
+ (IsElseLine() && (Style.AllowShortIfStatementsOnASingleLine ==
+ FormatStyle::SIS_AllIfsAndElse))) {
return Style.AllowShortIfStatementsOnASingleLine
? tryMergeSimpleControlStatement(I, E, Limit)
: 0;
@@ -471,7 +480,8 @@ class LineJoiner {
return 0;
Limit = limitConsideringMacros(I + 1, E, Limit);
AnnotatedLine &Line = **I;
- if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren))
+ if (!Line.First->is(tok::kw_do) && !Line.First->is(tok::kw_else) &&
+ !Line.Last->is(tok::kw_else) && Line.Last->isNot(tok::r_paren))
return 0;
// Only merge do while if do is the only statement on the line.
if (Line.First->is(tok::kw_do) && !Line.Last->is(tok::kw_do))
@@ -482,7 +492,8 @@ class LineJoiner {
TT_LineComment))
return 0;
// Only inline simple if's (no nested if or else), unless specified
- if (Style.AllowShortIfStatementsOnASingleLine != FormatStyle::SIS_Always) {
+ if (Style.AllowShortIfStatementsOnASingleLine ==
+ FormatStyle::SIS_WithoutElse) {
if (I + 2 != E && Line.startsWith(tok::kw_if) &&
I[2]->First->is(tok::kw_else))
return 0;
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 03364ff9dd987..38b4c654f817a 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -461,6 +461,69 @@ TEST_F(FormatTest, FormatIfWithoutCompoundStatement) {
" }\n"
"g();");
+ verifyFormat("if (a)\n"
+ " g();");
+ verifyFormat("if (a) {\n"
+ " g()\n"
+ "};");
+ verifyFormat("if (a)\n"
+ " g();\n"
+ "else\n"
+ " g();");
+ verifyFormat("if (a) {\n"
+ " g();\n"
+ "} else\n"
+ " g();");
+ verifyFormat("if (a)\n"
+ " g();\n"
+ "else {\n"
+ " g();\n"
+ "}");
+ verifyFormat("if (a) {\n"
+ " g();\n"
+ "} else {\n"
+ " g();\n"
+ "}");
+ verifyFormat("if (a)\n"
+ " g();\n"
+ "else if (b)\n"
+ " g();\n"
+ "else\n"
+ " g();");
+ verifyFormat("if (a) {\n"
+ " g();\n"
+ "} else if (b)\n"
+ " g();\n"
+ "else\n"
+ " g();");
+ verifyFormat("if (a)\n"
+ " g();\n"
+ "else if (b) {\n"
+ " g();\n"
+ "} else\n"
+ " g();");
+ verifyFormat("if (a)\n"
+ " g();\n"
+ "else if (b)\n"
+ " g();\n"
+ "else {\n"
+ " g();\n"
+ "}");
+ verifyFormat("if (a)\n"
+ " g();\n"
+ "else if (b) {\n"
+ " g();\n"
+ "} else {\n"
+ " g();\n"
+ "}");
+ verifyFormat("if (a) {\n"
+ " g();\n"
+ "} else if (b) {\n"
+ " g();\n"
+ "} else {\n"
+ " g();\n"
+ "}");
+
FormatStyle AllowsMergedIf = getLLVMStyle();
AllowsMergedIf.AlignEscapedNewlines = FormatStyle::ENAS_Left;
AllowsMergedIf.AllowShortIfStatementsOnASingleLine =
@@ -510,6 +573,59 @@ TEST_F(FormatTest, FormatIfWithoutCompoundStatement) {
AllowsMergedIf.ColumnLimit = 13;
verifyFormat("if (a)\n return;", AllowsMergedIf);
+
+ FormatStyle AllowsMergedIfElse = getLLVMStyle();
+ AllowsMergedIfElse.AllowShortIfStatementsOnASingleLine =
+ FormatStyle::SIS_AllIfsAndElse;
+ verifyFormat("if (a)\n"
+ " // comment\n"
+ " f();\n"
+ "else\n"
+ " // comment\n"
+ " f();",
+ AllowsMergedIfElse);
+ verifyFormat("{\n"
+ " if (a)\n"
+ " label:\n"
+ " f();\n"
+ " else\n"
+ " label:\n"
+ " f();\n"
+ "}",
+ AllowsMergedIfElse);
+ verifyFormat("if (a)\n"
+ " ;\n"
+ "else\n"
+ " ;",
+ AllowsMergedIfElse);
+ verifyFormat("if (a) {\n"
+ "} else {\n"
+ "}",
+ AllowsMergedIfElse);
+ verifyFormat("if (a) return;\n"
+ "else if (b) return;\n"
+ "else return;",
+ AllowsMergedIfElse);
+ verifyFormat("if (a) {\n"
+ "} else return;",
+ AllowsMergedIfElse);
+ verifyFormat("if (a) {\n"
+ "} else if (b) return;\n"
+ "else return;",
+ AllowsMergedIfElse);
+ verifyFormat("if (a) return;\n"
+ "else if (b) {\n"
+ "} else return;",
+ AllowsMergedIfElse);
+ verifyFormat("if (a)\n"
+ " if (b) return;\n"
+ " else return;",
+ AllowsMergedIfElse);
+ verifyFormat("if constexpr (a)\n"
+ " if constexpr (b) return;\n"
+ " else if constexpr (c) return;\n"
+ " else return;",
+ AllowsMergedIfElse);
}
TEST_F(FormatTest, FormatIfWithoutCompoundStatementButElseWith) {
@@ -529,7 +645,166 @@ TEST_F(FormatTest, FormatIfWithoutCompoundStatementButElseWith) {
" g();\n",
AllowsMergedIf);
- AllowsMergedIf.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Always;
+ verifyFormat("if (a) g();", AllowsMergedIf);
+ verifyFormat("if (a) {\n"
+ " g()\n"
+ "};",
+ AllowsMergedIf);
+ verifyFormat("if (a)\n"
+ " g();\n"
+ "else\n"
+ " g();",
+ AllowsMergedIf);
+ verifyFormat("if (a) {\n"
+ " g();\n"
+ "} else\n"
+ " g();",
+ AllowsMergedIf);
+ verifyFormat("if (a)\n"
+ " g();\n"
+ "else {\n"
+ " g();\n"
+ "}",
+ AllowsMergedIf);
+ verifyFormat("if (a) {\n"
+ " g();\n"
+ "} else {\n"
+ " g();\n"
+ "}",
+ AllowsMergedIf);
+ verifyFormat("if (a)\n"
+ " g();\n"
+ "else if (b)\n"
+ " g();\n"
+ "else\n"
+ " g();",
+ AllowsMergedIf);
+ verifyFormat("if (a) {\n"
+ " g();\n"
+ "} else if (b)\n"
+ " g();\n"
+ "else\n"
+ " g();",
+ AllowsMergedIf);
+ verifyFormat("if (a)\n"
+ " g();\n"
+ "else if (b) {\n"
+ " g();\n"
+ "} else\n"
+ " g();",
+ AllowsMergedIf);
+ verifyFormat("if (a)\n"
+ " g();\n"
+ "else if (b)\n"
+ " g();\n"
+ "else {\n"
+ " g();\n"
+ "}",
+ AllowsMergedIf);
+ verifyFormat("if (a)\n"
+ " g();\n"
+ "else if (b) {\n"
+ " g();\n"
+ "} else {\n"
+ " g();\n"
+ "}",
+ AllowsMergedIf);
+ verifyFormat("if (a) {\n"
+ " g();\n"
+ "} else if (b) {\n"
+ " g();\n"
+ "} else {\n"
+ " g();\n"
+ "}",
+ AllowsMergedIf);
+
+ AllowsMergedIf.AllowShortIfStatementsOnASingleLine =
+ FormatStyle::SIS_OnlyFirstIf;
+
+ 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);
+
+ verifyFormat("if (a) g();", AllowsMergedIf);
+ verifyFormat("if (a) {\n"
+ " g()\n"
+ "};",
+ AllowsMergedIf);
+ verifyFormat("if (a) g();\n"
+ "else\n"
+ " g();",
+ AllowsMergedIf);
+ verifyFormat("if (a) {\n"
+ " g();\n"
+ "} else\n"
+ " g();",
+ AllowsMergedIf);
+ verifyFormat("if (a) g();\n"
+ "else {\n"
+ " g();\n"
+ "}",
+ AllowsMergedIf);
+ verifyFormat("if (a) {\n"
+ " g();\n"
+ "} else {\n"
+ " g();\n"
+ "}",
+ AllowsMergedIf);
+ verifyFormat("if (a) g();\n"
+ "else if (b)\n"
+ " g();\n"
+ "else\n"
+ " g();",
+ AllowsMergedIf);
+ verifyFormat("if (a) {\n"
+ " g();\n"
+ "} else if (b)\n"
+ " g();\n"
+ "else\n"
+ " g();",
+ AllowsMergedIf);
+ verifyFormat("if (a) g();\n"
+ "else if (b) {\n"
+ " g();\n"
+ "} else\n"
+ " g();",
+ AllowsMergedIf);
+ verifyFormat("if (a) g();\n"
+ "else if (b)\n"
+ " g();\n"
+ "else {\n"
+ " g();\n"
+ "}",
+ AllowsMergedIf);
+ verifyFormat("if (a) g();\n"
+ "else if (b) {\n"
+ " g();\n"
+ "} else {\n"
+ " g();\n"
+ "}",
+ AllowsMergedIf);
+ verifyFormat("if (a) {\n"
+ " g();\n"
+ "} else if (b) {\n"
+ " g();\n"
+ "} else {\n"
+ " g();\n"
+ "}",
+ AllowsMergedIf);
+
+ AllowsMergedIf.AllowShortIfStatementsOnASingleLine =
+ FormatStyle::SIS_AllIfsAndElse;
verifyFormat("if (a) f();\n"
"else {\n"
@@ -545,6 +820,65 @@ TEST_F(FormatTest, FormatIfWithoutCompoundStatementButElseWith) {
" g();\n"
"}",
AllowsMergedIf);
+
+ verifyFormat("if (a) g();", AllowsMergedIf);
+ verifyFormat("if (a) {\n"
+ " g()\n"
+ "};",
+ AllowsMergedIf);
+ verifyFormat("if (a) g();\n"
+ "else g();",
+ AllowsMergedIf);
+ verifyFormat("if (a) {\n"
+ " g();\n"
+ "} else g();",
+ AllowsMergedIf);
+ verifyFormat("if (a) g();\n"
+ "else {\n"
+ " g();\n"
+ "}",
+ AllowsMergedIf);
+ verifyFormat("if (a) {\n"
+ " g();\n"
+ "} else {\n"
+ " g();\n"
+ "}",
+ AllowsMergedIf);
+ verifyFormat("if (a) g();\n"
+ "else if (b) g();\n"
+ "else g();",
+ AllowsMergedIf);
+ verifyFormat("if (a) {\n"
+ " g();\n"
+ "} else if (b) g();\n"
+ "else g();",
+ AllowsMergedIf);
+ verifyFormat("if (a) g();\n"
+ "else if (b) {\n"
+ " g();\n"
+ "} else g();",
+ AllowsMergedIf);
+ verifyFormat("if (a) g();\n"
+ "else if (b) g();\n"
+ "else {\n"
+ " g();\n"
+ "}",
+ AllowsMergedIf);
+ verifyFormat("if (a) g();\n"
+ "else if (b) {\n"
+ " g();\n"
+ "} else {\n"
+ " g();\n"
+ "}",
+ AllowsMergedIf);
+ verifyFormat("if (a) {\n"
+ " g();\n"
+ "} else if (b) {\n"
+ " g();\n"
+ "} else {\n"
+ " g();\n"
+ "}",
+ AllowsMergedIf);
}
TEST_F(FormatTest, FormatLoopsWithoutCompoundStatement) {
@@ -15814,7 +16148,7 @@ TEST_F(FormatTest, WhitesmithsBraceBreaking) {
FormatStyle BreakBeforeBraceShortIfs = WhitesmithsBraceStyle;
BreakBeforeBraceShortIfs.AllowShortIfStatementsOnASingleLine =
- FormatStyle::SIS_Always;
+ FormatStyle::SIS_OnlyFirstIf;
BreakBeforeBraceShortIfs.AllowShortLoopsOnASingleLine = true;
verifyFormat("void f(bool b)\n"
" {\n"
@@ -16694,14 +17028,21 @@ TEST_F(FormatTest, ParsesConfiguration) {
CHECK_PARSE("NamespaceIndentation: All", NamespaceIndentation,
FormatStyle::NI_All);
- Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Always;
+ Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_OnlyFirstIf;
CHECK_PARSE("AllowShortIfStatementsOnASingleLine: Never",
AllowShortIfStatementsOnASingleLine, FormatStyle::SIS_Never);
CHECK_PARSE("AllowShortIfStatementsOnASingleLine: WithoutElse",
AllowShortIfStatementsOnASingleLine,
FormatStyle::SIS_WithoutElse);
+ CHECK_PARSE("AllowShortIfStatementsOnASingleLine: OnlyFirstIf",
+ AllowShortIfStatementsOnASingleLine,
+ FormatStyle::SIS_OnlyFirstIf);
+ CHECK_PARSE("AllowShortIfStatementsOnASingleLine: AllIfsAndElse",
+ AllowShortIfStatementsOnASingleLine,
+ FormatStyle::SIS_AllIfsAndElse);
CHECK_PARSE("AllowShortIfStatementsOnASingleLine: Always",
- AllowShortIfStatementsOnASingleLine, FormatStyle::SIS_Always);
+ AllowShortIfStatementsOnASingleLine,
+ FormatStyle::SIS_OnlyFirstIf);
CHECK_PARSE("AllowShortIfStatementsOnASingleLine: false",
AllowShortIfStatementsOnASingleLine, FormatStyle::SIS_Never);
CHECK_PARSE("AllowShortIfStatementsOnASingleLine: true",
More information about the cfe-commits
mailing list