replica
/
hanchenye-llvm-project
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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

Summary:
Addressing: PR25010 - https://bugs.llvm.org/show_bug.cgi?id=25010

Code like:

```
    if(true) var++;
    else  {
        var--;
    }
```

is reformatted to be

```
  if (true)
    var++;
  else {
    var--;
  }
```

Even when `AllowShortIfStatementsOnASingleLine` is true

The following revision comes from a +1'd suggestion in the PR to support AllowShortIfElseStatementsOnASingleLine

This suppresses the clause prevents the merging of the if when there is a compound else

Reviewers: klimek, djasper, JonasToth, alexfh, krasimir, reuk
Reviewed By: reuk
Subscribers: reuk, Higuoxing, jdoerfert, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D59087

llvm-svn: 356029
This commit is contained in:
Paul Hoad 2019-03-13 08:07:46 +00:00
parent 4a725996e5
commit 6d294f28e9
9 changed files with 171 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
View File

@ -9,7 +9,7 @@ This check will try to enforce coding guidelines on the identifiers naming. It
supports one of the following casing types and tries to convert from one to
another if a mismatch is detected
Casing types inclde:
Casing types include:
- ``lower_case``,
- ``UPPER_CASE``,

2
clang-tools-extra/docs/conf.py
View File

@ -26,7 +26,7 @@ from datetime import date
# Add any Sphinx extension module names here, as strings. They can be extensions
# coming with Sphinx (named 'sphinx.ext.*') or your custom ones.
extensions = ['sphinx.ext.todo', 'sphinx.ext.mathjax']
extensions = ['sphinx.ext.todo', 'sphinx.ext.mathjax', 'sphinxcontrib.spelling' ]
# Add any paths that contain templates here, relative to this directory.
templates_path = ['_templates']

41
clang/docs/ClangFormatStyleOptions.rst
View File

@ -365,10 +365,47 @@ 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:
**AllowShortIfStatementsOnASingleLine** (``bool``)
If ``true``, ``if (a) return;`` can be put on a single line.
* ``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;
}
**AllowShortLoopsOnASingleLine** (``bool``)
If ``true``, ``while (true) continue;`` can be put on a single

32
clang/include/clang/Format/Format.h
View File

@ -241,8 +241,38 @@ 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.
bool AllowShortIfStatementsOnASingleLine;
ShortIfStyle AllowShortIfStatementsOnASingleLine;
/// If ``true``, ``while (true) continue;`` can be put on a single
/// line.

26
clang/lib/Format/Format.cpp
View File

@ -106,6 +106,18 @@ template <> struct ScalarEnumerationTraits<FormatStyle::ShortFunctionStyle> {
}
};
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);
@ -631,7 +643,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
LLVMStyle.AllowShortBlocksOnASingleLine = false;
LLVMStyle.AllowShortCaseLabelsOnASingleLine = false;
LLVMStyle.AllowShortIfStatementsOnASingleLine = false;
LLVMStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
LLVMStyle.AllowShortLoopsOnASingleLine = false;
LLVMStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
@ -737,7 +749,8 @@ FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
GoogleStyle.AccessModifierOffset = -1;
GoogleStyle.AlignEscapedNewlines = FormatStyle::ENAS_Left;
GoogleStyle.AllowShortIfStatementsOnASingleLine = true;
GoogleStyle.AllowShortIfStatementsOnASingleLine =
FormatStyle::SIS_WithoutElse;
GoogleStyle.AllowShortLoopsOnASingleLine = true;
GoogleStyle.AlwaysBreakBeforeMultilineStrings = true;
GoogleStyle.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
@ -804,7 +817,7 @@ FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
GoogleStyle.AlignOperands = false;
GoogleStyle.AlignTrailingComments = false;
GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
GoogleStyle.AllowShortIfStatementsOnASingleLine = false;
GoogleStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
GoogleStyle.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment;
GoogleStyle.ColumnLimit = 100;
@ -846,7 +859,8 @@ FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
FormatStyle getChromiumStyle(FormatStyle::LanguageKind Language) {
FormatStyle ChromiumStyle = getGoogleStyle(Language);
if (Language == FormatStyle::LK_Java) {
ChromiumStyle.AllowShortIfStatementsOnASingleLine = true;
ChromiumStyle.AllowShortIfStatementsOnASingleLine =
FormatStyle::SIS_WithoutElse;
ChromiumStyle.BreakAfterJavaFieldAnnotations = true;
ChromiumStyle.ContinuationIndentWidth = 8;
ChromiumStyle.IndentWidth = 4;
@ -859,12 +873,12 @@ FormatStyle getChromiumStyle(FormatStyle::LanguageKind Language) {
};
ChromiumStyle.SortIncludes = true;
} else if (Language == FormatStyle::LK_JavaScript) {
ChromiumStyle.AllowShortIfStatementsOnASingleLine = false;
ChromiumStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
ChromiumStyle.AllowShortLoopsOnASingleLine = false;
} else {
ChromiumStyle.AllowAllParametersOfDeclarationOnNextLine = false;
ChromiumStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
ChromiumStyle.AllowShortIfStatementsOnASingleLine = false;
ChromiumStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
ChromiumStyle.AllowShortLoopsOnASingleLine = false;
ChromiumStyle.BinPackParameters = false;
ChromiumStyle.DerivePointerAlignment = false;

4
clang/lib/Format/UnwrappedLineFormatter.cpp
View File

@ -413,10 +413,12 @@ 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).
// 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;
}
return 1;
}

