[PATCH] D34399: clang-format: introduce InlineOnly short function style
This revision was automatically updated to reflect the committed changes.
Closed by commit rL305912: clang-format: introduce InlineOnly short function
style (authored by Typz).
Changed prior to commit:
https://reviews.llvm.org/D34399?vs=103357&id=103370#toc
Repository:
rL LLVM
https://reviews.llvm.org/D34399
Files:
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -184,9 +184,23 @@
enum ShortFunctionStyle {
/// \brief Never merge functions into a single line.
SFS_None,
+/// \brief Only merge functions defined inside a class. Same as "inline",
+/// except it does not implies "empty": i.e. top level empty functions
+/// are not merged either.
+/// \code
+/// class Foo {
+/// void f() { foo(); }
+/// };
+/// void f() {
+/// foo();
+/// }
+/// void f() {
+/// }
+/// \endcode
+SFS_InlineOnly,
/// \brief Only merge empty functions.
/// \code
-/// void f() { bar(); }
+/// void f() {}
/// void f2() {
/// bar2();
/// }
@@ -197,6 +211,10 @@
/// class Foo {
/// void f() { foo(); }
/// };
+/// void f() {
+/// foo();
+/// }
+/// void f() {}
/// \endcode
SFS_Inline,
/// \brief Merge all functions fitting on a single line.
Index: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
@@ -226,7 +226,7 @@
Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All ||
(Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty &&
I[1]->First->is(tok::r_brace)) ||
-(Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Inline &&
+(Style.AllowShortFunctionsOnASingleLine & FormatStyle::SFS_InlineOnly &&
TheLine->Level != 0);
if (Style.CompactNamespaces) {
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -97,6 +97,7 @@
IO.enumCase(Value, "All", FormatStyle::SFS_All);
IO.enumCase(Value, "true", FormatStyle::SFS_All);
IO.enumCase(Value, "Inline", FormatStyle::SFS_Inline);
+IO.enumCase(Value, "InlineOnly", FormatStyle::SFS_InlineOnly);
IO.enumCase(Value, "Empty", FormatStyle::SFS_Empty);
}
};
Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2480,8 +2480,8 @@
return Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_None ||
Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Empty ||
(Left.NestingLevel == 0 && Line.Level == 0 &&
- Style.AllowShortFunctionsOnASingleLine ==
- FormatStyle::SFS_Inline);
+ Style.AllowShortFunctionsOnASingleLine &
+ FormatStyle::SFS_InlineOnly);
} else if (Style.Language == FormatStyle::LK_Java) {
if (Right.is(tok::plus) && Left.is(tok::string_literal) && Right.Next &&
Right.Next->is(tok::string_literal))
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -6509,6 +6509,52 @@
MergeInlineOnly);
}
+TEST_F(FormatTest, PullInlineOnlyFunctionDefinitionsIntoSingleLine) {
+ FormatStyle MergeInlineOnly = getLLVMStyle();
+ MergeInlineOnly.AllowShortFunctionsOnASingleLine =
+ FormatStyle::SFS_InlineOnly;
+ verifyFormat("class C {\n"
+ " int f() { return 42; }\n"
+ "};",
+ MergeInlineOnly);
+ verifyFormat("int f() {\n"
+ " return 42;\n"
+ "}",
+ MergeInlineOnly);
+
+ // SFS_InlineOnly does not imply SFS_Empty
+ verifyFormat("class C {\n"
+ " int f() {}\n"
+ "};",
+ MergeInlineOnly);
+ verifyFormat("int f() {\n"
+ "}",
+ MergeInlineOnly);
+
+ // Also verify behavior when BraceWrapping.AfterFunction = true
+ MergeInlineOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+ MergeInlineOnly.BraceWrapping.AfterFunction = true;
+ verifyFormat("class C {\n"
+ " int f() { return 42; }\n"
+ "};",
+ Merg
[PATCH] D34399: clang-format: introduce InlineOnly short function style
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. https://reviews.llvm.org/D34399 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34399: clang-format: introduce InlineOnly short function style
Typz updated this revision to Diff 103357.
Typz marked an inline comment as done.
Typz added a comment.
Fix according to review comments
https://reviews.llvm.org/D34399
Files:
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/TokenAnnotator.cpp
lib/Format/UnwrappedLineFormatter.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6509,6 +6509,52 @@
MergeInlineOnly);
}
+TEST_F(FormatTest, PullInlineOnlyFunctionDefinitionsIntoSingleLine) {
+ FormatStyle MergeInlineOnly = getLLVMStyle();
+ MergeInlineOnly.AllowShortFunctionsOnASingleLine =
+ FormatStyle::SFS_InlineOnly;
+ verifyFormat("class C {\n"
+ " int f() { return 42; }\n"
+ "};",
+ MergeInlineOnly);
+ verifyFormat("int f() {\n"
+ " return 42;\n"
+ "}",
+ MergeInlineOnly);
+
+ // SFS_InlineOnly does not imply SFS_Empty
+ verifyFormat("class C {\n"
+ " int f() {}\n"
+ "};",
+ MergeInlineOnly);
+ verifyFormat("int f() {\n"
+ "}",
+ MergeInlineOnly);
+
+ // Also verify behavior when BraceWrapping.AfterFunction = true
+ MergeInlineOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+ MergeInlineOnly.BraceWrapping.AfterFunction = true;
+ verifyFormat("class C {\n"
+ " int f() { return 42; }\n"
+ "};",
+ MergeInlineOnly);
+ verifyFormat("int f()\n"
+ "{\n"
+ " return 42;\n"
+ "}",
+ MergeInlineOnly);
+
+ // SFS_InlineOnly does not imply SFS_Empty
+ verifyFormat("int f()\n"
+ "{\n"
+ "}",
+ MergeInlineOnly);
+ verifyFormat("class C {\n"
+ " int f() {}\n"
+ "};",
+ MergeInlineOnly);
+}
+
TEST_F(FormatTest, SplitEmptyFunctionBody) {
FormatStyle Style = getLLVMStyle();
Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -226,7 +226,7 @@
Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All ||
(Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty &&
I[1]->First->is(tok::r_brace)) ||
-(Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Inline &&
+(Style.AllowShortFunctionsOnASingleLine & FormatStyle::SFS_InlineOnly &&
TheLine->Level != 0);
if (Style.CompactNamespaces) {
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2480,8 +2480,8 @@
return Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_None ||
Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Empty ||
(Left.NestingLevel == 0 && Line.Level == 0 &&
- Style.AllowShortFunctionsOnASingleLine ==
- FormatStyle::SFS_Inline);
+ Style.AllowShortFunctionsOnASingleLine &
+ FormatStyle::SFS_InlineOnly);
} else if (Style.Language == FormatStyle::LK_Java) {
if (Right.is(tok::plus) && Left.is(tok::string_literal) && Right.Next &&
Right.Next->is(tok::string_literal))
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -96,6 +96,7 @@
IO.enumCase(Value, "All", FormatStyle::SFS_All);
IO.enumCase(Value, "true", FormatStyle::SFS_All);
IO.enumCase(Value, "Inline", FormatStyle::SFS_Inline);
+IO.enumCase(Value, "InlineOnly", FormatStyle::SFS_InlineOnly);
IO.enumCase(Value, "Empty", FormatStyle::SFS_Empty);
}
};
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -184,9 +184,23 @@
enum ShortFunctionStyle {
/// \brief Never merge functions into a single line.
SFS_None,
+/// \brief Only merge functions defined inside a class. Same as "inline",
+/// except it does not implies "empty": i.e. top level empty functions
+/// are not merged either.
+/// \code
+/// class Foo {
+/// void f() { foo(); }
+/// };
+/// void f() {
+/// foo();
+/// }
+/// void f() {
+/// }
+/// \endcode
+SFS_InlineOnly,
/// \brief Only merge empty functions.
/// \code
-/// void f() { bar(); }
+/// void f() {}
/// void f2() {
/// bar2();
/// }
@@ -197,6 +211,10 @@
///
[PATCH] D34399: clang-format: introduce InlineOnly short function style
Typz marked 3 inline comments as done.
Typz added inline comments.
Comment at: include/clang/Format/Format.h:234
+ bool allowEmptyFunctionsOnASingleLine() const {
+ return AllowShortFunctionsOnASingleLine >= ShortFunctionStyle::SFS_Empty;
djasper wrote:
> I'd prefer these functions not to be in the public header file. They are
> implementation details. Either find a header/cpp-file in the lib/ directory
> to place them in or just remove them for now. They are both still quite small
> and called only twice each.
This is similar to the isCpp() method, to provide a better abstraction in the
implementation.
I cannot find any better place, so I will remove them.
https://reviews.llvm.org/D34399
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34399: clang-format: introduce InlineOnly short function style
djasper added inline comments.
Comment at: include/clang/Format/Format.h:234
+ bool allowEmptyFunctionsOnASingleLine() const {
+ return AllowShortFunctionsOnASingleLine >= ShortFunctionStyle::SFS_Empty;
I'd prefer these functions not to be in the public header file. They are
implementation details. Either find a header/cpp-file in the lib/ directory to
place them in or just remove them for now. They are both still quite small and
called only twice each.
Comment at: unittests/Format/FormatTest.cpp:6530
+ verifyFormat("int f() {\n"
+ "}", MergeInlineOnly);
+
Missing line break.
Comment at: unittests/Format/FormatTest.cpp:6548
+ "{\n"
+ "}", MergeInlineOnly);
+ verifyFormat("class C {\n"
Missing line break. Generally use clang-format on the patches :).
https://reviews.llvm.org/D34399
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34399: clang-format: introduce InlineOnly short function style
Typz created this revision.
Herald added subscribers: rengolin, klimek.
This is the same as Inline, except it does not imply all empty
functions are merged: with this style, empty functions are merged only
if they also match the 'inline' criteria (i.e. defined in a class).
This is helpful to avoid inlining functions in implementations files.
https://reviews.llvm.org/D34399
Files:
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/TokenAnnotator.cpp
lib/Format/UnwrappedLineFormatter.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -6509,6 +6509,49 @@
MergeInlineOnly);
}
+TEST_F(FormatTest, PullInlineOnlyFunctionDefinitionsIntoSingleLine) {
+ FormatStyle MergeInlineOnly = getLLVMStyle();
+ MergeInlineOnly.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_InlineOnly;
+ verifyFormat("class C {\n"
+ " int f() { return 42; }\n"
+ "};",
+ MergeInlineOnly);
+ verifyFormat("int f() {\n"
+ " return 42;\n"
+ "}",
+ MergeInlineOnly);
+
+ // SFS_InlineOnly does not imply SFS_Empty
+ verifyFormat("class C {\n"
+ " int f() {}\n"
+ "};",
+ MergeInlineOnly);
+ verifyFormat("int f() {\n"
+ "}", MergeInlineOnly);
+
+ // Also verify behavior when BraceWrapping.AfterFunction = true
+ MergeInlineOnly.BreakBeforeBraces = FormatStyle::BS_Custom;
+ MergeInlineOnly.BraceWrapping.AfterFunction = true;
+ verifyFormat("class C {\n"
+ " int f() { return 42; }\n"
+ "};",
+ MergeInlineOnly);
+ verifyFormat("int f()\n"
+ "{\n"
+ " return 42;\n"
+ "}",
+ MergeInlineOnly);
+
+ // SFS_InlineOnly does not imply SFS_Empty
+ verifyFormat("int f()\n"
+ "{\n"
+ "}", MergeInlineOnly);
+ verifyFormat("class C {\n"
+ " int f() {}\n"
+ "};",
+ MergeInlineOnly);
+}
+
TEST_F(FormatTest, SplitEmptyFunctionBody) {
FormatStyle Style = getLLVMStyle();
Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -224,9 +224,9 @@
// If necessary, change to something smarter.
bool MergeShortFunctions =
Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All ||
-(Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty &&
+(Style.allowEmptyFunctionsOnASingleLine() &&
I[1]->First->is(tok::r_brace)) ||
-(Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Inline &&
+(Style.allowShortInlineFunctionsOnASingleLine() &&
TheLine->Level != 0);
if (Style.CompactNamespaces) {
@@ -282,7 +282,7 @@
unsigned MergedLines = 0;
if (MergeShortFunctions ||
- (Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty &&
+ (Style.allowEmptyFunctionsOnASingleLine() &&
I[1]->First == I[1]->Last && I + 2 != E &&
I[2]->First->is(tok::r_brace))) {
MergedLines = tryMergeSimpleBlock(I + 1, E, Limit);
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2480,8 +2480,7 @@
return Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_None ||
Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Empty ||
(Left.NestingLevel == 0 && Line.Level == 0 &&
- Style.AllowShortFunctionsOnASingleLine ==
- FormatStyle::SFS_Inline);
+ Style.allowShortInlineFunctionsOnASingleLine());
} else if (Style.Language == FormatStyle::LK_Java) {
if (Right.is(tok::plus) && Left.is(tok::string_literal) && Right.Next &&
Right.Next->is(tok::string_literal))
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -96,6 +96,7 @@
IO.enumCase(Value, "All", FormatStyle::SFS_All);
IO.enumCase(Value, "true", FormatStyle::SFS_All);
IO.enumCase(Value, "Inline", FormatStyle::SFS_Inline);
+IO.enumCase(Value, "InlineOnly", FormatStyle::SFS_InlineOnly);
IO.enumCase(Value, "Empty", FormatStyle::SFS_Empty);
}
};
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -184,9 +184,23 @@
enum ShortFunctionStyle {
/// \brief Never merge functions into a
