[cfe-commits] r171974 - in /cfe/trunk: lib/Format/Format.cpp lib/Format/UnwrappedLineParser.cpp lib/Format/UnwrappedLineParser.h unittests/Format/FormatTest.cpp
Manuel Klimek
klimek at google.com
Wed Jan 9 07:25:02 PST 2013
Author: klimek
Date: Wed Jan 9 09:25:02 2013
New Revision: 171974
URL: http://llvm.org/viewvc/llvm-project?rev=171974&view=rev
Log:
Enables layouting unwrapped lines around preprocessor directives.
Previously, we'd always start at indent level 0 after a preprocessor
directive, now we layout the following snippet (column limit 69) as
follows:
functionCallTo(someOtherFunction(
withSomeParameters, whichInSequence,
areLongerThanALine(andAnotherCall,
B
withMoreParamters,
whichStronglyInfluenceTheLayout),
andMoreParameters),
trailing);
Note that the different jumping indent is a different issue that will be
addressed separately.
This is the first step towards handling #ifdef->#else->#endif chains
correctly.
Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.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=171974&r1=171973&r2=171974&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Jan 9 09:25:02 2013
@@ -803,18 +803,21 @@
void calculateExtraInformation(AnnotatedToken &Current) {
Current.SpaceRequiredBefore = spaceRequiredBefore(Current);
- if (Current.Type == TT_CtorInitializerColon || Current.Parent->Type ==
- TT_LineComment || (Current.is(tok::string_literal) &&
- Current.Parent->is(tok::string_literal))) {
- Current.MustBreakBefore = true;
- } else if (Current.is(tok::at) && Current.Parent->Parent->is(tok::at)) {
- // Don't put two objc's '@' on the same line. This could happen,
- // as in, @optional @property ...
+ if (Current.FormatTok.MustBreakBefore) {
Current.MustBreakBefore = true;
} else {
- Current.MustBreakBefore = false;
+ if (Current.Type == TT_CtorInitializerColon || Current.Parent->Type ==
+ TT_LineComment || (Current.is(tok::string_literal) &&
+ Current.Parent->is(tok::string_literal))) {
+ Current.MustBreakBefore = true;
+ } else if (Current.is(tok::at) && Current.Parent->Parent->is(tok::at)) {
+ // Don't put two objc's '@' on the same line. This could happen,
+ // as in, @optional @property ...
+ Current.MustBreakBefore = true;
+ } else {
+ Current.MustBreakBefore = false;
+ }
}
-
Current.CanBreakBefore = Current.MustBreakBefore || canBreakBefore(Current);
if (!Current.Children.empty())
Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=171974&r1=171973&r2=171974&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Wed Jan 9 09:25:02 2013
@@ -74,8 +74,9 @@
UnwrappedLineParser::UnwrappedLineParser(const FormatStyle &Style,
FormatTokenSource &Tokens,
UnwrappedLineConsumer &Callback)
- : RootTokenInitialized(false), Style(Style), Tokens(&Tokens),
- Callback(Callback) {
+ : Line(new UnwrappedLine), RootTokenInitialized(false),
+ LastInCurrentLine(NULL), MustBreakBeforeNextToken(false), Style(Style),
+ Tokens(&Tokens), Callback(Callback) {
}
bool UnwrappedLineParser::parse() {
@@ -126,9 +127,9 @@
addUnwrappedLine();
- Line.Level += AddLevels;
+ Line->Level += AddLevels;
parseLevel(/*HasOpeningBrace=*/true);
- Line.Level -= AddLevels;
+ Line->Level -= AddLevels;
if (!FormatTok.Tok.is(tok::r_brace))
return true;
@@ -139,7 +140,7 @@
void UnwrappedLineParser::parsePPDirective() {
assert(FormatTok.Tok.is(tok::hash) && "'#' expected");
- ScopedMacroState MacroState(Line, Tokens, FormatTok);
+ ScopedMacroState MacroState(*Line, Tokens, FormatTok);
nextToken();
if (FormatTok.Tok.getIdentifierInfo() == NULL) {
@@ -169,7 +170,7 @@
parseParens();
}
addUnwrappedLine();
- Line.Level = 1;
+ Line->Level = 1;
// Errors during a preprocessor directive can only affect the layout of the
// preprocessor directive, and thus we ignore them. An alternative approach
@@ -319,9 +320,9 @@
NeedsUnwrappedLine = true;
} else {
addUnwrappedLine();
- ++Line.Level;
+ ++Line->Level;
parseStructuralElement();
- --Line.Level;
+ --Line->Level;
}
if (FormatTok.Tok.is(tok::kw_else)) {
nextToken();
@@ -332,9 +333,9 @@
parseIfThenElse();
} else {
addUnwrappedLine();
- ++Line.Level;
+ ++Line->Level;
parseStructuralElement();
- --Line.Level;
+ --Line->Level;
}
} else if (NeedsUnwrappedLine) {
addUnwrappedLine();
@@ -363,9 +364,9 @@
addUnwrappedLine();
} else {
addUnwrappedLine();
- ++Line.Level;
+ ++Line->Level;
parseStructuralElement();
- --Line.Level;
+ --Line->Level;
}
}
@@ -376,9 +377,9 @@
parseBlock();
} else {
addUnwrappedLine();
- ++Line.Level;
+ ++Line->Level;
parseStructuralElement();
- --Line.Level;
+ --Line->Level;
}
// FIXME: Add error handling.
@@ -395,14 +396,14 @@
// FIXME: remove all asserts.
assert(FormatTok.Tok.is(tok::colon) && "':' expected");
nextToken();
- unsigned OldLineLevel = Line.Level;
- if (Line.Level > 0)
- --Line.Level;
+ unsigned OldLineLevel = Line->Level;
+ if (Line->Level > 0)
+ --Line->Level;
if (FormatTok.Tok.is(tok::l_brace)) {
parseBlock();
}
addUnwrappedLine();
- Line.Level = OldLineLevel;
+ Line->Level = OldLineLevel;
}
void UnwrappedLineParser::parseCaseLabel() {
@@ -423,9 +424,9 @@
addUnwrappedLine();
} else {
addUnwrappedLine();
- Line.Level += (Style.IndentCaseLabels ? 2 : 1);
+ Line->Level += (Style.IndentCaseLabels ? 2 : 1);
parseStructuralElement();
- Line.Level -= (Style.IndentCaseLabels ? 2 : 1);
+ Line->Level -= (Style.IndentCaseLabels ? 2 : 1);
}
}
@@ -444,7 +445,7 @@
case tok::l_brace:
nextToken();
addUnwrappedLine();
- ++Line.Level;
+ ++Line->Level;
parseComments();
break;
case tok::l_paren:
@@ -458,7 +459,7 @@
case tok::r_brace:
if (HasContents)
addUnwrappedLine();
- --Line.Level;
+ --Line->Level;
nextToken();
break;
case tok::semi:
@@ -501,8 +502,9 @@
FormatTok.Tok.is(tok::comment)) {
nextToken();
}
- Callback.consumeUnwrappedLine(Line);
+ Callback.consumeUnwrappedLine(*Line);
RootTokenInitialized = false;
+ LastInCurrentLine = NULL;
}
bool UnwrappedLineParser::eof() const {
@@ -513,26 +515,42 @@
if (eof())
return;
if (RootTokenInitialized) {
+ assert(LastInCurrentLine->Children.empty());
LastInCurrentLine->Children.push_back(FormatTok);
LastInCurrentLine = &LastInCurrentLine->Children.back();
} else {
- Line.RootToken = FormatTok;
+ Line->RootToken = FormatTok;
RootTokenInitialized = true;
- LastInCurrentLine = &Line.RootToken;
+ LastInCurrentLine = &Line->RootToken;
+ }
+ if (MustBreakBeforeNextToken) {
+ LastInCurrentLine->MustBreakBefore = true;
+ MustBreakBeforeNextToken = false;
}
readToken();
}
void UnwrappedLineParser::readToken() {
FormatTok = Tokens->getNextToken();
- while (!Line.InPPDirective && FormatTok.Tok.is(tok::hash) &&
+ while (!Line->InPPDirective && FormatTok.Tok.is(tok::hash) &&
((FormatTok.NewlinesBefore > 0 && FormatTok.HasUnescapedNewline) ||
FormatTok.IsFirst)) {
- // FIXME: This is incorrect - the correct way is to create a
- // data structure that will construct the parts around the preprocessor
- // directive as a structured \c UnwrappedLine.
- addUnwrappedLine();
+ UnwrappedLine* StoredLine = Line.take();
+ Line.reset(new UnwrappedLine(*StoredLine));
+ assert(LastInCurrentLine == NULL || LastInCurrentLine->Children.empty());
+ FormatToken *StoredLastInCurrentLine = LastInCurrentLine;
+ bool PreviousInitialized = RootTokenInitialized;
+ RootTokenInitialized = false;
+ LastInCurrentLine = NULL;
+
parsePPDirective();
+
+ assert(!RootTokenInitialized);
+ Line.reset(StoredLine);
+ RootTokenInitialized = PreviousInitialized;
+ LastInCurrentLine = StoredLastInCurrentLine;
+ assert(LastInCurrentLine == NULL || LastInCurrentLine->Children.empty());
+ MustBreakBeforeNextToken = true;
}
}
Modified: cfe/trunk/lib/Format/UnwrappedLineParser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.h?rev=171974&r1=171973&r2=171974&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineParser.h (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.h Wed Jan 9 09:25:02 2013
@@ -34,7 +34,7 @@
struct FormatToken {
FormatToken()
: NewlinesBefore(0), HasUnescapedNewline(false), WhiteSpaceLength(0),
- TokenLength(0), IsFirst(false) {
+ TokenLength(0), IsFirst(false), MustBreakBefore(false) {
}
/// \brief The \c Token.
@@ -68,6 +68,12 @@
/// \brief Indicates that this is the first token.
bool IsFirst;
+ /// \brief Whether there must be a line break before this token.
+ ///
+ /// This happens for example when a preprocessor directive ended directly
+ /// before the token.
+ bool MustBreakBefore;
+
// FIXME: We currently assume that there is exactly one token in this vector
// except for the very last token that does not have any children.
/// \brief All tokens that logically follow this token.
@@ -144,10 +150,11 @@
// FIXME: We are constantly running into bugs where Line.Level is incorrectly
// subtracted from beyond 0. Introduce a method to subtract from Line.Level
// and use that everywhere in the Parser.
- UnwrappedLine Line;
+ llvm::OwningPtr<UnwrappedLine> Line;
bool RootTokenInitialized;
FormatToken *LastInCurrentLine;
FormatToken FormatTok;
+ bool MustBreakBeforeNextToken;
const FormatStyle &Style;
FormatTokenSource *Tokens;
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=171974&r1=171973&r2=171974&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Jan 9 09:25:02 2013
@@ -46,17 +46,24 @@
std::string messUp(llvm::StringRef Code) {
std::string MessedUp(Code.str());
bool InComment = false;
+ bool InPreprocessorDirective = false;
bool JustReplacedNewline = false;
for (unsigned i = 0, e = MessedUp.size() - 1; i != e; ++i) {
if (MessedUp[i] == '/' && MessedUp[i + 1] == '/') {
if (JustReplacedNewline)
MessedUp[i - 1] = '\n';
InComment = true;
+ } else if (MessedUp[i] == '#' && JustReplacedNewline) {
+ MessedUp[i - 1] = '\n';
+ InPreprocessorDirective = true;
} else if (MessedUp[i] == '\\' && MessedUp[i + 1] == '\n') {
MessedUp[i] = ' ';
+ MessedUp[i + 1] = ' ';
} else if (MessedUp[i] == '\n') {
if (InComment) {
InComment = false;
+ } else if (InPreprocessorDirective) {
+ InPreprocessorDirective = false;
} else {
JustReplacedNewline = true;
MessedUp[i] = ' ';
@@ -84,6 +91,14 @@
}
};
+TEST_F(FormatTest, MessUp) {
+ EXPECT_EQ("1 2 3", messUp("1 2 3"));
+ EXPECT_EQ("1 2 3\n", messUp("1\n2\n3\n"));
+ EXPECT_EQ("a\n//b\nc", messUp("a\n//b\nc"));
+ EXPECT_EQ("a\n#b\nc", messUp("a\n#b\nc"));
+ EXPECT_EQ("a\n#b c d\ne", messUp("a\n#b\\\nc\\\nd\ne"));
+}
+
//===----------------------------------------------------------------------===//
// Basic function tests.
//===----------------------------------------------------------------------===//
@@ -545,7 +560,9 @@
}
TEST_F(FormatTest, MacroDefinitionInsideStatement) {
- EXPECT_EQ("int x,\n#define A\ny;", format("int x,\n#define A\ny;"));
+ EXPECT_EQ("int x,\n"
+ "#define A\n"
+ " y;", format("int x,\n#define A\ny;"));
}
TEST_F(FormatTest, HashInMacroDefinition) {
@@ -609,6 +626,23 @@
" aLooooooooooooooooooooooonPaaaaaaaaaaaaaaaaaaaaarmmmm);\n"));
}
+TEST_F(FormatTest, LayoutStatementsAroundPreprocessorDirectives) {
+ EXPECT_EQ("int\n"
+ "#define A\n"
+ " a;",
+ format("int\n#define A\na;"));
+ verifyFormat(
+ "functionCallTo(someOtherFunction(\n"
+ " withSomeParameters, whichInSequence,\n"
+ " areLongerThanALine(andAnotherCall,\n"
+ "#define A \\\n"
+ " B\n"
+ " withMoreParamters,\n"
+ " whichStronglyInfluenceTheLayout),\n"
+ " andMoreParameters),\n"
+ " trailing);", getLLVMStyleWithColumns(69));
+}
+
//===----------------------------------------------------------------------===//
// Line break tests.
//===----------------------------------------------------------------------===//
More information about the cfe-commits
mailing list