[cfe-commits] r171974 - in /cfe/trunk: lib/Format/Format.cpp lib/Format/UnwrappedLineParser.cpp lib/Format/UnwrappedLineParser.h unittests/Format/FormatTest.cpp
Nico Weber
thakis at chromium.org
Wed Jan 9 08:18:55 PST 2013
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?
>
> 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
More information about the cfe-commits
mailing list