Break after return type ignored with certain template parameters

[llvmbot]

Bugzilla Link	40696
Resolution	FIXED
Resolved on	Apr 06, 2019 05:52
Version	8.0
OS	Linux
Reporter	LLVM Bugzilla Contributor
CC	@mydeveloperday
Fixed by commit(s)	rL357837

Extended Description

The break after return type style doesn't seem to work if the function's first parameter is a template with an integer template-parameter.

Given the source: int TestFn(A<8> a) { return a; }

When run with the Mozilla style, clang-format reformats the code as:

int TestFn(A<8> a) { return a; }

Note the return type is on the same line as the function name. If the function signature is changed such that the first parameter does not have an integer template parameter, it behaves as expected. For example, changing <8> to formats the code with the return type on its own line.

int TestFn(A a) { return a; }

This is using clang-format version 8.0.0 (trunk 345971):

[mydeveloperday]

mentioned in issue llvm/llvm-bugzilla-archive#41170

[mydeveloperday]

WIP https://reviews.llvm.org/D59309

[mydeveloperday]

Running clang-format with the --debug option shows

The first function is not seen a a function declaration unlike the second

int TestFn(A<8> a)

= AnnotatedTokens(L=0): M=0 C=0 T=Unknown S=1 B=0 BK=0 P=0 Name=int L=3 PPK=2 FakeLParens= FakeRParens=0 II=0x1ee40292450 Text='int' M=0 C=1 T=StartOfName S=1 B=0 BK=0 P=220 Name=identifier L=10 PPK=2 FakeLParens= FakeRParens=0 II=0x1ee402d7640 Text='TestFn' M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=l_paren L=11 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='(' M=0 C=1 T=Unknown S=0 B=0 BK=0 P=140 Name=identifier L=12 PPK=2 FakeLParens= FakeRParens=0 II=0x1ee402d7670 Text='A' M=0 C=0 T=TemplateOpener S=0 B=0 BK=0 P=50 Name=less L=13 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='<' M=0 C=1 T=Unknown S=0 B=0 BK=0 P=380 Name=numeric_constant L=14 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='8' M=0 C=0 T=TemplateCloser S=0 B=0 BK=0 P=290 Name=greater L=15 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='>' M=0 C=1 T=StartOfName S=1 B=0 BK=0 P=240 Name=identifier L=17 PPK=2 FakeLParens= FakeRParens=0 II=0x1ee402d76a0 Text='a' M=0 C=0 T=Unknown S=0 B=0 BK=0 P=43 Name=r_paren L=18 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=')'

int TestFn(A a)

AnnotatedTokens(L=0): M=0 C=0 T=Unknown S=1 B=0 BK=0 P=0 Name=int L=3 PPK=2 FakeLParens= FakeRParens=0 II=0x1ee40292450 Text='int' M=1 C=1 T=FunctionDeclarationName S=1 B=0 BK=0 P=220 Name=identifier L=83 PPK=2 FakeLParens= FakeRParens=0 II=0x1ee402d7640 Text='TestFn' M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=l_paren L=84 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='(' M=0 C=1 T=Unknown S=0 B=0 BK=0 P=140 Name=identifier L=85 PPK=2 FakeLParens= FakeRParens=0 II=0x1ee402d7670 Text='A' M=0 C=0 T=TemplateOpener S=0 B=0 BK=0 P=50 Name=less L=86 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='<' M=0 C=1 T=Unknown S=0 B=0 BK=0 P=380 Name=bool L=90 PPK=2 FakeLParens= FakeRParens=0 II=0x1ee40292a30 Text='bool' M=0 C=0 T=TemplateCloser S=0 B=0 BK=0 P=290 Name=greater L=91 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='>' M=0 C=1 T=StartOfName S=1 B=0 BK=0 P=240 Name=identifier L=93 PPK=2 FakeLParens= FakeRParens=0 II=0x1ee402d76a0 Text='a' M=0 C=0 T=Unknown S=0 B=0 BK=0 P=43 Name=r_paren L=94 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=')'

Note the T=StartOfName vs T=FunctionDeclarationName

The rule for must breaking before return type is this

if (!Current->MustBreakBefore && InFunctionDecl &&
    Current->is(TT_FunctionDeclarationName))
  Current->MustBreakBefore = mustBreakForReturnType(Line);

As the first function is not a TT_FunctionDeclarationName it won't break

The fix is in the isFunctionDeclarationName call

static bool isFunctionDeclarationName(const FormatToken &Current, const AnnotatedLine &Line) ...

it looks over the tokens of the parameter list, it wasn't considering the tok::numberic_constant as something that can be seen in an argument list

 if (Tok->is(tok::kw_const) || Tok->isSimpleTypeSpecifier() ||

• Tok->isOneOf(TT_PointerOrReference, TT_StartOfName, tok::ellipsis))
• Tok->isOneOf(TT_PointerOrReference, TT_StartOfName,
• tok::numeric_constant, tok::ellipsis)) return true;

Adding that resolves this issue

int TestFn(A<8> a) { return a; }

int TestFn(A a) { return a; }