[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
Thu Jan 10 07:01:22 PST 2013
On Wed, Jan 9, 2013 at 5:18 PM, Nico Weber <thakis at chromium.org> wrote:
> On Wed, Jan 9, 2013 at 7:25 AM, Manuel Klimek <klimek at google.com> wrote:
> > 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.
>
> Cool! Is there a tracking bug for #ifdef chains?
>
Nope. If you have opinions on how certain things should be formatted, or
what we should prioritize, please file bugs :D
>
> >
> > 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.
> >
> //===----------------------------------------------------------------------===//
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130110/7c5e573c/attachment.html>
More information about the cfe-commits
mailing list