]> granicus.if.org Git - clang/commit
[clang-format] Fixed one-line if statement
authorKrasimir Georgiev <krasimir@google.com>
Mon, 11 Sep 2017 10:12:16 +0000 (10:12 +0000)
committerKrasimir Georgiev <krasimir@google.com>
Mon, 11 Sep 2017 10:12:16 +0000 (10:12 +0000)
commit889a43bfc96b5a0c3004d6e5a90164440622c0d8
tree99d97d5a36f7c5dc3bb69b5fe10c55adc0f8f14c
parent9b80cb1f6c92a08f352286c3360ca49594e598da
[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
lib/Format/UnwrappedLineFormatter.cpp
unittests/Format/FormatTest.cpp