maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

Better support for constructor initializers. 

We used to format initializers like this (with a sort of hacky implementation):
Constructor()
    : Val1(A),
      Val2(B) {

and now format like this (with a somewhat better solution):
Constructor()
    : Val1(A), Val2(B) {

assuming this would not fit on a single line. Also added tests.

As a side effect we now first analyze whether an UnwrappedLine needs to be
split at all. If not, not splitting it is the best solution by definition. As
this should be a very common case in normal code, not exploring the entire
solution space can provide significant speedup.

llvm-svn: 170457
This commit is contained in:
Daniel Jasper 2012-12-18 21:05:13 +00:00
parent 78eaf05fa7
commit 2af6bbe7e0
2 changed files with 86 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

93
clang/lib/Format/Format.cpp
View File

@ -37,6 +37,7 @@ struct TokenAnnotation {
TT_OverloadedOperator,
TT_PointerOrReference,
TT_ConditionalExpr,
TT_CtorInitializerColon,
TT_LineComment,
TT_BlockComment
};
@ -73,7 +74,6 @@ FormatStyle getGoogleStyle() {
}
struct OptimizationParameters {
unsigned PenaltyExtraLine;
unsigned PenaltyIndentLevel;
};
@ -83,13 +83,9 @@ public:
const UnwrappedLine &Line,
const std::vector<TokenAnnotation> &Annotations,
tooling::Replacements &Replaces, bool StructuralError)
: Style(Style),
SourceMgr(SourceMgr),
Line(Line),
Annotations(Annotations),
Replaces(Replaces),
: Style(Style), SourceMgr(SourceMgr), Line(Line),
Annotations(Annotations), Replaces(Replaces),
StructuralError(StructuralError) {
Parameters.PenaltyExtraLine = 100;
Parameters.PenaltyIndentLevel = 5;
}
@ -100,8 +96,6 @@ public:
// Initialize state dependent on indent.
IndentState State;
State.Column = Indent;
State.CtorInitializerOnNewLine = false;
State.InCtorInitializer = false;
State.ConsumedTokens = 0;
State.Indent.push_back(Indent + 4);
State.LastSpace.push_back(Indent);
@ -110,11 +104,33 @@ public:
// The first token has already been indented and thus consumed.
moveStateToNextToken(State);
// Check whether the UnwrappedLine can be put onto a single line. If so,
// this is bound to be the optimal solution (by definition) and we don't
// need to analyze the entire solution space.
unsigned Columns = State.Column;
bool FitsOnALine = true;
for (unsigned i = 1, n = Line.Tokens.size(); i != n; ++i) {
Columns += (Annotations[i].SpaceRequiredBefore ? 1 : 0) +
Line.Tokens[i].Tok.getLength();
// A special case for the colon of a constructor initializer as this only
// needs to be put on a new line if the line needs to be split.
if (Columns > Style.ColumnLimit ||
(Annotations[i].MustBreakBefore &&
Annotations[i].Type != TokenAnnotation::TT_CtorInitializerColon)) {
FitsOnALine = false;
break;
}
}
// Start iterating at 1 as we have correctly formatted of Token #0 above.
for (unsigned i = 1, n = Line.Tokens.size(); i != n; ++i) {
unsigned NoBreak = calcPenalty(State, false, UINT_MAX);
unsigned Break = calcPenalty(State, true, NoBreak);
addTokenToState(Break < NoBreak, false, State);
if (FitsOnALine) {
addTokenToState(false, false, State);
} else {
unsigned NoBreak = calcPenalty(State, false, UINT_MAX);
unsigned Break = calcPenalty(State, true, NoBreak);
addTokenToState(Break < NoBreak, false, State);
}
}
}
@ -146,9 +162,6 @@ private:
/// on a level.
std::vector<unsigned> FirstLessLess;
bool CtorInitializerOnNewLine;
bool InCtorInitializer;
/// \brief Comparison operator to be able to used \c IndentState in \c map.
bool operator<(const IndentState &Other) const {
if (Other.ConsumedTokens != ConsumedTokens)
@ -212,11 +225,8 @@ private:
State.LastSpace[ParenLevel] = State.Indent[ParenLevel];
if (Current.Tok.is(tok::colon) &&
Annotations[Index].Type != TokenAnnotation::TT_ConditionalExpr) {
Annotations[Index].Type != TokenAnnotation::TT_ConditionalExpr)
State.Indent[ParenLevel] += 2;
State.CtorInitializerOnNewLine = true;
State.InCtorInitializer = true;
}
} else {
unsigned Spaces = Annotations[Index].SpaceRequiredBefore ? 1 : 0;
if (Annotations[Index].Type == TokenAnnotation::TT_LineComment)
@ -228,10 +238,7 @@ private:
if (Previous.Tok.is(tok::l_paren) ||
Annotations[Index - 1].Type == TokenAnnotation::TT_TemplateOpener)
State.Indent[ParenLevel] = State.Column;
if (Current.Tok.is(tok::colon)) {
State.Indent[ParenLevel] = State.Column + 3;
State.InCtorInitializer = true;
}
// Top-level spaces are exempt as that mostly leads to better results.
State.Column += Spaces;
if (Spaces > 0 && ParenLevel != 0)
@ -279,10 +286,8 @@ private:
"Tried to calculate penalty for splitting after the last token");
const FormatToken &Left = Line.Tokens[Index];
const FormatToken &Right = Line.Tokens[Index + 1];
if (Left.Tok.is(tok::semi))
if (Left.Tok.is(tok::semi) || Left.Tok.is(tok::comma))
return 0;
if (Left.Tok.is(tok::comma))
return 1;
if (Left.Tok.is(tok::equal) || Left.Tok.is(tok::l_paren) ||
Left.Tok.is(tok::pipepipe) || Left.Tok.is(tok::ampamp))
return 2;
@ -313,18 +318,10 @@ private:
if (NewLine && !Annotations[State.ConsumedTokens].CanBreakBefore)
return UINT_MAX;
if (State.ConsumedTokens > 0 && !NewLine &&
State.CtorInitializerOnNewLine &&
Line.Tokens[State.ConsumedTokens - 1].Tok.is(tok::comma))
return UINT_MAX;
if (NewLine && State.InCtorInitializer && !State.CtorInitializerOnNewLine)
return UINT_MAX;
unsigned CurrentPenalty = 0;
if (NewLine) {
CurrentPenalty += Parameters.PenaltyIndentLevel * State.Indent.size() +
Parameters.PenaltyExtraLine + splitPenalty(State.ConsumedTokens - 1);
splitPenalty(State.ConsumedTokens - 1);
}
addTokenToState(NewLine, true, State);
@ -413,9 +410,7 @@ class TokenAnnotator {
public:
TokenAnnotator(const UnwrappedLine &Line, const FormatStyle &Style,
SourceManager &SourceMgr)
: Line(Line),
Style(Style),
SourceMgr(SourceMgr) {
: Line(Line), Style(Style), SourceMgr(SourceMgr) {
}
/// \brief A parser that gathers additional information about tokens.
@ -427,9 +422,7 @@ public:
public:
AnnotatingParser(const SmallVector<FormatToken, 16> &Tokens,
std::vector<TokenAnnotation> &Annotations)
: Tokens(Tokens),
Annotations(Annotations),
Index(0) {
: Tokens(Tokens), Annotations(Annotations), Index(0) {
}
bool parseAngle() {
@ -496,6 +489,10 @@ public:
switch (Tokens[CurrentIndex].Tok.getKind()) {
case tok::l_paren:
parseParens();
if (Index < Tokens.size() && Tokens[Index].Tok.is(tok::colon)) {
Annotations[Index].Type = TokenAnnotation::TT_CtorInitializerColon;
next();
}
break;
case tok::l_square:
parseSquare();
@ -557,7 +554,10 @@ public:
Annotation.CanBreakBefore =
canBreakBetween(Line.Tokens[i - 1], Line.Tokens[i]);
if (Line.Tokens[i].Tok.is(tok::colon)) {
if (Annotation.Type == TokenAnnotation::TT_CtorInitializerColon) {
Annotation.MustBreakBefore = true;
Annotation.SpaceRequiredBefore = true;
} else if (Line.Tokens[i].Tok.is(tok::colon)) {
Annotation.SpaceRequiredBefore =
Line.Tokens[0].Tok.isNot(tok::kw_case) && i != e - 1;
} else if (Annotations[i - 1].Type == TokenAnnotation::TT_UnaryOperator) {
@ -769,9 +769,7 @@ private:
class LexerBasedFormatTokenSource : public FormatTokenSource {
public:
LexerBasedFormatTokenSource(Lexer &Lex, SourceManager &SourceMgr)
: GreaterStashed(false),
Lex(Lex),
SourceMgr(SourceMgr),
: GreaterStashed(false), Lex(Lex), SourceMgr(SourceMgr),
IdentTable(Lex.getLangOpts()) {
Lex.SetKeepWhitespaceMode(true);
}
@ -831,10 +829,7 @@ class Formatter : public UnwrappedLineConsumer {
public:
Formatter(const FormatStyle &Style, Lexer &Lex, SourceManager &SourceMgr,
const std::vector<CharSourceRange> &Ranges)
: Style(Style),
Lex(Lex),
SourceMgr(SourceMgr),
Ranges(Ranges),
: Style(Style), Lex(Lex), SourceMgr(SourceMgr), Ranges(Ranges),
StructuralError(false) {
}

52
clang/unittests/Format/FormatTest.cpp
View File

@ -374,6 +374,41 @@ TEST_F(FormatTest, FormatsAwesomeMethodCall) {
" parameter, parameter, parameter)), SecondLongCall(parameter));");
}
TEST_F(FormatTest, ConstructorInitializers) {
verifyFormat("Constructor() : Initializer(FitsOnTheLine) {\n}");
verifyFormat(
"SomeClass::Constructor()\n"
" : aaaaaaaaaaaaa(aaaaaaaaaaaaaa), aaaaaaaaaaaaaaa(aaaaaaaaaaaa) {\n"
"}");
verifyFormat(
"SomeClass::Constructor()\n"
" : aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa),\n"
" aaaaaaaaaaaaaaa(aaaaaaaaaaaa) {\n"
"}");
verifyFormat("Constructor()\n"
" : aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaa),\n"
" aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaaaaa,\n"
" aaaaaaaaaaaaaaaaaaaaaaaaaaa),\n"
" aaaaaaaaaaaaaaaaaaaaaaa() {\n"
"}");
// Here a line could be saved by splitting the second initializer onto two
// lines, but that is not desireable.
verifyFormat("Constructor()\n"
" : aaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaaa),\n"
" aaaaaaaaaaa(aaaaaaaaaaa),\n"
" aaaaaaaaaaaaaaaaaaaaat(aaaaaaaaaaaaaaaaaaaaaaaaaaaa) {\n"
"}");
verifyGoogleFormat("MyClass::MyClass(int var)\n"
" : some_var_(var), // 4 space indent\n"
" some_other_var_(var + 1) { // lined up\n"
"}");
}
TEST_F(FormatTest, BreaksAsHighAsPossible) {
verifyFormat(
"if ((aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa && aaaaaaaaaaaaaaaaaaaaaaaaaa) ||\n"
@ -461,13 +496,11 @@ TEST_F(FormatTest, UnderstandsEquals) {
}
TEST_F(FormatTest, WrapsAtFunctionCallsIfNecessary) {
verifyFormat(
"LoooooooooooooooooooooooooooooooooooooongObject\n"
" .looooooooooooooooooooooooooooooooooooooongFunction();");
verifyFormat("LoooooooooooooooooooooooooooooooooooooongObject\n"
" .looooooooooooooooooooooooooooooooooooooongFunction();");
verifyFormat(
"LoooooooooooooooooooooooooooooooooooooongObject\n"
" ->looooooooooooooooooooooooooooooooooooooongFunction();");
verifyFormat("LoooooooooooooooooooooooooooooooooooooongObject\n"
" ->looooooooooooooooooooooooooooooooooooooongFunction();");
verifyFormat(
"LooooooooooooooooooooooooooooooooongObject->shortFunction(Parameter1,\n"
@ -485,10 +518,9 @@ TEST_F(FormatTest, WrapsAtFunctionCallsIfNecessary) {
"function(LoooooooooooooooooooooooooooooooooooongObject\n"
" ->loooooooooooooooooooooooooooooooooooooooongFunction());");
verifyFormat(
"if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaa) ||\n"
" aaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {\n"
"}");
verifyFormat("if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaa) ||\n"
" aaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {\n"
"}");
}
TEST_F(FormatTest, UnderstandsTemplateParameters) {