Clang-format bug patch

[Maciocha, Pawel (Nokia - PL/Wroclaw) via cfe-commits]

To Whom It May Concern,



I am writing in reference to a patch I created in response to the bug I have decided to work on.

Bug comes from the official site:

https://bugs.llvm.org/show_bug.cgi?id=34001



Short overview:



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.



Please find the attached diff files. There are two. One created by simple svn diff command and one with postscript "NoWhitespaces" which ignores every whitespace while creating patch file. Has been done for more readable format.



I would be very grateful if that could go under review. Thank you.



Best regards,



Pawel Maciocha
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170821/ee2b7dc7/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: UnwrappedLineFormatter.patch
Type: application/octet-stream
Size: 5710 bytes
Desc: UnwrappedLineFormatter.patch
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170821/ee2b7dc7/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: UnwrappedLineFormatterNoWhitespaces.patch
Type: application/octet-stream
Size: 2447 bytes
Desc: UnwrappedLineFormatterNoWhitespaces.patch
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170821/ee2b7dc7/attachment-0003.obj>