clang-format removes space between 'override' and curly brace when there's a trailing semicolon

[zmodem]

Consider:

$ echo 'struct S { void f() override {}; };' | clang-format -style=chromium
struct S {
  void f() override{};
};

Note the missing space between override and {.

This only seems to happen when there's a trailing semicolon --- which is of course unnecessary and -Wextra-semi warns about it --- but it would be nice if clang-format could handle it.

[llvmbot]

@llvm/issue-subscribers-clang-format

[rymiel]

Issue is around here:

llvm-project/clang/lib/Format/UnwrappedLineParser.cpp

Lines 711 to 713 in 8856137

	ProbablyBracedList = ProbablyBracedList ||
	(NextTok->is(tok::semi) &&
	(!ExpectClassBody || LBraceStack.size() != 1));

It is infact doing what the comment says, but I guess it needs extra context that it's parsing a declaration

FWIW, all that's needed to reproduce this is void f() {};, which gets formatted into void f(){};. Putting any statement in the curly braces correctly makes the braces act as FunctionLBrace

Also, a trailing annotation such as noexcept manages to pull the parens and braces apart *somehow*, but the braces are still mis-annotated

[mydeveloperday]

Oh! definitely related

void a() {}
void a() {}
void a() { return a; }
void a() { return a; }
void a() override {}
void a() override{};

[mydeveloperday]

Very much seems to come down to what tryToParseBracedList() thinks...

[mydeveloperday]

Perhaps we need another pass, after the assignment of TT_FunctionDeclarationName, where we go back over and set the TT_FunctionLBrace, and find the unnecessary ; after the empty {}.

I think this could fix the spacing and mark the ; optional so they get removed later

@owenca any thoughts?

TokenAnnotator Annotator(Style, Keywords);
 Annotator.annotate(Line);
 Annotator.calculateFormattingInformation(Line);

 Annotator.enusreFunctionLBrace();
 ^^^^^

[mydeveloperday]

More test cases

void a() {}
void a(){};
void a() { return a; }
void a() { return a; }
void a() override {}
void a() override{};
void a() noexcept {}
void a() noexcept(false){};
auto a() -> void {}
auto a() -> void{};

[owenca]

Perhaps we need another pass, after the assignment of TT_FunctionDeclarationName, where we go back over and set the TT_FunctionLBrace, and find the unnecessary ; after the empty {}.

I think this could fix the spacing and mark the ; optional so they get removed later

@owenca any thoughts?

It might work, but I don't think it's worth fixing, especially at the cost of adding another pass.

BTW I succeeded in fixing all of the above test cases (except void a() noexcept(false){};) and passing FormatTests (including the empty function test cases for RemoveSemicolon) without calling the annotator in the parser. However, I abandoned the work because it didn't cover function definitions with a trailing return type, etc.