Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.
Eugene.Zelenko added a subscriber: Eugene.Zelenko. Eugene.Zelenko closed this revision. Eugene.Zelenko added a comment. Committed in https://reviews.llvm.org/rL256046. https://reviews.llvm.org/D10370 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.
djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.
Generally looks good.
Comment at: include/clang/Format/Format.h:166
@@ +165,3 @@
+ enum ReturnTypeBreakingStyle {
+/// Break after return type automatically.
+/// \c PenaltyReturnTypeOnItsOwnLine is taken into account.
Use tools/dump_format_style.py to update the documentation, too.
Comment at: include/clang/Format/Format.h:171
@@ +170,3 @@
+RTBS_All,
+/// Always break after the return types of top level functions.
+RTBS_TopLevel,
nit: top-level
Comment at: lib/Format/TokenAnnotator.cpp:1642
@@ +1641,3 @@
+if (!Current->MustBreakBefore && InFunctionDecl &&
+Current->is(TT_FunctionDeclarationName)) {
+ Current->MustBreakBefore = mustBreakForReturnType(Line, *Current);
No braces, I think.
Comment at: lib/Format/TokenAnnotator.h:168
@@ -158,1 +167,3 @@
+ bool mustBreakForReturnType(const AnnotatedLine ,
+ FormatToken ) const;
Some comment might help. E.g. at the very least, does that mean must break
before or after "Token" (alternatively, name that Left or Right).
http://reviews.llvm.org/D10370
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.
zturner added inline comments. Comment at: lib/Format/TokenAnnotator.h:168 @@ -158,1 +167,3 @@ + bool mustBreakForReturnType(const AnnotatedLine , + FormatToken ) const; djasper wrote: > Some comment might help. E.g. at the very least, does that mean must break > before or after "Token" (alternatively, name that Left or Right). Turns out this variable wasn't even used. I'll just delete it. http://reviews.llvm.org/D10370 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.
zturner added a comment. ping http://reviews.llvm.org/D10370 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.
zturner updated this revision to Diff 42895.
zturner added a comment.
Attempt to address remaining issues in patch.
This is my first time touching anything having to do with clang, so there's a
good chance I don't know what i'm doing and this needs more work. Let me know.
`AlwaysBreakAfterDefinitionReturnType` is used only as a way to initialize
`AlwaysBreakAfterReturnType`, and is ignored during any actual logic.
http://reviews.llvm.org/D10370
Files:
include/clang/Format/Format.h
lib/Format/ContinuationIndenter.cpp
lib/Format/Format.cpp
lib/Format/TokenAnnotator.cpp
lib/Format/TokenAnnotator.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -4732,28 +4732,82 @@
" \"c\";");
}
-TEST_F(FormatTest, DefinitionReturnTypeBreakingStyle) {
+TEST_F(FormatTest, ReturnTypeBreakingStyle) {
FormatStyle Style = getLLVMStyle();
- Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_TopLevel;
- verifyFormat("class C {\n"
+ // No declarations or definitions should be moved to own line.
+ Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
+ verifyFormat("class A {\n"
" int f() { return 1; }\n"
+ " int g();\n"
+ "};\n"
+ "int f() { return 1; }\n"
+ "int g();\n",
+ Style);
+
+ // All declarations and definitions should have the return type moved to its
+ // own
+ // line.
+ Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All;
+ verifyFormat("class E {\n"
+ " int\n"
+ " f() {\n"
+ "return 1;\n"
+ " }\n"
+ " int\n"
+ " g();\n"
"};\n"
"int\n"
"f() {\n"
" return 1;\n"
- "}",
+ "}\n"
+ "int\n"
+ "g();\n",
Style);
- Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
+
+ // Top-level definitions, and no kinds of declarations should have the
+ // return type moved to its own line.
+ Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_TopLevelDefinitions;
+ verifyFormat("class B {\n"
+ " int f() { return 1; }\n"
+ " int g();\n"
+ "};\n"
+ "int\n"
+ "f() {\n"
+ " return 1;\n"
+ "}\n"
+ "int g();\n",
+ Style);
+
+ // Top-level definitions and declarations should have the return type moved
+ // to its own line.
+ Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_TopLevel;
verifyFormat("class C {\n"
+ " int f() { return 1; }\n"
+ " int g();\n"
+ "};\n"
+ "int\n"
+ "f() {\n"
+ " return 1;\n"
+ "}\n"
+ "int\n"
+ "g();\n",
+ Style);
+
+ // All definitions should have the return type moved to its own line, but no
+ // kinds of declarations.
+ Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_AllDefinitions;
+ verifyFormat("class D {\n"
" int\n"
" f() {\n"
"return 1;\n"
" }\n"
+ " int g();\n"
"};\n"
"int\n"
"f() {\n"
" return 1;\n"
- "}",
+ "}\n"
+ "int g();\n",
Style);
verifyFormat("const char *\n"
"f(void) {\n" // Break here.
@@ -5688,7 +5742,7 @@
verifyFormat("a __attribute__((unused))\n"
"aaa(int i);");
FormatStyle AfterType = getLLVMStyle();
- AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
+ AfterType.AlwaysBreakAfterReturnType = FormatStyle::RTBS_AllDefinitions;
verifyFormat("__attribute__((nodebug)) void\n"
"foo() {}\n",
AfterType);
@@ -9829,6 +9883,19 @@
CHECK_PARSE("BreakBeforeBraces: Custom", BreakBeforeBraces,
FormatStyle::BS_Custom);
+ Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_All;
+ CHECK_PARSE("AlwaysBreakAfterReturnType: None", AlwaysBreakAfterReturnType,
+ FormatStyle::RTBS_None);
+ CHECK_PARSE("AlwaysBreakAfterReturnType: All", AlwaysBreakAfterReturnType,
+ FormatStyle::RTBS_All);
+ CHECK_PARSE("AlwaysBreakAfterReturnType: TopLevel",
+ AlwaysBreakAfterReturnType, FormatStyle::DRTBS_TopLevel);
+ CHECK_PARSE("AlwaysBreakAfterReturnType: AllDefinitions",
+ AlwaysBreakAfterReturnType, FormatStyle::RTBS_AllDefinitions);
+ CHECK_PARSE("AlwaysBreakAfterReturnType: TopLevelDefinitions",
+
Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.
djasper added a comment. I'd just prefer the setting of the new option and ignore the old one then, but I don't think it matters to much. Warning or err'ing out also seems like a reasonable approach. http://reviews.llvm.org/D10370 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.
djasper added a comment. I think we can add the option AlwaysBreakAfterReturnType but still be backwards compatible. If clang-format finds only the old option in a configuration file, it can choose the appropriate value of the new option. I also like using just this one option better than using the one option with the added bool option. http://reviews.llvm.org/D10370 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.
zturner updated this revision to Diff 42774.
zturner added a comment.
This patch was very old, so it didn't apply against ToT. So this initial
update is just to rebase against ToT. I will make changes in a followup patch.
http://reviews.llvm.org/D10370
Files:
include/clang/Format/Format.h
lib/Format/ContinuationIndenter.cpp
lib/Format/Format.cpp
lib/Format/TokenAnnotator.cpp
lib/Format/TokenAnnotator.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -4732,6 +4732,44 @@
" \"c\";");
}
+TEST_F(FormatTest, DeclarationReturnTypeBreakingStyle) {
+ FormatStyle Style = getLLVMStyle();
+ Style.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_TopLevel;
+ verifyFormat("class C {\n"
+ " int f();\n"
+ "};\n"
+ "int\n"
+ "f();",
+ Style);
+ Style.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+ verifyFormat("class C {\n"
+ " int\n"
+ " f();\n"
+ "};\n"
+ "int\n"
+ "f();",
+ Style);
+ verifyFormat("const char *f(void) { return \"\"; }\n"
+ "const char *\n"
+ "bar(void);\n",
+ Style);
+ verifyFormat("template T *f(T ) { return NULL; }\n"
+ "template \n"
+ "T *\n"
+ "f(T );\n",
+ Style);
+ Style.BreakBeforeBraces = FormatStyle::BS_Stroustrup;
+ verifyFormat("const char *f(void) { return \"\"; }\n"
+ "const char *\n"
+ "bar(void);\n",
+ Style);
+ verifyFormat("template T *f(T ) { return NULL; }\n"
+ "template \n"
+ "T *\n"
+ "f(T );\n",
+ Style);
+}
+
TEST_F(FormatTest, DefinitionReturnTypeBreakingStyle) {
FormatStyle Style = getLLVMStyle();
Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_TopLevel;
@@ -4812,6 +4850,46 @@
Style);
}
+TEST_F(FormatTest, AlwaysBreakAfterDeclarationAndDefinitionReturnTypeMixed) {
+ FormatStyle AfterType = getLLVMStyle();
+ AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_None;
+ AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
+ verifyFormat("void f(void) {\n" // No break here.
+ " f();\n"
+ " f();\n"
+ "}\n"
+ "void bar(void);\n", // No break here.
+ AfterType);
+ AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+ AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
+ verifyFormat("void f(void) {\n" // No break here.
+ " f();\n"
+ " f();\n"
+ "}\n"
+ "void\n"
+ "bar(void);\n", // Break here.
+ AfterType);
+ AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_None;
+ AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
+ verifyFormat("void\n"
+ "f(void) {\n" // Break here.
+ " f();\n"
+ " f();\n"
+ "}\n"
+ "void bar(void);\n", // No break here.
+ AfterType);
+ AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+ AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
+ verifyFormat("void\n"
+ "f(void) {\n" // Break here.
+ " f();\n"
+ " f();\n"
+ "}\n"
+ "void\n"
+ "bar(void);\n", // Break here.
+ AfterType);
+}
+
TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) {
FormatStyle NoBreak = getLLVMStyle();
NoBreak.AlwaysBreakBeforeMultilineStrings = false;
@@ -9829,6 +9907,15 @@
CHECK_PARSE("BreakBeforeBraces: Custom", BreakBeforeBraces,
FormatStyle::BS_Custom);
+ Style.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+ CHECK_PARSE("AlwaysBreakAfterDeclarationReturnType: None",
+ AlwaysBreakAfterDeclarationReturnType, FormatStyle::DRTBS_None);
+ CHECK_PARSE("AlwaysBreakAfterDeclarationReturnType: All",
+ AlwaysBreakAfterDeclarationReturnType, FormatStyle::DRTBS_All);
+ CHECK_PARSE("AlwaysBreakAfterDeclarationReturnType: TopLevel",
+ AlwaysBreakAfterDeclarationReturnType,
+ FormatStyle::DRTBS_TopLevel);
+
Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
CHECK_PARSE("AlwaysBreakAfterDefinitionReturnType: None",
AlwaysBreakAfterDefinitionReturnType, FormatStyle::DRTBS_None);
Index: lib/Format/TokenAnnotator.h
===
--- lib/Format/TokenAnnotator.h
Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.
zturner added a subscriber: zturner. zturner added a comment. What needs to happen for this to go in? If I understand correctly, it is either: 1. Add a new option `TreatDeclarationsLikeDefinitions` 2. Merge this option into `AlwaysBreakAfterDefinitionReturnType` and make it an enum with 5 values. In theory I think option 2 is the best, but are you concerned about backwards compatibility? It breaks anyone already using `AlwaysBreakAfterDefinitionReturnType` and forces them to update their style files. Is this an important consideration? I don't really like option 1 because then we are tied to treating declarations like definitions for all current and future options including ones we haven't imagined yet. That might not be the best choice. Let me know what to do, and I will finish out this patch if strager won't. http://reviews.llvm.org/D10370 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.
djasper added a comment. Maintenance is not about writing 7 lines of code correctly, but ensuring that all these options work correctly and in combination with all other options and that options remain discoverable and well documented. So, please change this to using the one enum with 5 configuration values. http://reviews.llvm.org/D10370 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.
djasper added inline comments. Comment at: docs/ClangFormatStyleOptions.rst:221-235 @@ -220,3 +220,17 @@ -**AlwaysBreakAfterDefinitionReturnType** (``DefinitionReturnTypeBreakingStyle``) +**AlwaysBreakAfterDeclarationReturnType** (``ReturnTypeBreakingStyle``) + The function declaration return type breaking style to use. + + Possible values: + + * ``DRTBS_None`` (in configuration: ``None``) +Break after return type automatically. +``PenaltyReturnTypeOnItsOwnLine`` is taken into account. + * ``DRTBS_All`` (in configuration: ``All``) +Always break after the return type. + * ``DRTBS_TopLevel`` (in configuration: ``TopLevel``) +Always break after the return types of top level functions. + + +**AlwaysBreakAfterDefinitionReturnType** (``ReturnTypeBreakingStyle``) The function definition return type breaking style to use. Same as I am arguing on some of your other patches. Fewer options are easier to maintain and easier to discover. Comment at: include/clang/Format/Format.h:133-136 @@ -128,6 +132,6 @@ /// \brief If \c true, always break before multiline string literals. /// /// This flag is mean to make cases where there are multiple multiline strings /// in a file look more consistent. Thus, it will only take effect if wrapping /// the string at that point leads to it being indented Only one member in style, but parsing the configuration file should be backwards compatible. If both are present in the configuration file, the non-deprecated one should win. There is a few examples of where the configuration options are parsed for backwards compatibility, e.g. DerivePointerBinding. Comment at: lib/Format/ContinuationIndenter.cpp:129-132 @@ -128,5 +128,6 @@ if (Current.is(TT_FunctionDeclarationName) && Style.AlwaysBreakAfterDefinitionReturnType == FormatStyle::DRTBS_None && + Style.AlwaysBreakAfterDeclarationReturnType == FormatStyle::DRTBS_None && State.Column < 6) return false; You mean just the "!Line.Last->isOneOf(tok::semi, tok::comment);"? You can put this into a member function of AnnotatedLine maybe? http://reviews.llvm.org/D10370 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.
strager updated this revision to Diff 35038.
strager added a comment.
Fix missing IsDefinition check in ContinuationIndenter::canBreak.
http://reviews.llvm.org/D10370
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/ContinuationIndenter.cpp
lib/Format/Format.cpp
lib/Format/TokenAnnotator.cpp
lib/Format/TokenAnnotator.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -4658,6 +4658,44 @@
" \"c\";");
}
+TEST_F(FormatTest, DeclarationReturnTypeBreakingStyle) {
+ FormatStyle Style = getLLVMStyle();
+ Style.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_TopLevel;
+ verifyFormat("class C {\n"
+ " int f();\n"
+ "};\n"
+ "int\n"
+ "f();",
+ Style);
+ Style.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+ verifyFormat("class C {\n"
+ " int\n"
+ " f();\n"
+ "};\n"
+ "int\n"
+ "f();",
+ Style);
+ verifyFormat("const char *f(void) { return \"\"; }\n"
+ "const char *\n"
+ "bar(void);\n",
+ Style);
+ verifyFormat("template T *f(T ) { return NULL; }\n"
+ "template \n"
+ "T *\n"
+ "f(T );\n",
+ Style);
+ Style.BreakBeforeBraces = FormatStyle::BS_Stroustrup;
+ verifyFormat("const char *f(void) { return \"\"; }\n"
+ "const char *\n"
+ "bar(void);\n",
+ Style);
+ verifyFormat("template T *f(T ) { return NULL; }\n"
+ "template \n"
+ "T *\n"
+ "f(T );\n",
+ Style);
+}
+
TEST_F(FormatTest, DefinitionReturnTypeBreakingStyle) {
FormatStyle Style = getLLVMStyle();
Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_TopLevel;
@@ -4738,6 +4776,46 @@
Style);
}
+TEST_F(FormatTest, AlwaysBreakAfterDeclarationAndDefinitionReturnTypeMixed) {
+ FormatStyle AfterType = getLLVMStyle();
+ AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_None;
+ AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
+ verifyFormat("void f(void) {\n" // No break here.
+ " f();\n"
+ " f();\n"
+ "}\n"
+ "void bar(void);\n", // No break here.
+ AfterType);
+ AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+ AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
+ verifyFormat("void f(void) {\n" // No break here.
+ " f();\n"
+ " f();\n"
+ "}\n"
+ "void\n"
+ "bar(void);\n", // Break here.
+ AfterType);
+ AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_None;
+ AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
+ verifyFormat("void\n"
+ "f(void) {\n" // Break here.
+ " f();\n"
+ " f();\n"
+ "}\n"
+ "void bar(void);\n", // No break here.
+ AfterType);
+ AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+ AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
+ verifyFormat("void\n"
+ "f(void) {\n" // Break here.
+ " f();\n"
+ " f();\n"
+ "}\n"
+ "void\n"
+ "bar(void);\n", // Break here.
+ AfterType);
+}
+
TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) {
FormatStyle NoBreak = getLLVMStyle();
NoBreak.AlwaysBreakBeforeMultilineStrings = false;
@@ -9409,6 +9487,15 @@
CHECK_PARSE("BreakBeforeBraces: GNU", BreakBeforeBraces, FormatStyle::BS_GNU);
CHECK_PARSE("BreakBeforeBraces: WebKit", BreakBeforeBraces, FormatStyle::BS_WebKit);
+ Style.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+ CHECK_PARSE("AlwaysBreakAfterDeclarationReturnType: None",
+ AlwaysBreakAfterDeclarationReturnType, FormatStyle::DRTBS_None);
+ CHECK_PARSE("AlwaysBreakAfterDeclarationReturnType: All",
+ AlwaysBreakAfterDeclarationReturnType, FormatStyle::DRTBS_All);
+ CHECK_PARSE("AlwaysBreakAfterDeclarationReturnType: TopLevel",
+ AlwaysBreakAfterDeclarationReturnType,
+ FormatStyle::DRTBS_TopLevel);
+
Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
CHECK_PARSE("AlwaysBreakAfterDefinitionReturnType: None",
AlwaysBreakAfterDefinitionReturnType, FormatStyle::DRTBS_None);
Index: lib/Format/TokenAnnotator.h
===
---
Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.
strager added inline comments. Comment at: docs/ClangFormatStyleOptions.rst:221-235 @@ -220,3 +220,17 @@ -**AlwaysBreakAfterDefinitionReturnType** (``DefinitionReturnTypeBreakingStyle``) +**AlwaysBreakAfterDeclarationReturnType** (``ReturnTypeBreakingStyle``) + The function declaration return type breaking style to use. + + Possible values: + + * ``DRTBS_None`` (in configuration: ``None``) +Break after return type automatically. +``PenaltyReturnTypeOnItsOwnLine`` is taken into account. + * ``DRTBS_All`` (in configuration: ``All``) +Always break after the return type. + * ``DRTBS_TopLevel`` (in configuration: ``TopLevel``) +Always break after the return types of top level functions. + + +**AlwaysBreakAfterDefinitionReturnType** (``ReturnTypeBreakingStyle``) The function definition return type breaking style to use. djasper wrote: > Same as I am arguing on some of your other patches. Fewer options are easier > to maintain and easier to discover. I think having separate options for separate cases is easier to maintain. Current method (separate option): ``` FormatStyle::ReturnTypeBreakingStyle BreakStyle = Line.mightBeFunctionDefinition() ? Style.AlwaysBreakAfterDefinitionReturnType : Style.AlwaysBreakAfterDeclarationReturnType; if ((BreakStyle == FormatStyle::DRTBS_All || (BreakStyle == FormatStyle::DRTBS_TopLevel && Line.Level == 0))) Current->MustBreakBefore = true; ``` Proposed method: ``` auto BreakStyle = Style.AlwaysBreakAfterReturnType; if (BreakStyle == FormatStyle::DRTBS_All || (BreakStyle == FormatStyle::DRTBS_TopLevel && Line.Level == 0) || (!Line->mightBeFunctionDefinition() && (BreakStyle == FormatStyle::DRTBS_AllDeclarations) || (BreakStyle == FormatStyle::DRTBS_TopLevelDeclarations && Line.Level == 0))) Current->MustBreakBefore = true; ``` http://reviews.llvm.org/D10370 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.
strager updated this revision to Diff 34947.
strager added a comment.
Rebase.
http://reviews.llvm.org/D10370
Files:
docs/ClangFormatStyleOptions.rst
include/clang/Format/Format.h
lib/Format/ContinuationIndenter.cpp
lib/Format/Format.cpp
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -4658,6 +4658,44 @@
" \"c\";");
}
+TEST_F(FormatTest, DeclarationReturnTypeBreakingStyle) {
+ FormatStyle Style = getLLVMStyle();
+ Style.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_TopLevel;
+ verifyFormat("class C {\n"
+ " int f();\n"
+ "};\n"
+ "int\n"
+ "f();",
+ Style);
+ Style.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+ verifyFormat("class C {\n"
+ " int\n"
+ " f();\n"
+ "};\n"
+ "int\n"
+ "f();",
+ Style);
+ verifyFormat("const char *f(void) { return \"\"; }\n"
+ "const char *\n"
+ "bar(void);\n",
+ Style);
+ verifyFormat("template T *f(T ) { return NULL; }\n"
+ "template \n"
+ "T *\n"
+ "f(T );\n",
+ Style);
+ Style.BreakBeforeBraces = FormatStyle::BS_Stroustrup;
+ verifyFormat("const char *f(void) { return \"\"; }\n"
+ "const char *\n"
+ "bar(void);\n",
+ Style);
+ verifyFormat("template T *f(T ) { return NULL; }\n"
+ "template \n"
+ "T *\n"
+ "f(T );\n",
+ Style);
+}
+
TEST_F(FormatTest, DefinitionReturnTypeBreakingStyle) {
FormatStyle Style = getLLVMStyle();
Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_TopLevel;
@@ -4738,6 +4776,46 @@
Style);
}
+TEST_F(FormatTest, AlwaysBreakAfterDeclarationAndDefinitionReturnTypeMixed) {
+ FormatStyle AfterType = getLLVMStyle();
+ AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_None;
+ AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
+ verifyFormat("void f(void) {\n" // No break here.
+ " f();\n"
+ " f();\n"
+ "}\n"
+ "void bar(void);\n", // No break here.
+ AfterType);
+ AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+ AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
+ verifyFormat("void f(void) {\n" // No break here.
+ " f();\n"
+ " f();\n"
+ "}\n"
+ "void\n"
+ "bar(void);\n", // Break here.
+ AfterType);
+ AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_None;
+ AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
+ verifyFormat("void\n"
+ "f(void) {\n" // Break here.
+ " f();\n"
+ " f();\n"
+ "}\n"
+ "void bar(void);\n", // No break here.
+ AfterType);
+ AfterType.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+ AfterType.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
+ verifyFormat("void\n"
+ "f(void) {\n" // Break here.
+ " f();\n"
+ " f();\n"
+ "}\n"
+ "void\n"
+ "bar(void);\n", // Break here.
+ AfterType);
+}
+
TEST_F(FormatTest, AlwaysBreakBeforeMultilineStrings) {
FormatStyle NoBreak = getLLVMStyle();
NoBreak.AlwaysBreakBeforeMultilineStrings = false;
@@ -9409,6 +9487,15 @@
CHECK_PARSE("BreakBeforeBraces: GNU", BreakBeforeBraces, FormatStyle::BS_GNU);
CHECK_PARSE("BreakBeforeBraces: WebKit", BreakBeforeBraces, FormatStyle::BS_WebKit);
+ Style.AlwaysBreakAfterDeclarationReturnType = FormatStyle::DRTBS_All;
+ CHECK_PARSE("AlwaysBreakAfterDeclarationReturnType: None",
+ AlwaysBreakAfterDeclarationReturnType, FormatStyle::DRTBS_None);
+ CHECK_PARSE("AlwaysBreakAfterDeclarationReturnType: All",
+ AlwaysBreakAfterDeclarationReturnType, FormatStyle::DRTBS_All);
+ CHECK_PARSE("AlwaysBreakAfterDeclarationReturnType: TopLevel",
+ AlwaysBreakAfterDeclarationReturnType,
+ FormatStyle::DRTBS_TopLevel);
+
Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
CHECK_PARSE("AlwaysBreakAfterDefinitionReturnType: None",
AlwaysBreakAfterDefinitionReturnType, FormatStyle::DRTBS_None);
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1593,15 +1593,18 @@
