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("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa __attribute__((unused))\n"
"aaaaaaaaaaaaaaaaaaaaaaa(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",
+ AlwaysBreakAfterReturnType,
+ FormatStyle::RTBS_TopLevelDefinitions);
+
Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
CHECK_PARSE("AlwaysBreakAfterDefinitionReturnType: None",
AlwaysBreakAfterDefinitionReturnType, FormatStyle::DRTBS_None);
Index: lib/Format/TokenAnnotator.h
===================================================================
--- lib/Format/TokenAnnotator.h
+++ lib/Format/TokenAnnotator.h
@@ -86,6 +86,15 @@
return startsWith(First, Tokens...);
}
+ /// \c true if this line looks like a function definition instead of a
+ /// function declaration. Asserts MightBeFunctionDecl.
+ bool mightBeFunctionDefinition() const {
+ assert(MightBeFunctionDecl);
+ // FIXME: Line.Last points to other characters than tok::semi
+ // and tok::lbrace.
+ return !Last->isOneOf(tok::semi, tok::comment);
+ }
+
FormatToken *First;
FormatToken *Last;
@@ -156,6 +165,9 @@
bool canBreakBefore(const AnnotatedLine &Line, const FormatToken &Right);
+ bool mustBreakForReturnType(const AnnotatedLine &Line,
+ FormatToken &Token) const;
+
void printDebugInfo(const AnnotatedLine &Line);
void calculateUnbreakableTailLengths(AnnotatedLine &Line);
Index: lib/Format/TokenAnnotator.cpp
===================================================================
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1562,6 +1562,31 @@
return false;
}
+bool TokenAnnotator::mustBreakForReturnType(const AnnotatedLine &Line,
+ FormatToken &Token) const {
+ assert(Line.MightBeFunctionDecl);
+
+ if (Line.MightBeFunctionDecl &&
+ (Style.AlwaysBreakAfterReturnType == FormatStyle::RTBS_TopLevel ||
+ Style.AlwaysBreakAfterReturnType ==
+ FormatStyle::RTBS_TopLevelDefinitions) &&
+ Line.Level > 0)
+ return false;
+
+ switch (Style.AlwaysBreakAfterReturnType) {
+ case FormatStyle::RTBS_None:
+ return false;
+ case FormatStyle::RTBS_All:
+ case FormatStyle::RTBS_TopLevel:
+ return true;
+ case FormatStyle::RTBS_AllDefinitions:
+ case FormatStyle::RTBS_TopLevelDefinitions:
+ return Line.mightBeFunctionDefinition();
+ }
+
+ return false;
+}
+
void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) {
for (SmallVectorImpl<AnnotatedLine *>::iterator I = Line.Children.begin(),
E = Line.Children.end();
@@ -1613,15 +1638,10 @@
Current->MustBreakBefore =
Current->MustBreakBefore || mustBreakBefore(Line, *Current);
- if ((Style.AlwaysBreakAfterDefinitionReturnType == FormatStyle::DRTBS_All ||
- (Style.AlwaysBreakAfterDefinitionReturnType ==
- FormatStyle::DRTBS_TopLevel &&
- Line.Level == 0)) &&
- InFunctionDecl && Current->is(TT_FunctionDeclarationName) &&
- !Line.Last->isOneOf(tok::semi, tok::comment)) // Only for definitions.
- // FIXME: Line.Last points to other characters than tok::semi
- // and tok::lbrace.
- Current->MustBreakBefore = true;
+ if (!Current->MustBreakBefore && InFunctionDecl &&
+ Current->is(TT_FunctionDeclarationName)) {
+ Current->MustBreakBefore = mustBreakForReturnType(Line, *Current);
+ }
Current->CanBreakBefore =
Current->MustBreakBefore || canBreakBefore(Line, *Current);
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -105,6 +105,18 @@
};
template <>
+struct ScalarEnumerationTraits<FormatStyle::ReturnTypeBreakingStyle> {
+ static void enumeration(IO &IO, FormatStyle::ReturnTypeBreakingStyle &Value) {
+ IO.enumCase(Value, "None", FormatStyle::RTBS_None);
+ IO.enumCase(Value, "All", FormatStyle::RTBS_All);
+ IO.enumCase(Value, "TopLevel", FormatStyle::RTBS_TopLevel);
+ IO.enumCase(Value, "TopLevelDefinitions",
+ FormatStyle::RTBS_TopLevelDefinitions);
+ IO.enumCase(Value, "AllDefinitions", FormatStyle::RTBS_AllDefinitions);
+ }
+};
+
+template <>
struct ScalarEnumerationTraits<FormatStyle::DefinitionReturnTypeBreakingStyle> {
static void
enumeration(IO &IO, FormatStyle::DefinitionReturnTypeBreakingStyle &Value) {
@@ -233,6 +245,21 @@
Style.AllowShortLoopsOnASingleLine);
IO.mapOptional("AlwaysBreakAfterDefinitionReturnType",
Style.AlwaysBreakAfterDefinitionReturnType);
+ IO.mapOptional("AlwaysBreakAfterReturnType",
+ Style.AlwaysBreakAfterReturnType);
+ // If AlwaysBreakAfterDefinitionReturnType was specified but
+ // AlwaysBreakAfterReturnType was not, initialize the latter from the
+ // former for backwards compatibility.
+ if (Style.AlwaysBreakAfterDefinitionReturnType != FormatStyle::DRTBS_None &&
+ Style.AlwaysBreakAfterReturnType == FormatStyle::RTBS_None) {
+ if (Style.AlwaysBreakAfterDefinitionReturnType == FormatStyle::DRTBS_All)
+ Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_AllDefinitions;
+ else if (Style.AlwaysBreakAfterDefinitionReturnType ==
+ FormatStyle::DRTBS_TopLevel)
+ Style.AlwaysBreakAfterReturnType =
+ FormatStyle::RTBS_TopLevelDefinitions;
+ }
+
IO.mapOptional("AlwaysBreakBeforeMultilineStrings",
Style.AlwaysBreakBeforeMultilineStrings);
IO.mapOptional("AlwaysBreakTemplateDeclarations",
@@ -449,6 +476,7 @@
LLVMStyle.AllowShortCaseLabelsOnASingleLine = false;
LLVMStyle.AllowShortIfStatementsOnASingleLine = false;
LLVMStyle.AllowShortLoopsOnASingleLine = false;
+ LLVMStyle.AlwaysBreakAfterReturnType = FormatStyle::RTBS_None;
LLVMStyle.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_None;
LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
LLVMStyle.AlwaysBreakTemplateDeclarations = false;
@@ -585,6 +613,8 @@
FormatStyle MozillaStyle = getLLVMStyle();
MozillaStyle.AllowAllParametersOfDeclarationOnNextLine = false;
MozillaStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
+ MozillaStyle.AlwaysBreakAfterReturnType =
+ FormatStyle::RTBS_TopLevelDefinitions;
MozillaStyle.AlwaysBreakAfterDefinitionReturnType =
FormatStyle::DRTBS_TopLevel;
MozillaStyle.AlwaysBreakTemplateDeclarations = true;
@@ -624,6 +654,7 @@
FormatStyle getGNUStyle() {
FormatStyle Style = getLLVMStyle();
Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
+ Style.AlwaysBreakAfterReturnType = FormatStyle::RTBS_AllDefinitions;
Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
Style.BreakBeforeBraces = FormatStyle::BS_GNU;
Style.BreakBeforeTernaryOperators = true;
Index: lib/Format/ContinuationIndenter.cpp
===================================================================
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -125,10 +125,10 @@
// Don't break after very short return types (e.g. "void") as that is often
// unexpected.
- if (Current.is(TT_FunctionDeclarationName) &&
- Style.AlwaysBreakAfterDefinitionReturnType == FormatStyle::DRTBS_None &&
- State.Column < 6)
- return false;
+ if (Current.is(TT_FunctionDeclarationName) && State.Column < 6) {
+ if (Style.AlwaysBreakAfterReturnType == FormatStyle::RTBS_None)
+ return false;
+ }
return !State.Stack.back().NoLineBreak;
}
Index: include/clang/Format/Format.h
===================================================================
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -160,9 +160,29 @@
DRTBS_TopLevel,
};
- /// \brief The function definition return type breaking style to use.
+ /// \brief Different ways to break after the function definition or
+ /// declaration return type.
+ enum ReturnTypeBreakingStyle {
+ /// Break after return type automatically.
+ /// \c PenaltyReturnTypeOnItsOwnLine is taken into account.
+ RTBS_None,
+ /// Always break after the return type.
+ RTBS_All,
+ /// Always break after the return types of top level functions.
+ RTBS_TopLevel,
+ /// Always break after the return type of function definitions.
+ RTBS_AllDefinitions,
+ /// Always break after the return type of top-level definitions.
+ RTBS_TopLevelDefinitions,
+ };
+
+ /// \brief The function definition return type breaking style to use. This
+ /// option is deprecated and is retained for backwards compatibility.
DefinitionReturnTypeBreakingStyle AlwaysBreakAfterDefinitionReturnType;
+ /// \brief The function declaration return type breaking style to use.
+ ReturnTypeBreakingStyle AlwaysBreakAfterReturnType;
+
/// \brief If \c true, always break before multiline string literals.
///
/// This flag is mean to make cases where there are multiple multiline strings
@@ -582,8 +602,7 @@
AllowShortIfStatementsOnASingleLine ==
R.AllowShortIfStatementsOnASingleLine &&
AllowShortLoopsOnASingleLine == R.AllowShortLoopsOnASingleLine &&
- AlwaysBreakAfterDefinitionReturnType ==
- R.AlwaysBreakAfterDefinitionReturnType &&
+ AlwaysBreakAfterReturnType == R.AlwaysBreakAfterReturnType &&
AlwaysBreakBeforeMultilineStrings ==
R.AlwaysBreakBeforeMultilineStrings &&
AlwaysBreakTemplateDeclarations ==
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
