MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: djasper, klimek, reuk, JonasToth, alexfh,
krasimir, russellmcc.
MyDeveloperDay added a project: clang-tools-extra.
Herald added a subscriber: jdoerfert.
An addendum to D59087: [clang-format] [PR25010]
AllowShortIfStatementsOnASingleLine not working if an "else" statement is
present <https://reviews.llvm.org/D59087> following discussions with submitter
of https://bugs.llvm.org/show_bug.cgi?id=25010
it didn't make sense that, we could only do short if and not short else if and
short else if they didn't have compound statments
if (true) return 0;
if (true) return 0;
else return 1;
if (true) return 0;
else if (false) return 1;
if (true) return 0;
else if (false) return 1;
else return 2;
if (true) return 0;
else if (false) return 1;
else {
return 2;
}
The following revision extends the capability form D59087: [clang-format]
[PR25010] AllowShortIfStatementsOnASingleLine not working if an "else"
statement is present <https://reviews.llvm.org/D59087> to allow that if the
correct setting is provided
existing true/false still honor the original meaning of
AllowShortIfStatementsOnASingleLine
but the additional options now allow more fine grained control over how else/if
and else should/could be handled.
https://reviews.llvm.org/D59408
Files:
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/unittests/Format/FormatTest.cpp
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -505,6 +505,35 @@
" g();\n",
AllowsMergedIf);
+ AllowsMergedIf.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_AlwaysNoElse;
+
+ verifyFormat("if (a) f();\n"
+ "else {\n"
+ " g();\n"
+ "}",
+ AllowsMergedIf);
+ verifyFormat("if (a) f();\n"
+ "else\n"
+ " g();\n",
+ AllowsMergedIf);
+ verifyFormat("if (a) f();\n"
+ "else if (b) g();\n",
+ AllowsMergedIf);
+ verifyFormat("if (a) f();\n"
+ "else if (b) g();\n"
+ "else\n"
+ " h();\n",
+ AllowsMergedIf);
+ verifyFormat("if (a) f();\n"
+ "else {\n"
+ " if (a) f();\n"
+ " else {\n"
+ " g();\n"
+ " }\n"
+ " g();\n"
+ "}",
+ AllowsMergedIf);
+
AllowsMergedIf.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Always;
verifyFormat("if (a) f();\n"
@@ -521,6 +550,16 @@
" g();\n"
"}",
AllowsMergedIf);
+ verifyFormat("if (a) f();\n"
+ "else g();\n",
+ AllowsMergedIf);
+ verifyFormat("if (a) f();\n"
+ "else if (b) g();\n",
+ AllowsMergedIf);
+ verifyFormat("if (a) f();\n"
+ "else if (b) g();\n"
+ "else h();\n",
+ AllowsMergedIf);
}
TEST_F(FormatTest, FormatLoopsWithoutCompoundStatement) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -363,6 +363,18 @@
? tryMergeSimpleControlStatement(I, E, Limit)
: 0;
}
+ if (TheLine->First->startsSequence(tok::kw_else, tok::kw_if)) {
+ return (Style.AllowShortIfStatementsOnASingleLine >=
+ FormatStyle::SIS_AlwaysNoElse)
+ ? tryMergeSimpleControlStatement(I, E, Limit)
+ : 0;
+ }
+ if (TheLine->First->is(tok::kw_else)) {
+ return Style.AllowShortIfStatementsOnASingleLine ==
+ FormatStyle::SIS_Always
+ ? tryMergeSimpleControlStatement(I, E, Limit)
+ : 0;
+ }
if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) {
return Style.AllowShortLoopsOnASingleLine
? tryMergeSimpleControlStatement(I, E, Limit)
@@ -406,7 +418,8 @@
return 0;
Limit = limitConsideringMacros(I + 1, E, Limit);
AnnotatedLine &Line = **I;
- if (Line.Last->isNot(tok::r_paren))
+ if (Line.Last->isNot(tok::r_paren) &&
+ Style.AllowShortIfStatementsOnASingleLine < FormatStyle::SIS_Always)
return 0;
if (1 + I[1]->Last->TotalLength > Limit)
return 0;
@@ -414,7 +427,8 @@
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_AlwaysNoElse) {
if (I + 2 != E && Line.startsWith(tok::kw_if) &&
I[2]->First->is(tok::kw_else))
return 0;
@@ -487,7 +501,7 @@
// Check that the current line allows merging. This depends on whether we
// are in a control flow statements as well as several style flags.
- if (Line.First->isOneOf(tok::kw_else, tok::kw_case) ||
+ if (Line.First->is(tok::kw_case) ||
(Line.First->Next && Line.First->Next->is(tok::kw_else)))
return 0;
// default: in switch statement
@@ -496,9 +510,10 @@
if (Tok && Tok->is(tok::colon))
return 0;
}
- if (Line.First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_do, tok::kw_try,
- tok::kw___try, tok::kw_catch, tok::kw___finally,
- tok::kw_for, tok::r_brace, Keywords.kw___except)) {
+ if (Line.First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while, tok::kw_do,
+ tok::kw_try, tok::kw___try, tok::kw_catch,
+ tok::kw___finally, tok::kw_for, tok::r_brace,
+ Keywords.kw___except)) {
if (!Style.AllowShortBlocksOnASingleLine)
return 0;
// Don't merge when we can't except the case when
@@ -508,6 +523,11 @@
!Style.BraceWrapping.AfterControlStatement &&
!I[1]->First->is(tok::r_brace))
return 0;
+ if (!Style.AllowShortIfStatementsOnASingleLine &&
+ Line.startsWith(tok::kw_else) &&
+ !Style.BraceWrapping.AfterControlStatement &&
+ !I[1]->First->is(tok::r_brace))
+ return 0;
if (!Style.AllowShortIfStatementsOnASingleLine &&
Line.startsWith(tok::kw_if) &&
Style.BraceWrapping.AfterControlStatement && I + 2 != E &&
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -110,6 +110,7 @@
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, "AlwaysNoElse", FormatStyle::SIS_AlwaysNoElse);
IO.enumCase(Value, "WithoutElse", FormatStyle::SIS_WithoutElse);
// For backward compatibility.
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -260,13 +260,33 @@
/// return;
/// \endcode
SIS_WithoutElse,
- /// Always put short ifs on the same line if
- /// the else is not a compound statement or not.
+ /// If Else statements have no braces don't put them
+ /// on the same line.
+ /// \code
+ /// if (a) return;
+ /// else
+ /// return;
+ /// \endcode
+ SIS_AlwaysNoElse,
+ /// Always put short if/else/else if on the same line if
+ /// the not compound statement.
/// \code
/// if (a) return;
/// else {
/// return;
/// }
+ /// if (a) return;
+ /// else if (b) return;
+ /// else {
+ /// return;
+ /// }
+ ///
+ /// if (a) return;
+ /// else if (b) return;
+ /// else return;
+ ///
+ /// if (a) return;
+ /// else return;
/// \endcode
SIS_Always,
};
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -397,16 +397,43 @@
return;
}
+ * ``SIS_AlwaysNoElse`` (in configuration: ``AlwaysNoElse``)
+ Allow short if/else if statements even if the else is a compound statement.
+
+ .. code-block:: c++
+
+ if (a) return;
+ else {
+ return;
+ }
+
* ``SIS_Always`` (in configuration: ``Always``)
- Allow short if statements even if the else is a compound statement.
+ Allow short if/else if/else statements when they are not compound statements.
.. code-block:: c++
+ if (a) return;
+ else return;
+
+ if (a) return;
+ else if (b) return;
+ else return;
+
if (a) return;
else {
return;
}
+ if (a) return;
+ else if (b) return;
+ else {
+ return;
+ }
+
+ if (a) return;
+ else if (b) return;
+ else return;
+
**AllowShortLoopsOnASingleLine** (``bool``)
If ``true``, ``while (true) continue;`` can be put on a single
line.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
