[clang] 69e7727 - [clang-format] Add support to remove unnecessary semicolons after function definition
via cfe-commits
cfe-commits at lists.llvm.org
Sun Oct 9 03:19:26 PDT 2022
Author: mydeveloperday
Date: 2022-10-09T11:18:16+01:00
New Revision: 69e772768e957629d41ed53eaf4dcda96fa9dac7
URL: https://github.com/llvm/llvm-project/commit/69e772768e957629d41ed53eaf4dcda96fa9dac7
DIFF: https://github.com/llvm/llvm-project/commit/69e772768e957629d41ed53eaf4dcda96fa9dac7.diff
LOG: [clang-format] Add support to remove unnecessary semicolons after function definition
Fixes: https://github.com/llvm/llvm-project/issues/58217
This change is to remove extraneous and unnecessary ';' from after a function definition, its off by default and carries the same "code modification" warning as some of our other code manipulating changes.
Reviewed By: HazardyKnusperkeks, owenpan
Differential Revision: https://reviews.llvm.org/D135466
Added:
Modified:
clang/docs/ClangFormatStyleOptions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp
Removed:
################################################################################
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 3be291578828d..ec53c3eef8540 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -3758,6 +3758,23 @@ the configuration (without a prefix: ``Auto``).
}
}
+**RemoveSemicolon** (``Boolean``) :versionbadge:`clang-format 16`
+ Remove semicolons after the closing brace of a non-empty function.
+
+ .. warning::
+
+ Setting this option to `true` could lead to incorrect code formatting due
+ to clang-format's lack of complete semantic information. As such, extra
+ care should be taken to review code changes made by this option.
+
+ .. code-block:: c++
+
+ false: true:
+
+ int max(int a, int b) { int max(int a, int b) {
+ return a > b ? a : b; return a > b ? a : b;
+ }; }
+
**RequiresClausePosition** (``RequiresClausePositionStyle``) :versionbadge:`clang-format 15`
The position of the ``requires`` clause.
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7907e34753b66..513a5eda80801 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -525,6 +525,7 @@ AST Matchers
clang-format
------------
+- Add `RemoveSemicolon` option for removing `;` after a non-empty function definition.
clang-extdef-mapping
--------------------
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index c8b22f5ebde06..ee1863a63bc9a 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -3055,6 +3055,23 @@ struct FormatStyle {
/// \version 14
bool RemoveBracesLLVM;
+ /// Remove semicolons after the closing brace of a non-empty function.
+ /// \warning
+ /// Setting this option to `true` could lead to incorrect code formatting due
+ /// to clang-format's lack of complete semantic information. As such, extra
+ /// care should be taken to review code changes made by this option.
+ /// \endwarning
+ /// \code
+ /// false: true:
+ ///
+ /// int max(int a, int b) { int max(int a, int b) {
+ /// return a > b ? a : b; return a > b ? a : b;
+ /// }; }
+ ///
+ /// \endcode
+ /// \version 16
+ bool RemoveSemicolon;
+
/// \brief The possible positions for the requires clause. The
/// ``IndentRequires`` option is only used if the ``requires`` is put on the
/// start of a line.
@@ -3969,6 +3986,7 @@ struct FormatStyle {
RawStringFormats == R.RawStringFormats &&
ReferenceAlignment == R.ReferenceAlignment &&
RemoveBracesLLVM == R.RemoveBracesLLVM &&
+ RemoveSemicolon == R.RemoveSemicolon &&
RequiresClausePosition == R.RequiresClausePosition &&
SeparateDefinitionBlocks == R.SeparateDefinitionBlocks &&
ShortNamespaceLines == R.ShortNamespaceLines &&
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 5731ffeec014d..37f171aa1f35b 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -852,6 +852,7 @@ template <> struct MappingTraits<FormatStyle> {
IO.mapOptional("ReferenceAlignment", Style.ReferenceAlignment);
IO.mapOptional("ReflowComments", Style.ReflowComments);
IO.mapOptional("RemoveBracesLLVM", Style.RemoveBracesLLVM);
+ IO.mapOptional("RemoveSemicolon", Style.RemoveSemicolon);
IO.mapOptional("RequiresClausePosition", Style.RequiresClausePosition);
IO.mapOptional("SeparateDefinitionBlocks", Style.SeparateDefinitionBlocks);
IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines);
@@ -1297,6 +1298,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.UseTab = FormatStyle::UT_Never;
LLVMStyle.ReflowComments = true;
LLVMStyle.RemoveBracesLLVM = false;
+ LLVMStyle.RemoveSemicolon = false;
LLVMStyle.SpacesInParentheses = false;
LLVMStyle.SpacesInSquareBrackets = false;
LLVMStyle.SpaceInEmptyBlock = false;
@@ -1915,7 +1917,59 @@ class BracesRemover : public TokenAnalyzer {
Token = Token->Next) {
if (!Token->Optional)
continue;
- assert(Token->isOneOf(tok::l_brace, tok::r_brace));
+ if (!Token->isOneOf(tok::l_brace, tok::r_brace))
+ continue;
+ auto Next = Token->Next;
+ assert(Next || Token == Line->Last);
+ if (!Next && NextLine)
+ Next = NextLine->First;
+ SourceLocation Start;
+ if (Next && Next->NewlinesBefore == 0 && Next->isNot(tok::eof)) {
+ Start = Token->Tok.getLocation();
+ Next->WhitespaceRange = Token->WhitespaceRange;
+ } else {
+ Start = Token->WhitespaceRange.getBegin();
+ }
+ const auto Range =
+ CharSourceRange::getCharRange(Start, Token->Tok.getEndLoc());
+ cantFail(Result.add(tooling::Replacement(SourceMgr, Range, "")));
+ }
+ }
+ }
+};
+
+class SemiRemover : public TokenAnalyzer {
+public:
+ SemiRemover(const Environment &Env, const FormatStyle &Style)
+ : TokenAnalyzer(Env, Style) {}
+
+ std::pair<tooling::Replacements, unsigned>
+ analyze(TokenAnnotator &Annotator,
+ SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
+ FormatTokenLexer &Tokens) override {
+ AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
+ tooling::Replacements Result;
+ removeSemi(AnnotatedLines, Result);
+ return {Result, 0};
+ }
+
+private:
+ void removeSemi(SmallVectorImpl<AnnotatedLine *> &Lines,
+ tooling::Replacements &Result) {
+ 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);
+ if (!Line->Affected)
+ continue;
+ 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))
+ continue;
auto Next = Token->Next;
assert(Next || Token == Line->Last);
if (!Next && NextLine)
@@ -3280,6 +3334,12 @@ reformat(const FormatStyle &Style, StringRef Code,
});
}
+ if (Style.RemoveSemicolon) {
+ Passes.emplace_back([&](const Environment &Env) {
+ return SemiRemover(Env, Expanded).process(/*SkipAnnotation=*/true);
+ });
+ }
+
if (Style.FixNamespaceComments) {
Passes.emplace_back([&](const Environment &Env) {
return NamespaceEndCommentsFixer(Env, Expanded).process();
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index f39c107e5269e..2145cfe06b835 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -920,6 +920,9 @@ FormatToken *UnwrappedLineParser::parseBlock(
return IfLBrace;
}
+ const bool IsFunctionRBrace =
+ FormatTok->is(tok::r_brace) && Tok->is(TT_FunctionLBrace);
+
auto RemoveBraces = [=]() mutable {
if (!SimpleBlock)
return false;
@@ -959,6 +962,17 @@ FormatToken *UnwrappedLineParser::parseBlock(
// Munch the closing brace.
nextToken(/*LevelDifference=*/-AddLevels);
+
+ // When this is a function block and there is an unnecessary semicolon
+ // afterwards then mark it as optional (so the RemoveSemi pass can get rid of
+ // it later).
+ if (Style.RemoveSemicolon && IsFunctionRBrace) {
+ while (FormatTok->is(tok::semi)) {
+ FormatTok->Optional = true;
+ nextToken();
+ }
+ }
+
HandleVerilogBlockLabel();
if (MacroBlock && FormatTok->is(tok::l_paren))
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index fd567befec974..e29fe1066623e 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -20281,6 +20281,7 @@ TEST_F(FormatTest, ParsesConfigurationBools) {
CHECK_PARSE_BOOL(Cpp11BracedListStyle);
CHECK_PARSE_BOOL(ReflowComments);
CHECK_PARSE_BOOL(RemoveBracesLLVM);
+ CHECK_PARSE_BOOL(RemoveSemicolon);
CHECK_PARSE_BOOL(SortUsingDeclarations);
CHECK_PARSE_BOOL(SpacesInParentheses);
CHECK_PARSE_BOOL(SpacesInSquareBrackets);
@@ -26740,6 +26741,51 @@ TEST_F(FormatTest, FormatsVariableTemplates) {
"inline bool var = is_integral_v<T> && is_signed_v<T>;");
}
+TEST_F(FormatTest, RemoveSemicolon) {
+ FormatStyle Style = getLLVMStyle();
+ Style.RemoveSemicolon = true;
+
+ verifyFormat("int max(int a, int b) { return a > b ? a : b; }",
+ "int max(int a, int b) { return a > b ? a : b; };", Style);
+
+ verifyFormat("int max(int a, int b) { return a > b ? a : b; }",
+ "int max(int a, int b) { return a > b ? a : b; };;", Style);
+
+ verifyFormat("class Foo {\n"
+ " int getSomething() const { return something; }\n"
+ "};",
+ "class Foo {\n"
+ " int getSomething() const { return something; };\n"
+ "};",
+ Style);
+
+ verifyFormat("class Foo {\n"
+ " int getSomething() const { return something; }\n"
+ "};",
+ "class Foo {\n"
+ " int getSomething() const { return something; };;\n"
+ "};",
+ Style);
+
+ verifyFormat("for (;;) {\n"
+ "}",
+ 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("void foo() {} //\n"
+ "int bar;",
+ "void foo() {}; //\n"
+ "; int bar;",
+ Style);
+#endif
+}
+
} // namespace
} // namespace format
} // namespace clang
More information about the cfe-commits
mailing list