[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D59309#1428807 <https://reviews.llvm.org/D59309#1428807>, @MyDeveloperDay wrote:

> In D59309#1428799 <https://reviews.llvm.org/D59309#1428799>, @klimek wrote:
>
> > Why did the numeric constant break this? Which path would the function run through?
>
>
> as its looking though the parameter list is doesn't recognize a numeric constant as being a valid thing it would see in an argument list and so falls through the bottom to return false, it will only be where there is a numeric_constant as the first argument, if it has a second argument and that argument is just a simple type it would still fail, but switch them around and it won't
>
> As I look more at this, even this would (and still would) fail to break
>
>   int TestFn(A=1)
>   {
>     return a;
>   }
>
>
> originally I didn't have the counting of the <> but one of the unit tests failed where we had
>
>   verifyFormat("LoooooooooooooooooooooooooooooooooooooooongType\n"
>                "    LoooooooooooooooooooooooooooooooooooooooongVariable(1);");
>   
>   
>
> I guess they wanted to do something like this when the type and name is very long
>
> int
>  variable(1);
>
> but I think this gets thought of as being a potential FunctionDecl
>
> so I held off just adding tok::numeric_constant to the isOneOf(), and went with the use case
>
> I think perhaps I could solve the default argument by adding a
>
> startsSequence(tok::equal, tok::numeric_constant)


How does this even work for

  void Test(A a)
  {
  }

This code seems to deeply assume that the { is the last thing in the line for a definition. One question is whether we can just mark this as a function definition when we see the { in the UnwrappedLineParser...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59309/new/

https://reviews.llvm.org/D59309