Skip to content

[clang-format] Fix RemoveSemicolon for empty functions #82278

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions clang/lib/Format/Format.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2261,27 +2261,36 @@ class SemiRemover : public TokenAnalyzer {
FormatTokenLexer &Tokens) override {
AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
tooling::Replacements Result;
removeSemi(AnnotatedLines, Result);
removeSemi(Annotator, AnnotatedLines, Result);
return {Result, 0};
}

private:
void removeSemi(SmallVectorImpl<AnnotatedLine *> &Lines,
void removeSemi(TokenAnnotator &Annotator,
SmallVectorImpl<AnnotatedLine *> &Lines,
tooling::Replacements &Result) {
auto PrecededByFunctionRBrace = [](const FormatToken &Tok) {
const auto *Prev = Tok.Previous;
if (!Prev || Prev->isNot(tok::r_brace))
return false;
const auto *LBrace = Prev->MatchingParen;
return LBrace && LBrace->is(TT_FunctionLBrace);
};
const auto &SourceMgr = Env.getSourceManager();
const auto End = Lines.end();
for (auto I = Lines.begin(); I != End; ++I) {
const auto Line = *I;
removeSemi(Line->Children, Result);
removeSemi(Annotator, Line->Children, Result);
if (!Line->Affected)
continue;
Annotator.calculateFormattingInformation(*Line);
const auto NextLine = I + 1 == End ? nullptr : I[1];
for (auto Token = Line->First; Token && !Token->Finalized;
Token = Token->Next) {
if (!Token->Optional)
continue;
if (Token->isNot(tok::semi))
if (Token->isNot(tok::semi) ||
(!Token->Optional && !PrecededByFunctionRBrace(*Token))) {
continue;
}
auto Next = Token->Next;
assert(Next || Token == Line->Last);
if (!Next && NextLine)
Expand Down Expand Up @@ -3677,7 +3686,7 @@ reformat(const FormatStyle &Style, StringRef Code,
FormatStyle S = Expanded;
S.RemoveSemicolon = true;
Passes.emplace_back([&, S = std::move(S)](const Environment &Env) {
return SemiRemover(Env, S).process(/*SkipAnnotation=*/true);
return SemiRemover(Env, S).process();
});
}

Expand Down
19 changes: 13 additions & 6 deletions clang/unittests/Format/FormatTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26707,13 +26707,20 @@ TEST_F(FormatTest, RemoveSemicolon) {

verifyIncompleteFormat("class C final [[deprecated(l]] {});", Style);

// These tests are here to show a problem that may not be easily
// solved, our implementation to remove semicolons is only as good
// as our FunctionLBrace detection and this fails for empty braces
// because we can't distringuish this from a bracelist.
// We will enable when that is resolved.
#if 0
verifyFormat("void main() {}", "void main() {};", Style);

verifyFormat("struct Foo {\n"
" Foo() {}\n"
" ~Foo() {}\n"
"};",
"struct Foo {\n"
" Foo() {};\n"
" ~Foo() {};\n"
"};",
Style);

// We can't (and probably shouldn't) support the following.
#if 0
verifyFormat("void foo() {} //\n"
"int bar;",
"void foo() {}; //\n"
Expand Down