Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 04fbc46

Browse files
authoredFeb 21, 2024
[clang-format] Fix RemoveSemicolon for empty functions (#82278)
Fixes #79833.
1 parent ec516ff commit 04fbc46

File tree

2 files changed

+29
-13
lines changed

2 files changed

+29
-13
lines changed
 

‎clang/lib/Format/Format.cpp

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2261,27 +2261,36 @@ class SemiRemover : public TokenAnalyzer {
22612261
FormatTokenLexer &Tokens) override {
22622262
AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
22632263
tooling::Replacements Result;
2264-
removeSemi(AnnotatedLines, Result);
2264+
removeSemi(Annotator, AnnotatedLines, Result);
22652265
return {Result, 0};
22662266
}
22672267

22682268
private:
2269-
void removeSemi(SmallVectorImpl<AnnotatedLine *> &Lines,
2269+
void removeSemi(TokenAnnotator &Annotator,
2270+
SmallVectorImpl<AnnotatedLine *> &Lines,
22702271
tooling::Replacements &Result) {
2272+
auto PrecededByFunctionRBrace = [](const FormatToken &Tok) {
2273+
const auto *Prev = Tok.Previous;
2274+
if (!Prev || Prev->isNot(tok::r_brace))
2275+
return false;
2276+
const auto *LBrace = Prev->MatchingParen;
2277+
return LBrace && LBrace->is(TT_FunctionLBrace);
2278+
};
22712279
const auto &SourceMgr = Env.getSourceManager();
22722280
const auto End = Lines.end();
22732281
for (auto I = Lines.begin(); I != End; ++I) {
22742282
const auto Line = *I;
2275-
removeSemi(Line->Children, Result);
2283+
removeSemi(Annotator, Line->Children, Result);
22762284
if (!Line->Affected)
22772285
continue;
2286+
Annotator.calculateFormattingInformation(*Line);
22782287
const auto NextLine = I + 1 == End ? nullptr : I[1];
22792288
for (auto Token = Line->First; Token && !Token->Finalized;
22802289
Token = Token->Next) {
2281-
if (!Token->Optional)
2282-
continue;
2283-
if (Token->isNot(tok::semi))
2290+
if (Token->isNot(tok::semi) ||
2291+
(!Token->Optional && !PrecededByFunctionRBrace(*Token))) {
22842292
continue;
2293+
}
22852294
auto Next = Token->Next;
22862295
assert(Next || Token == Line->Last);
22872296
if (!Next && NextLine)
@@ -3677,7 +3686,7 @@ reformat(const FormatStyle &Style, StringRef Code,
36773686
FormatStyle S = Expanded;
36783687
S.RemoveSemicolon = true;
36793688
Passes.emplace_back([&, S = std::move(S)](const Environment &Env) {
3680-
return SemiRemover(Env, S).process(/*SkipAnnotation=*/true);
3689+
return SemiRemover(Env, S).process();
36813690
});
36823691
}
36833692

‎clang/unittests/Format/FormatTest.cpp

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26720,13 +26720,20 @@ TEST_F(FormatTest, RemoveSemicolon) {
2672026720

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

26723-
// These tests are here to show a problem that may not be easily
26724-
// solved, our implementation to remove semicolons is only as good
26725-
// as our FunctionLBrace detection and this fails for empty braces
26726-
// because we can't distringuish this from a bracelist.
26727-
// We will enable when that is resolved.
26728-
#if 0
2672926723
verifyFormat("void main() {}", "void main() {};", Style);
26724+
26725+
verifyFormat("struct Foo {\n"
26726+
" Foo() {}\n"
26727+
" ~Foo() {}\n"
26728+
"};",
26729+
"struct Foo {\n"
26730+
" Foo() {};\n"
26731+
" ~Foo() {};\n"
26732+
"};",
26733+
Style);
26734+
26735+
// We can't (and probably shouldn't) support the following.
26736+
#if 0
2673026737
verifyFormat("void foo() {} //\n"
2673126738
"int bar;",
2673226739
"void foo() {}; //\n"

0 commit comments

Comments
 (0)
Please sign in to comment.