[PATCH] D37140: [clang-format] Fixed one-line if statement

[Pawel Maciocha via Phabricator via cfe-commits]

PriMee added a comment.

Sorry for wrong formatting before. Some inline comments added.



================
Comment at: lib/Format/UnwrappedLineFormatter.cpp:286
     }
+    if (TheLine->Last->is(tok::l_brace) &&
+        TheLine->First != TheLine->Last &&
----------------
krasimir wrote:
> No tests fail if this `if` statement gets removed. Please either remove it or add a test for it.
That's true. Why is that? Without this if statement tests would pass because our block would have been handled by the construct I have mentioned in problem description (Line 311). Adding a very similar test case with BraceWrapping.AfterFunction flag changed would show that we need this if statement. To my mind, it doesn't make sense to add such test case.


================
Comment at: lib/Format/UnwrappedLineFormatter.cpp:300
+      return Style.AllowShortBlocksOnASingleLine ? tryMergeSimpleBlock(I-1, E, Limit) : 0;
+    }
     if (TheLine->Last->is(tok::l_brace)) {
----------------
krasimir wrote:
> krasimir wrote:
> > Please reformat the newly added code with `clang-format`.
> Why in this case we use `Style.AllowShortBlocksOnASingleLine` and in the case above we use `AfterControlStatement`?
> Also, why `tryMergeSimpleBlock` instead of `tryMergeControlStatement`?
We use `AfterControlStatement` because we want to ensure that the block with the opening brace in separate line is correct. The flag implies this fact. 
We use `tryMergeControlStatement` only when our control statement is not within braces, `tryMergeSimpleBlock` in every other case. 




================
Comment at: unittests/Format/FormatTest.cpp:537
+               "}",
+               AllowSimpleBracedStatements);
+
----------------
krasimir wrote:
> Why is this example not on a single line? Is it because it has an `else`?
Exactly. Shouldn't this work like that? There is a comment:
`// Don't merge "if (a) { .. } else {".` on line 548 in UnwrappedLineFormatter.cpp



https://reviews.llvm.org/D37140