[clang-format] Add BreakFunctionDeclarationParameters option #158745
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-format Author: Fanteria (Fanteria) ChangesAdd a new clang-format option Closes #158742 Full diff: https://github.com/llvm/llvm-project/pull/158745.diff 5 Files Affected:
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 9413b9a348b76..ab1afb8e9697a 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -3565,6 +3565,21 @@ the configuration (without a prefix: ``Auto``).
+.. _BreakFunctionDeclarationParameters:
+
+**BreakFunctionDeclarationParameters** (``Boolean``) :versionbadge:`clang-format 22` :ref:`¶ <BreakFunctionDeclarationParameters>`
+ If ``true``, clang-format will always break before function declaration
+ parameters.
+
+ .. code-block:: c++
+
+ true:
+ void functionDeclaration(
+ int A, int B);
+
+ false:
+ void functionDeclaration(int A, int B);
+
.. _BreakFunctionDefinitionParameters:
**BreakFunctionDefinitionParameters** (``Boolean``) :versionbadge:`clang-format 19` :ref:`¶ <BreakFunctionDefinitionParameters>`
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 342fefcfc408c..22f60b3db6770 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2389,6 +2389,20 @@ struct FormatStyle {
/// \version 19
bool BreakFunctionDefinitionParameters;
+ /// If ``true``, clang-format will always break before function declaration
+ /// parameters.
+ /// \code
+ /// true:
+ /// void functionDeclaration(
+ /// int A, int B);
+ ///
+ /// false:
+ /// void functionDeclaration(int A, int B);
+ ///
+ /// \endcode
+ /// \version 22
+ bool BreakFunctionDeclarationParameters;
+
/// Break after each annotation on a field in Java files.
/// \code{.java}
/// true: false:
@@ -5497,6 +5511,8 @@ struct FormatStyle {
BreakConstructorInitializers == R.BreakConstructorInitializers &&
BreakFunctionDefinitionParameters ==
R.BreakFunctionDefinitionParameters &&
+ BreakFunctionDeclarationParameters ==
+ R.BreakFunctionDeclarationParameters &&
BreakInheritanceList == R.BreakInheritanceList &&
BreakStringLiterals == R.BreakStringLiterals &&
BreakTemplateDeclarations == R.BreakTemplateDeclarations &&
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 68e9618432035..83d2f645df0f8 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1083,6 +1083,8 @@ template <> struct MappingTraits<FormatStyle> {
Style.BreakConstructorInitializers);
IO.mapOptional("BreakFunctionDefinitionParameters",
Style.BreakFunctionDefinitionParameters);
+ IO.mapOptional("BreakFunctionDeclarationParameters",
+ Style.BreakFunctionDeclarationParameters);
IO.mapOptional("BreakInheritanceList", Style.BreakInheritanceList);
IO.mapOptional("BreakStringLiterals", Style.BreakStringLiterals);
IO.mapOptional("BreakTemplateDeclarations",
@@ -1617,6 +1619,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.BreakBinaryOperations = FormatStyle::BBO_Never;
LLVMStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
LLVMStyle.BreakFunctionDefinitionParameters = false;
+ LLVMStyle.BreakFunctionDeclarationParameters = false;
LLVMStyle.BreakInheritanceList = FormatStyle::BILS_BeforeColon;
LLVMStyle.BreakStringLiterals = true;
LLVMStyle.BreakTemplateDeclarations = FormatStyle::BTDS_MultiLine;
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index d97f56751ea69..8420b29628b62 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -5626,6 +5626,12 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
return true;
}
+ if (Style.BreakFunctionDeclarationParameters && Line.MightBeFunctionDecl &&
+ !Line.mightBeFunctionDefinition() && Left.MightBeFunctionDeclParen &&
+ Left.ParameterCount > 0) {
+ return true;
+ }
+
// Ignores the first parameter as this will be handled separately by
// BreakFunctionDefinitionParameters or AlignAfterOpenBracket.
if (Style.BinPackParameters == FormatStyle::BPPS_AlwaysOnePerLine &&
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index d9db06667d802..5d7e6e423e153 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -8143,6 +8143,39 @@ TEST_F(FormatTest, BreakFunctionDefinitionParameters) {
Input, Style);
}
+TEST_F(FormatTest, BreakFunctionDeclarationParameters) {
+ StringRef Input = "void functionDecl(paramA, paramB, paramC);\n"
+ "void emptyFunctionDefinition() {}\n"
+ "void functionDefinition(int A, int B, int C) {}\n"
+ "Class::Class(int A, int B) : m_A(A), m_B(B) {}";
+ verifyFormat(Input);
+
+ FormatStyle Style = getLLVMStyle();
+ EXPECT_FALSE(Style.BreakFunctionDeclarationParameters);
+ Style.BreakFunctionDeclarationParameters = true;
+ verifyFormat("void functionDecl(\n"
+ " paramA, paramB, paramC);\n"
+ "void emptyFunctionDefinition() {}\n"
+ "void functionDefinition(int A, int B, int C) {}\n"
+ "class Class {\n"
+ " Class(\n"
+ " int A, int B);\n"
+ "};\n",
+ Input, Style);
+
+ // Test the style where all parameters are on their own lines.
+ Style.AllowAllParametersOfDeclarationOnNextLine = false;
+ Style.BinPackParameters = FormatStyle::BPPS_OnePerLine;
+ verifyFormat("void functionDecl(\n"
+ " paramA,\n"
+ " paramB,\n"
+ " paramC);\n"
+ "void emptyFunctionDefinition() {}\n"
+ "void functionDefinition(int A, int B, int C) {}\n"
+ "Class::Class(int A, int B) : m_A(A), m_B(B) {}",
+ Input, Style);
+}
+
TEST_F(FormatTest, BreakBeforeInlineASMColon) {
FormatStyle Style = getLLVMStyle();
Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_Never;
|
|
@llvm/pr-subscribers-clang Author: Fanteria (Fanteria) ChangesAdd a new clang-format option Closes #158742 Full diff: https://github.com/llvm/llvm-project/pull/158745.diff 5 Files Affected:
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 9413b9a348b76..ab1afb8e9697a 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -3565,6 +3565,21 @@ the configuration (without a prefix: ``Auto``).
+.. _BreakFunctionDeclarationParameters:
+
+**BreakFunctionDeclarationParameters** (``Boolean``) :versionbadge:`clang-format 22` :ref:`¶ <BreakFunctionDeclarationParameters>`
+ If ``true``, clang-format will always break before function declaration
+ parameters.
+
+ .. code-block:: c++
+
+ true:
+ void functionDeclaration(
+ int A, int B);
+
+ false:
+ void functionDeclaration(int A, int B);
+
.. _BreakFunctionDefinitionParameters:
**BreakFunctionDefinitionParameters** (``Boolean``) :versionbadge:`clang-format 19` :ref:`¶ <BreakFunctionDefinitionParameters>`
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 342fefcfc408c..22f60b3db6770 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -2389,6 +2389,20 @@ struct FormatStyle {
/// \version 19
bool BreakFunctionDefinitionParameters;
+ /// If ``true``, clang-format will always break before function declaration
+ /// parameters.
+ /// \code
+ /// true:
+ /// void functionDeclaration(
+ /// int A, int B);
+ ///
+ /// false:
+ /// void functionDeclaration(int A, int B);
+ ///
+ /// \endcode
+ /// \version 22
+ bool BreakFunctionDeclarationParameters;
+
/// Break after each annotation on a field in Java files.
/// \code{.java}
/// true: false:
@@ -5497,6 +5511,8 @@ struct FormatStyle {
BreakConstructorInitializers == R.BreakConstructorInitializers &&
BreakFunctionDefinitionParameters ==
R.BreakFunctionDefinitionParameters &&
+ BreakFunctionDeclarationParameters ==
+ R.BreakFunctionDeclarationParameters &&
BreakInheritanceList == R.BreakInheritanceList &&
BreakStringLiterals == R.BreakStringLiterals &&
BreakTemplateDeclarations == R.BreakTemplateDeclarations &&
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 68e9618432035..83d2f645df0f8 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1083,6 +1083,8 @@ template <> struct MappingTraits<FormatStyle> {
Style.BreakConstructorInitializers);
IO.mapOptional("BreakFunctionDefinitionParameters",
Style.BreakFunctionDefinitionParameters);
+ IO.mapOptional("BreakFunctionDeclarationParameters",
+ Style.BreakFunctionDeclarationParameters);
IO.mapOptional("BreakInheritanceList", Style.BreakInheritanceList);
IO.mapOptional("BreakStringLiterals", Style.BreakStringLiterals);
IO.mapOptional("BreakTemplateDeclarations",
@@ -1617,6 +1619,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.BreakBinaryOperations = FormatStyle::BBO_Never;
LLVMStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
LLVMStyle.BreakFunctionDefinitionParameters = false;
+ LLVMStyle.BreakFunctionDeclarationParameters = false;
LLVMStyle.BreakInheritanceList = FormatStyle::BILS_BeforeColon;
LLVMStyle.BreakStringLiterals = true;
LLVMStyle.BreakTemplateDeclarations = FormatStyle::BTDS_MultiLine;
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index d97f56751ea69..8420b29628b62 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -5626,6 +5626,12 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
return true;
}
+ if (Style.BreakFunctionDeclarationParameters && Line.MightBeFunctionDecl &&
+ !Line.mightBeFunctionDefinition() && Left.MightBeFunctionDeclParen &&
+ Left.ParameterCount > 0) {
+ return true;
+ }
+
// Ignores the first parameter as this will be handled separately by
// BreakFunctionDefinitionParameters or AlignAfterOpenBracket.
if (Style.BinPackParameters == FormatStyle::BPPS_AlwaysOnePerLine &&
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index d9db06667d802..5d7e6e423e153 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -8143,6 +8143,39 @@ TEST_F(FormatTest, BreakFunctionDefinitionParameters) {
Input, Style);
}
+TEST_F(FormatTest, BreakFunctionDeclarationParameters) {
+ StringRef Input = "void functionDecl(paramA, paramB, paramC);\n"
+ "void emptyFunctionDefinition() {}\n"
+ "void functionDefinition(int A, int B, int C) {}\n"
+ "Class::Class(int A, int B) : m_A(A), m_B(B) {}";
+ verifyFormat(Input);
+
+ FormatStyle Style = getLLVMStyle();
+ EXPECT_FALSE(Style.BreakFunctionDeclarationParameters);
+ Style.BreakFunctionDeclarationParameters = true;
+ verifyFormat("void functionDecl(\n"
+ " paramA, paramB, paramC);\n"
+ "void emptyFunctionDefinition() {}\n"
+ "void functionDefinition(int A, int B, int C) {}\n"
+ "class Class {\n"
+ " Class(\n"
+ " int A, int B);\n"
+ "};\n",
+ Input, Style);
+
+ // Test the style where all parameters are on their own lines.
+ Style.AllowAllParametersOfDeclarationOnNextLine = false;
+ Style.BinPackParameters = FormatStyle::BPPS_OnePerLine;
+ verifyFormat("void functionDecl(\n"
+ " paramA,\n"
+ " paramB,\n"
+ " paramC);\n"
+ "void emptyFunctionDefinition() {}\n"
+ "void functionDefinition(int A, int B, int C) {}\n"
+ "Class::Class(int A, int B) : m_A(A), m_B(B) {}",
+ Input, Style);
+}
+
TEST_F(FormatTest, BreakBeforeInlineASMColon) {
FormatStyle Style = getLLVMStyle();
Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_Never;
|
|
@mydeveloperday @owenca Could you please review this PR when you have a moment? Thank you! |
|
|
||
| FormatStyle Style = getLLVMStyle(); | ||
| EXPECT_FALSE(Style.BreakFunctionDeclarationParameters); | ||
| Style.BreakFunctionDeclarationParameters = true; |
There was a problem hiding this comment.
your documentation suggest that having this as false, doesn't break, but actually almost looks like it "joins" the lines, but looking at the code I can't see that is the case,
does false really mean "Leave" or "join"...could you add a test if you mean Join but also move it to an enum so we can have a "Leave" case..
How does this interact with other options? doing similar things..
| /// | ||
| /// \endcode | ||
| /// \version 22 | ||
| bool BreakFunctionDeclarationParameters; |
There was a problem hiding this comment.
I can't quite tell but it feel like true is "AlwaysBreakFunctionDeclarationParameters" because false doesn't feel like NeverBreak... just depends on what is there currently?
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions cpp,h -- clang/include/clang/Format/Format.h clang/lib/Format/Format.cpp clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/FormatTest.cpp
View the diff from clang-format here.diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 8420b2962..4b0b3ed98 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -5627,8 +5627,8 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
}
if (Style.BreakFunctionDeclarationParameters && Line.MightBeFunctionDecl &&
- !Line.mightBeFunctionDefinition() && Left.MightBeFunctionDeclParen &&
- Left.ParameterCount > 0) {
+ !Line.mightBeFunctionDefinition() && Left.MightBeFunctionDeclParen &&
+ Left.ParameterCount > 0) {
return true;
}
|
| /// | ||
| /// \endcode | ||
| /// \version 22 | ||
| bool BreakFunctionDeclarationParameters; |
There was a problem hiding this comment.
Please sort the option (here and elsewhere) alphabetically.
| Style.BreakConstructorInitializers); | ||
| IO.mapOptional("BreakFunctionDefinitionParameters", | ||
| Style.BreakFunctionDefinitionParameters); | ||
| IO.mapOptional("BreakFunctionDeclarationParameters", |
There was a problem hiding this comment.
You need to add a parsing test for this.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add a new clang-format option
BreakFunctionDeclarationParametersto control whether parameters in function declarations can be broken onto new lines, similar to the existingBreakFunctionDefinitionParametersoption.Closes #158742