MyDeveloperDay updated this revision to Diff 466291.
MyDeveloperDay marked 16 inline comments as done.
MyDeveloperDay added a comment.
- Switch to the looping method and remove the need to change parseBlock (phew!)
thanks @owenpan for the guidance
- Add tests for double `;;`
- Change the name to RemoveSemicolon
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135466/new/
https://reviews.llvm.org/D135466
Files:
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
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -20281,6 +20281,7 @@
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 @@
"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
+ // when we fix that..then these tests can be corrected to remove the ;
+ // and to add a space between the `)` and `{`
+ verifyFormat("void main(){};", "void main() {};", Style);
+ verifyFormat("void foo(){}; //\n"
+ ";\n"
+ "int bar;",
+ "void foo(){}; //\n"
+ "; int bar;",
+ Style);
+}
+
} // namespace
} // namespace format
} // namespace clang
Index: clang/lib/Format/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -920,6 +920,9 @@
return IfLBrace;
}
+ const bool IsFunctionRBrace =
+ FormatTok->is(tok::r_brace) && Tok->is(TT_FunctionLBrace);
+
auto RemoveBraces = [=]() mutable {
if (!SimpleBlock)
return false;
@@ -959,6 +962,14 @@
// Munch the closing brace.
nextToken(/*LevelDifference=*/-AddLevels);
+
+ if (Style.RemoveSemicolon && IsFunctionRBrace) {
+ while (FormatTok->is(tok::semi)) {
+ FormatTok->Optional = true;
+ nextToken();
+ }
+ }
+
HandleVerilogBlockLabel();
if (MacroBlock && FormatTok->is(tok::l_paren))
@@ -978,8 +989,12 @@
parseStructuralElement();
}
- if (MunchSemi && FormatTok->is(tok::semi))
+ // 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 (MunchSemi && FormatTok->is(tok::semi)) {
nextToken();
+ }
if (PPStartHash == PPEndHash) {
Line->MatchingOpeningBlockLineIndex = OpeningLineIndex;
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -852,6 +852,7 @@
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 @@
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 @@
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 @@
});
}
+ 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();
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -3055,6 +3055,24 @@
/// \version 14
bool RemoveBracesLLVM;
+ /// Remove redundant semicolons after the closing brace of a 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 +3987,7 @@
RawStringFormats == R.RawStringFormats &&
ReferenceAlignment == R.ReferenceAlignment &&
RemoveBracesLLVM == R.RemoveBracesLLVM &&
+ RemoveSemicolon == R.RemoveSemicolon &&
RequiresClausePosition == R.RequiresClausePosition &&
SeparateDefinitionBlocks == R.SeparateDefinitionBlocks &&
ShortNamespaceLines == R.ShortNamespaceLines &&
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -525,6 +525,7 @@
clang-format
------------
+- Add `RemoveSemicolon` option for removing unnecessary `;` after a function definition.
clang-extdef-mapping
--------------------
Index: clang/docs/ClangFormatStyleOptions.rst
===================================================================
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -3758,6 +3758,24 @@
}
}
+**RemoveSemicolon** (``Boolean``) :versionbadge:`clang-format 16`
+ Remove redundant semicolons after the closing brace of a 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.
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
