[PATCH] Fix bug in clang-format while merging short function (PR19461)

[Manuel Klimek]

On Mon, Apr 21, 2014 at 3:33 PM, Dinesh Dwivedi <dinesh.d at samsung.com>wrote:

> Hi djasper, chapuni, klimek,
>
> if Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All,
> clang-format tries to join short functions and prints in single line.
> This code was not taking care of cases where there is something in between
> function header and body e.g. attached test cases where
> there are preprocessor directives in between function header and body.
>
> Attached patch fixes this issue by checking if we have both header and
> body and nothing between them while trying to merge functions.
>
> Please let me know if this is ok.
>
> TEST PLAN
>   Added testcases for different BreakBeforeBraces styles.
>
> http://reviews.llvm.org/D3439
>
> Files:
>   lib/Format/Format.cpp
>   unittests/Format/FormatTest.cpp
>
> Index: lib/Format/Format.cpp
> ===================================================================
> --- lib/Format/Format.cpp
> +++ lib/Format/Format.cpp
> @@ -559,7 +559,7 @@
>        Limit -= 2;
>
>        unsigned MergedLines = 0;
> -      if (MergeShortFunctions) {
> +      if (MergeShortFunctions && TheLine->MustBeDeclaration) {
>

I don't think this is the right fix; I think we'll need to figure out
whether there is a "gap" in the line, and in that case not merge it.


>          MergedLines = tryMergeSimpleBlock(I + 1, E, Limit);
>          // If we managed to merge the block, count the function header,
> which is
>          // on a separate line.
> Index: unittests/Format/FormatTest.cpp
> ===================================================================
> --- unittests/Format/FormatTest.cpp
> +++ unittests/Format/FormatTest.cpp
> @@ -7336,6 +7336,26 @@
>                 "}\n"
>                 "}",
>                 BreakBeforeBrace);
> +
> +  verifyFormat("#ifdef _DEBUG\n"
> +               "int foo(int i = 0)\n"
> +               "#else\n"
> +               "int foo(int i = 5)\n"
> +               "#endif\n"
> +               "{\n"
> +               "  return i;\n"
> +               "}\n"
> +               "void foo1()\n"
> +               "#ifdef _DEBUG\n"
> +               "{\n"
> +               "  return 0;\n"
> +               "}\n"
> +               "#else\n"
> +               "{\n"
> +               "  return 5;\n"
> +               "}\n"
> +               "#endif",
> +               BreakBeforeBrace);
>  }
>
>  TEST_F(FormatTest, AllmanBraceBreaking) {
> @@ -7418,6 +7438,26 @@
>                 "}\n",
>                 BreakBeforeBrace);
>
> +  verifyFormat("#ifdef _DEBUG\n"
> +               "int foo(int i = 0)\n"
> +               "#else\n"
> +               "int foo(int i = 5)\n"
> +               "#endif\n"
> +               "{\n"
> +               "  return i;\n"
> +               "}\n"
> +               "void foo1()\n"
> +               "#ifdef _DEBUG\n"
> +               "{\n"
> +               "  return 0;\n"
> +               "}\n"
> +               "#else\n"
> +               "{\n"
> +               "  return 5;\n"
> +               "}\n"
> +               "#endif",
> +               BreakBeforeBrace);
> +
>    BreakBeforeBrace.ColumnLimit = 19;
>    verifyFormat("void f() { int i; }", BreakBeforeBrace);
>    BreakBeforeBrace.ColumnLimit = 18;
> @@ -7538,6 +7578,26 @@
>                 "  Y = 0,\n"
>                 "}\n",
>                 GNUBraceStyle);
> +
> +  verifyFormat("#ifdef _DEBUG\n"
> +               "int foo(int i = 0)\n"
> +               "#else\n"
> +               "int foo(int i = 5)\n"
> +               "#endif\n"
> +               "{\n"
> +               "  return i;\n"
> +               "}\n"
> +               "void foo1()\n"
> +               "#ifdef _DEBUG\n"
> +               "{\n"
> +               "  return 0;\n"
> +               "}\n"
> +               "#else\n"
> +               "{\n"
> +               "  return 5;\n"
> +               "}\n"
> +               "#endif",
> +               GNUBraceStyle);
>  }
>  TEST_F(FormatTest, CatchExceptionReferenceBinding) {
>    verifyFormat("void f() {\n"
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140424/5ecbbde7/attachment.html>