On Wed, Jan 9, 2013 at 5:18 PM, Nico Weber <[email protected]> wrote:
> On Wed, Jan 9, 2013 at 7:25 AM, Manuel Klimek <[email protected]> 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 > > [email protected] > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
