r178641 - Improve formatting of for loops and multi-variable DeclStmts.
Daniel Jasper
djasper at google.com
Wed Apr 3 06:36:18 PDT 2013
Author: djasper
Date: Wed Apr 3 08:36:17 2013
New Revision: 178641
URL: http://llvm.org/viewvc/llvm-project?rev=178641&view=rev
Log:
Improve formatting of for loops and multi-variable DeclStmts.
This combines several related changes:
a) Don't break before after the variable types in for loops with a
single variable.
b) Better indent DeclStmts defining multiple variables.
Before:
bool aaaaaaaaaaaaaaaaaaaaaaaaa =
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaa),
bbbbbbbbbbbbbbbbbbbbbbbbb =
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(bbbbbbbbbbbbbbbb);
for (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaa = aaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaa;
aaaaaaaaaaa != aaaaaaaaaaaaaaaaaaa; ++aaaaaaaaaaa) {
}
After:
bool aaaaaaaaaaaaaaaaaaaaaaaaa =
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaa),
bbbbbbbbbbbbbbbbbbbbbbbbb =
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(bbbbbbbbbbbbbbbb);
for (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaa =
aaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaa;
aaaaaaaaaaa != aaaaaaaaaaaaaaaaaaa; ++aaaaaaaaaaa) {
}
Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/lib/Format/TokenAnnotator.h
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=178641&r1=178640&r2=178641&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Apr 3 08:36:17 2013
@@ -487,7 +487,6 @@ public:
State.Stack.push_back(
ParenState(FirstIndent, FirstIndent, !Style.BinPackParameters,
/*HasMultiParameterLine=*/ false));
- State.VariablePos = 0;
State.LineContainsContinuedForLoopSection = false;
State.ParenLevel = 0;
State.StartOfStringLiteral = 0;
@@ -537,7 +536,7 @@ private:
AvoidBinPacking(AvoidBinPacking), BreakBeforeParameter(false),
HasMultiParameterLine(HasMultiParameterLine), ColonPos(0),
StartOfFunctionCall(0), NestedNameSpecifierContinuation(0),
- CallContinuation(0) {}
+ CallContinuation(0), VariablePos(0) {}
/// \brief The position to which a specific parenthesis level needs to be
/// indented.
@@ -591,6 +590,11 @@ private:
/// contains the start column of the second line. Otherwise 0.
unsigned CallContinuation;
+ /// \brief The column of the first variable name in a variable declaration.
+ ///
+ /// Used to align further variables if necessary.
+ unsigned VariablePos;
+
bool operator<(const ParenState &Other) const {
if (Indent != Other.Indent)
return Indent < Other.Indent;
@@ -618,6 +622,8 @@ private:
Other.NestedNameSpecifierContinuation;
if (CallContinuation != Other.CallContinuation)
return CallContinuation < Other.CallContinuation;
+ if (VariablePos != Other.VariablePos)
+ return VariablePos < Other.VariablePos;
return false;
}
};
@@ -632,11 +638,6 @@ private:
/// \brief The token that needs to be next formatted.
const AnnotatedToken *NextToken;
- /// \brief The column of the first variable name in a variable declaration.
- ///
- /// Used to align further variables if necessary.
- unsigned VariablePos;
-
/// \brief \c true if this line contains a continued for-loop section.
bool LineContainsContinuedForLoopSection;
@@ -660,8 +661,6 @@ private:
return NextToken < Other.NextToken;
if (Column != Other.Column)
return Column < Other.Column;
- if (VariablePos != Other.VariablePos)
- return VariablePos < Other.VariablePos;
if (LineContainsContinuedForLoopSection !=
Other.LineContainsContinuedForLoopSection)
return LineContainsContinuedForLoopSection;
@@ -727,10 +726,9 @@ private:
}
} else if (Current.Type == TT_ConditionalExpr) {
State.Column = State.Stack.back().QuestionColumn;
- } else if (Previous.is(tok::comma) && State.VariablePos != 0 &&
- ((RootToken.is(tok::kw_for) && State.ParenLevel == 1) ||
- State.ParenLevel == 0)) {
- State.Column = State.VariablePos;
+ } else if (Previous.is(tok::comma) &&
+ State.Stack.back().VariablePos != 0) {
+ State.Column = State.Stack.back().VariablePos;
} else if (Previous.ClosesTemplateDeclaration ||
(Current.Type == TT_StartOfName && State.ParenLevel == 0)) {
State.Column = State.Stack.back().Indent;
@@ -799,10 +797,13 @@ private:
State.Stack.back().BreakBeforeParameter = true;
}
} else {
- // FIXME: Put VariablePos into ParenState and remove second part of if().
if (Current.is(tok::equal) &&
- (RootToken.is(tok::kw_for) || State.ParenLevel == 0))
- State.VariablePos = State.Column - Previous.FormatTok.TokenLength;
+ (RootToken.is(tok::kw_for) || State.ParenLevel == 0)) {
+ State.Stack.back().VariablePos =
+ State.Column - Previous.FormatTok.TokenLength;
+ if (Previous.PartOfMultiVariableDeclStmt)
+ State.Stack.back().LastSpace = State.Stack.back().VariablePos;
+ }
unsigned Spaces = State.NextToken->SpacesRequiredBefore;
@@ -828,7 +829,7 @@ private:
State.Stack.back().HasMultiParameterLine = true;
State.Column += Spaces;
- if (Current.is(tok::l_paren) && Previous.is(tok::kw_if))
+ if (Current.is(tok::l_paren) && Previous.isOneOf(tok::kw_if, tok::kw_for))
// Treat the condition inside an if as if it was a second function
// parameter, i.e. let nested calls have an indent of 4.
State.Stack.back().LastSpace = State.Column + 1; // 1 is length of "(".
@@ -926,9 +927,11 @@ private:
}
// Remove scopes created by fake parenthesis.
+ unsigned VariablePos = State.Stack.back().VariablePos;
for (unsigned i = 0, e = Current.FakeRParens; i != e; ++i) {
State.Stack.pop_back();
}
+ State.Stack.back().VariablePos = VariablePos;
if (Current.is(tok::string_literal)) {
State.StartOfStringLiteral = State.Column;
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=178641&r1=178640&r2=178641&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Wed Apr 3 08:36:17 2013
@@ -408,6 +408,10 @@ private:
Tok->FormatTok.Tok.getIdentifierInfo() == &Ident_in)
Tok->Type = TT_ObjCForIn;
break;
+ case tok::comma:
+ if (Contexts.back().FirstStartOfName)
+ Contexts.back().FirstStartOfName->PartOfMultiVariableDeclStmt = true;
+ break;
default:
break;
}
@@ -538,7 +542,8 @@ private:
: ContextKind(ContextKind), BindingStrength(BindingStrength),
LongestObjCSelectorName(0), ColonIsForRangeExpr(false),
ColonIsObjCMethodExpr(false), FirstObjCSelectorName(NULL),
- IsExpression(IsExpression), CanBeExpression(true) {}
+ FirstStartOfName(NULL), IsExpression(IsExpression),
+ CanBeExpression(true) {}
tok::TokenKind ContextKind;
unsigned BindingStrength;
@@ -546,6 +551,7 @@ private:
bool ColonIsForRangeExpr;
bool ColonIsObjCMethodExpr;
AnnotatedToken *FirstObjCSelectorName;
+ AnnotatedToken *FirstStartOfName;
bool IsExpression;
bool CanBeExpression;
};
@@ -600,8 +606,10 @@ private:
((Current.Parent->is(tok::identifier) &&
Current.Parent->FormatTok.Tok.getIdentifierInfo()
->getPPKeywordID() == tok::pp_not_keyword) ||
+ isSimpleTypeSpecifier(*Current.Parent) ||
Current.Parent->Type == TT_PointerOrReference ||
Current.Parent->Type == TT_TemplateCloser)) {
+ Contexts.back().FirstStartOfName = &Current;
Current.Type = TT_StartOfName;
} else if (Current.isOneOf(tok::star, tok::amp, tok::ampamp)) {
Current.Type =
@@ -720,6 +728,39 @@ private:
return TT_UnaryOperator;
}
+ // FIXME: This is copy&pasted from Sema. Put it in a common place and remove
+ // duplication.
+ /// \brief Determine whether the token kind starts a simple-type-specifier.
+ bool isSimpleTypeSpecifier(const AnnotatedToken &Tok) const {
+ switch (Tok.FormatTok.Tok.getKind()) {
+ case tok::kw_short:
+ case tok::kw_long:
+ case tok::kw___int64:
+ case tok::kw___int128:
+ case tok::kw_signed:
+ case tok::kw_unsigned:
+ case tok::kw_void:
+ case tok::kw_char:
+ case tok::kw_int:
+ case tok::kw_half:
+ case tok::kw_float:
+ case tok::kw_double:
+ case tok::kw_wchar_t:
+ case tok::kw_bool:
+ case tok::kw___underlying_type:
+ return true;
+ case tok::annot_typename:
+ case tok::kw_char16_t:
+ case tok::kw_char32_t:
+ case tok::kw_typeof:
+ case tok::kw_decltype:
+ return Lex.getLangOpts().CPlusPlus;
+ default:
+ break;
+ }
+ return false;
+ }
+
SmallVector<Context, 8> Contexts;
SourceManager &SourceMgr;
@@ -886,7 +927,7 @@ unsigned TokenAnnotator::splitPenalty(co
const AnnotatedToken &Right = Tok;
if (Right.Type == TT_StartOfName) {
- if (Line.First.is(tok::kw_for))
+ if (Line.First.is(tok::kw_for) && Right.PartOfMultiVariableDeclStmt)
return 3;
else if (Line.MightBeFunctionDecl && Right.BindingStrength == 1)
// FIXME: Clean up hack of using BindingStrength to find top-level names.
Modified: cfe/trunk/lib/Format/TokenAnnotator.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.h?rev=178641&r1=178640&r2=178641&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.h (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.h Wed Apr 3 08:36:17 2013
@@ -76,8 +76,8 @@ public:
ClosesTemplateDeclaration(false), MatchingParen(NULL),
ParameterCount(0), BindingStrength(0), SplitPenalty(0),
LongestObjCSelectorName(0), Parent(NULL), FakeLParens(0),
- FakeRParens(0), LastInChainOfCalls(false) {
- }
+ FakeRParens(0), LastInChainOfCalls(false),
+ PartOfMultiVariableDeclStmt(false) {}
bool is(tok::TokenKind Kind) const { return FormatTok.Tok.is(Kind); }
@@ -166,6 +166,11 @@ public:
/// \brief Is this the last "." or "->" in a builder-type call?
bool LastInChainOfCalls;
+ /// \brief Is this token part of a \c DeclStmt defining multiple variables?
+ ///
+ /// Only set if \c Type == \c TT_StartOfName.
+ bool PartOfMultiVariableDeclStmt;
+
const AnnotatedToken *getPreviousNoneComment() const {
AnnotatedToken *Tok = Parent;
while (Tok != NULL && Tok->is(tok::comment))
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=178641&r1=178640&r2=178641&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Apr 3 08:36:17 2013
@@ -311,8 +311,8 @@ TEST_F(FormatTest, FormatsForLoop) {
verifyFormat(
"for (MachineFun::iterator IIII = PrevIt, EEEE = F.end(); IIII != EEEE;\n"
" ++IIIII) {\n}");
- verifyFormat("for (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
- " aaaaaaaaaaa = aaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaa;\n"
+ verifyFormat("for (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaa =\n"
+ " aaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaa;\n"
" aaaaaaaaaaa != aaaaaaaaaaaaaaaaaaa; ++aaaaaaaaaaa) {\n}");
verifyFormat("for (llvm::ArrayRef<NamedDecl *>::iterator\n"
" I = FD->getDeclsInPrototypeScope().begin(),\n"
@@ -329,6 +329,10 @@ TEST_F(FormatTest, FormatsForLoop) {
verifyFormat("for (int aaaaaaaaaaa = 1; aaaaaaaaaaa <= bbbbbbbbbbbbbbb;\n"
" aaaaaaaaaaa++, bbbbbbbbbbbbbbbbb++) {\n"
"}");
+ verifyFormat("for (some_namespace::SomeIterator iter( // force break\n"
+ " aaaaaaaaaa);\n"
+ " iter; ++iter) {\n"
+ "}");
FormatStyle NoBinPacking = getLLVMStyle();
NoBinPacking.BinPackParameters = false;
@@ -1188,8 +1192,9 @@ TEST_F(FormatTest, FormatsSmallMacroDefi
}
TEST_F(FormatTest, DoesNotBreakPureVirtualFunctionDefinition) {
- verifyFormat("virtual void write(ELFWriter *writerrr,\n"
- " OwningPtr<FileOutputBuffer> &buffer) = 0;");
+ verifyFormat(
+ "virtual void\n"
+ "write(ELFWriter *writerrr, OwningPtr<FileOutputBuffer> &buffer) = 0;");
}
TEST_F(FormatTest, LayoutUnknownPPDirective) {
@@ -1378,7 +1383,7 @@ TEST_F(FormatTest, MixingPreprocessorDir
TEST_F(FormatTest, LayoutStatementsAroundPreprocessorDirectives) {
EXPECT_EQ("int\n"
"#define A\n"
- " a;",
+ "a;",
format("int\n#define A\na;"));
verifyFormat("functionCallTo(\n"
" someOtherFunction(\n"
@@ -1613,7 +1618,7 @@ TEST_F(FormatTest, BreaksDesireably) {
}
TEST_F(FormatTest, FormatsOneParameterPerLineIfNecessary) {
- FormatStyle NoBinPacking = getLLVMStyle();
+ FormatStyle NoBinPacking = getGoogleStyle();
NoBinPacking.BinPackParameters = false;
verifyFormat("f(aaaaaaaaaaaaaaaaaaaa,\n"
" aaaaaaaaaaaaaaaaaaaa,\n"
@@ -1851,14 +1856,13 @@ TEST_F(FormatTest, DeclarationsOfMultipl
" aaaaaaaaaaa = aaaaaa->aaaaaaaaaaa();");
verifyFormat("bool a = true, b = false;");
- // FIXME: Indentation looks weird.
verifyFormat("bool aaaaaaaaaaaaaaaaaaaaaaaaa =\n"
- " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaa),\n"
+ " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaa),\n"
" bbbbbbbbbbbbbbbbbbbbbbbbb =\n"
" bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb(bbbbbbbbbbbbbbbb);");
verifyFormat(
"bool aaaaaaaaaaaaaaaaaaaaa =\n"
- " bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb && cccccccccccccccccccccccccccc,\n"
+ " bbbbbbbbbbbbbbbbbbbbbbbbbbbb && cccccccccccccccccccccccccccc,\n"
" d = e && f;");
}
@@ -2051,7 +2055,7 @@ TEST_F(FormatTest, WrapsTemplateDeclarat
" aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);");
verifyFormat("template <typename T>\n"
"void aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
- " int aaaaaaaaaaaaaaaaa);");
+ " int aaaaaaaaaaaaaaaaaaaaaa);");
verifyFormat(
"template <typename T1, typename T2 = char, typename T3 = char,\n"
" typename T4 = char>\n"
More information about the cfe-commits
mailing list