75
clang/unittests/Format/FormatTest.cpp
View File

@ -439,7 +439,8 @@ TEST_F(FormatTest, FormatIfWithoutCompoundStatement) {
FormatStyle AllowsMergedIf = getLLVMStyle();
AllowsMergedIf.AlignEscapedNewlines = FormatStyle::ENAS_Left;
AllowsMergedIf.AllowShortIfStatementsOnASingleLine = true;
AllowsMergedIf.AllowShortIfStatementsOnASingleLine =
FormatStyle::SIS_WithoutElse;
verifyFormat("if (a)\n"
" // comment\n"
" f();",
@ -487,6 +488,41 @@ 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;
@ -515,7 +551,8 @@ TEST_F(FormatTest, FormatShortBracedStatements) {
AllowSimpleBracedStatements.ColumnLimit = 40;
AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = true;
AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = true;
AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine =
FormatStyle::SIS_WithoutElse;
AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;
AllowSimpleBracedStatements.BreakBeforeBraces = FormatStyle::BS_Custom;
@ -563,7 +600,8 @@ TEST_F(FormatTest, FormatShortBracedStatements) {
"};",
AllowSimpleBracedStatements);
AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = false;
AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine =
FormatStyle::SIS_Never;
verifyFormat("if (true) {}", AllowSimpleBracedStatements);
verifyFormat("if (true) {\n"
" f();\n"
@ -588,7 +626,8 @@ TEST_F(FormatTest, FormatShortBracedStatements) {
"}",
AllowSimpleBracedStatements);
AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = true;
AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine =
FormatStyle::SIS_WithoutElse;
AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;
AllowSimpleBracedStatements.BraceWrapping.AfterControlStatement = true;
@ -625,7 +664,8 @@ TEST_F(FormatTest, FormatShortBracedStatements) {
"}",
AllowSimpleBracedStatements);
AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = false;
AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine =
FormatStyle::SIS_Never;
verifyFormat("if (true) {}", AllowSimpleBracedStatements);
verifyFormat("if (true)\n"
"{\n"
@ -659,7 +699,7 @@ TEST_F(FormatTest, FormatShortBracedStatements) {
TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
FormatStyle Style = getLLVMStyleWithColumns(60);
Style.AllowShortBlocksOnASingleLine = true;
Style.AllowShortIfStatementsOnASingleLine = true;
Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
Style.BreakBeforeBraces = FormatStyle::BS_Allman;
EXPECT_EQ("#define A \\\n"
" if (HANDLEwernufrnuLwrmviferuvnierv) \\\n"
@ -3157,7 +3197,7 @@ TEST_F(FormatTest, GraciouslyHandleIncorrectPreprocessorConditions) {
TEST_F(FormatTest, FormatsJoinedLinesOnSubsequentRuns) {
FormatStyle SingleLine = getLLVMStyle();
SingleLine.AllowShortIfStatementsOnASingleLine = true;
SingleLine.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
verifyFormat("#if 0\n"
"#elif 1\n"
"#endif\n"
@ -8000,7 +8040,8 @@ TEST_F(FormatTest, FormatHashIfExpressions) {
TEST_F(FormatTest, MergeHandlingInTheFaceOfPreprocessorDirectives) {
FormatStyle AllowsMergedIf = getGoogleStyle();
AllowsMergedIf.AllowShortIfStatementsOnASingleLine = true;
AllowsMergedIf.AllowShortIfStatementsOnASingleLine =
FormatStyle::SIS_WithoutElse;
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);
@ -10425,7 +10466,8 @@ TEST_F(FormatTest, AllmanBraceBreaking) {
AllmanBraceStyle.ColumnLimit = 80;
FormatStyle BreakBeforeBraceShortIfs = AllmanBraceStyle;
BreakBeforeBraceShortIfs.AllowShortIfStatementsOnASingleLine = true;
BreakBeforeBraceShortIfs.AllowShortIfStatementsOnASingleLine =
FormatStyle::SIS_WithoutElse;
BreakBeforeBraceShortIfs.AllowShortLoopsOnASingleLine = true;
verifyFormat("void f(bool b)\n"
"{\n"
@ -10887,7 +10929,6 @@ 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);
@ -11150,6 +11191,20 @@ 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();

2
clang/unittests/Format/FormatTestSelective.cpp
View File

@ -98,7 +98,7 @@ TEST_F(FormatTestSelective, ReformatsMovedLines) {
}
TEST_F(FormatTestSelective, FormatsIfWithoutCompoundStatement) {
Style.AllowShortIfStatementsOnASingleLine = true;
Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
EXPECT_EQ("if (a) return;", format("if(a)\nreturn;", 7, 1));
EXPECT_EQ("if (a) return; // comment",
format("if(a)\nreturn; // comment", 20, 1));

14
llvm/utils/release/build_llvm_package.bat
View File

@ -42,13 +42,13 @@ mkdir %build_dir%
cd %build_dir%
echo Checking out %branch% at r%revision%...
svn.exe export -r %revision% http://llvm.org/svn/llvm-project/llvm/%branch% llvm || exit /b
svn.exe export -r %revision% http://llvm.org/svn/llvm-project/cfe/%branch% llvm/tools/clang || exit /b
svn.exe export -r %revision% http://llvm.org/svn/llvm-project/clang-tools-extra/%branch% llvm/tools/clang/tools/extra || exit /b
svn.exe export -r %revision% http://llvm.org/svn/llvm-project/lld/%branch% llvm/tools/lld || exit /b
svn.exe export -r %revision% http://llvm.org/svn/llvm-project/compiler-rt/%branch% llvm/projects/compiler-rt || exit /b
svn.exe export -r %revision% http://llvm.org/svn/llvm-project/openmp/%branch% llvm/projects/openmp || exit /b
svn.exe export -r %revision% http://llvm.org/svn/llvm-project/lldb/%branch% llvm/tools/lldb || exit /b
REM svn.exe export -r %revision% http://llvm.org/svn/llvm-project/llvm/%branch% llvm || exit /b
REM svn.exe export -r %revision% http://llvm.org/svn/llvm-project/cfe/%branch% llvm/tools/clang || exit /b
REM svn.exe export -r %revision% http://llvm.org/svn/llvm-project/clang-tools-extra/%branch% llvm/tools/clang/tools/extra || exit /b
REM svn.exe export -r %revision% http://llvm.org/svn/llvm-project/lld/%branch% llvm/tools/lld || exit /b
REM svn.exe export -r %revision% http://llvm.org/svn/llvm-project/compiler-rt/%branch% llvm/projects/compiler-rt || exit /b
REM svn.exe export -r %revision% http://llvm.org/svn/llvm-project/openmp/%branch% llvm/projects/openmp || exit /b
REM svn.exe export -r %revision% http://llvm.org/svn/llvm-project/lldb/%branch% llvm/tools/lldb || exit /b
REM Setting CMAKE_CL_SHOWINCLUDES_PREFIX to work around PR27226.