summaryrefslogtreecommitdiffstats
path: root/unittests/Format/FormatTest.cpp
diff options
context:
space:
mode:
authorKrasimir Georgiev <krasimir@google.com>2017-09-11 10:12:16 +0000
committerKrasimir Georgiev <krasimir@google.com>2017-09-11 10:12:16 +0000
commit889a43bfc96b5a0c3004d6e5a90164440622c0d8 (patch)
tree99d97d5a36f7c5dc3bb69b5fe10c55adc0f8f14c /unittests/Format/FormatTest.cpp
parent9b80cb1f6c92a08f352286c3360ca49594e598da (diff)
[clang-format] Fixed one-line if statement
Summary: **Short overview:** Fixed bug: https://bugs.llvm.org/show_bug.cgi?id=34001 Clang-format bug resulting in a strange behavior of control statements short blocks. Different flags combinations do not guarantee expected result. Turned on option AllowShortBlocksOnASingleLine does not work as intended. **Description of the problem:** Cpp source file UnwrappedLineFormatter does not handle AllowShortBlocksOnASingleLine flag as it should. Putting a single-line control statement without any braces, clang-format works as expected (depending on AllowShortIfStatementOnASingleLine or AllowShortLoopsOnASingleLine value). Putting a single-line control statement in braces, we can observe strange and incorrect behavior. Our short block is intercepted by tryFitMultipleLinesInOne function. The function returns a number of lines to be merged. Unfortunately, our control statement block is not covered properly. There are several if-return statements, but none of them handles our block. A block is identified by the line first token and by left and right braces. A function block works as expected, there is such an if-return statement doing proper job. A control statement block, from the other hand, falls into strange conditional construct, which depends on BraceWrapping.AfterFunction flag (with condition that the line’s last token is left brace, what is possible in our case) or goes even further. That should definitely not happen. **Description of the patch:** By adding three different if statements, we guarantee that our short control statement block, however it looks like (different brace wrapping flags may be turned on), is handled properly and does not fall into wrong conditional construct. Depending on appropriate options we return either 0 (when something disturbs our merging attempt) or let another function (tryMergeSimpleBlock) take the responsibility of returned result (number of merged lines). Nevertheless, one more correction is required in mentioned tryMergeSimpleBlock function. The function, previously, returned either 0 or 2. The problem was that this did not handle the case when our block had the left brace in a separate line, not the header one. After change, after adding condition, we return the result compatible with block’s structure. In case of left brace in the header’s line we do everything as before the patch. In case of left brace in a separate line we do the job similar to the one we do in case of a “non-header left brace” function short block. To be precise, we try to merge the block ignoring the header line. Then, if success, we increment our returned result. **After fix:** **CONFIG:** ``` AllowShortBlocksOnASingleLine: true AllowShortIfStatementsOnASingleLine: true BreakBeforeBraces: Custom BraceWrapping: { AfterClass: true, AfterControlStatement: true, AfterEnum: true, AfterFunction: true, AfterNamespace: false, AfterStruct: true, AfterUnion: true, BeforeCatch: true, BeforeElse: true } ``` **BEFORE:** ``` if (statement) doSomething(); if (statement) { doSomething(); } if (statement) { doSomething(); } if (statement) { doSomething(); } if (statement) doSomething(); if (statement) { doSomething1(); doSomething2(); } ``` **AFTER:** ``` if (statement) doSomething(); if (statement) { doSomething(); } if (statement) { doSomething(); } if (statement) { doSomething(); } if (statement) doSomething(); if (statement) { doSomething1(); doSomething2(); } ``` Contributed by @PriMee! Reviewers: krasimir, djasper Reviewed By: krasimir Subscribers: cfe-commits, klimek Differential Revision: https://reviews.llvm.org/D37140 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@312904 91177308-0d34-0410-b5e6-96231b3b80d8
Diffstat (limited to 'unittests/Format/FormatTest.cpp')
-rw-r--r--unittests/Format/FormatTest.cpp79
1 files changed, 79 insertions, 0 deletions
diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index 9acd7841d1..c7685c6cfc 100644
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -439,11 +439,16 @@ TEST_F(FormatTest, FormatLoopsWithoutCompoundStatement) {
TEST_F(FormatTest, FormatShortBracedStatements) {
FormatStyle AllowSimpleBracedStatements = getLLVMStyle();
+ AllowSimpleBracedStatements.ColumnLimit = 40;
AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = true;
AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = true;
AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;
+ AllowSimpleBracedStatements.BreakBeforeBraces = FormatStyle::BS_Custom;
+ AllowSimpleBracedStatements.BraceWrapping.AfterFunction = true;
+ AllowSimpleBracedStatements.BraceWrapping.SplitEmptyRecord = false;
+
verifyFormat("if (true) {}", AllowSimpleBracedStatements);
verifyFormat("if constexpr (true) {}", AllowSimpleBracedStatements);
verifyFormat("while (true) {}", AllowSimpleBracedStatements);
@@ -452,6 +457,10 @@ TEST_F(FormatTest, FormatShortBracedStatements) {
verifyFormat("if constexpr (true) { f(); }", AllowSimpleBracedStatements);
verifyFormat("while (true) { f(); }", AllowSimpleBracedStatements);
verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
+ verifyFormat("if (true) {\n"
+ " ffffffffffffffffffffffffffffffffffffffffffffffffffffff();\n"
+ "}",
+ AllowSimpleBracedStatements);
verifyFormat("if (true) { //\n"
" f();\n"
"}",
@@ -482,6 +491,7 @@ TEST_F(FormatTest, FormatShortBracedStatements) {
AllowSimpleBracedStatements);
AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = false;
+ verifyFormat("if (true) {}", AllowSimpleBracedStatements);
verifyFormat("if (true) {\n"
" f();\n"
"}",
@@ -494,14 +504,83 @@ TEST_F(FormatTest, FormatShortBracedStatements) {
AllowSimpleBracedStatements);
AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = false;
+ verifyFormat("while (true) {}", AllowSimpleBracedStatements);
verifyFormat("while (true) {\n"
" f();\n"
"}",
AllowSimpleBracedStatements);
+ verifyFormat("for (;;) {}", AllowSimpleBracedStatements);
verifyFormat("for (;;) {\n"
" f();\n"
"}",
AllowSimpleBracedStatements);
+
+ AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = true;
+ AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;
+ AllowSimpleBracedStatements.BraceWrapping.AfterControlStatement = true;
+
+ verifyFormat("if (true) {}", AllowSimpleBracedStatements);
+ verifyFormat("if constexpr (true) {}", AllowSimpleBracedStatements);
+ verifyFormat("while (true) {}", AllowSimpleBracedStatements);
+ verifyFormat("for (;;) {}", AllowSimpleBracedStatements);
+ verifyFormat("if (true) { f(); }", AllowSimpleBracedStatements);
+ verifyFormat("if constexpr (true) { f(); }", AllowSimpleBracedStatements);
+ verifyFormat("while (true) { f(); }", AllowSimpleBracedStatements);
+ verifyFormat("for (;;) { f(); }", AllowSimpleBracedStatements);
+ verifyFormat("if (true)\n"
+ "{\n"
+ " ffffffffffffffffffffffffffffffffffffffffffffffffffffff();\n"
+ "}",
+ AllowSimpleBracedStatements);
+ verifyFormat("if (true)\n"
+ "{ //\n"
+ " f();\n"
+ "}",
+ AllowSimpleBracedStatements);
+ verifyFormat("if (true)\n"
+ "{\n"
+ " f();\n"
+ " f();\n"
+ "}",
+ AllowSimpleBracedStatements);
+ verifyFormat("if (true)\n"
+ "{\n"
+ " f();\n"
+ "} else\n"
+ "{\n"
+ " f();\n"
+ "}",
+ AllowSimpleBracedStatements);
+
+ AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine = false;
+ verifyFormat("if (true) {}", AllowSimpleBracedStatements);
+ verifyFormat("if (true)\n"
+ "{\n"
+ " f();\n"
+ "}",
+ AllowSimpleBracedStatements);
+ verifyFormat("if (true)\n"
+ "{\n"
+ " f();\n"
+ "} else\n"
+ "{\n"
+ " f();\n"
+ "}",
+ AllowSimpleBracedStatements);
+
+ AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = false;
+ verifyFormat("while (true) {}", AllowSimpleBracedStatements);
+ verifyFormat("while (true)\n"
+ "{\n"
+ " f();\n"
+ "}",
+ AllowSimpleBracedStatements);
+ verifyFormat("for (;;) {}", AllowSimpleBracedStatements);
+ verifyFormat("for (;;)\n"
+ "{\n"
+ " f();\n"
+ "}",
+ AllowSimpleBracedStatements);
}
TEST_F(FormatTest, ParseIfElse) {