Author: alexfh Date: Tue Sep 10 07:29:48 2013 New Revision: 190405 URL: http://llvm.org/viewvc/llvm-project?rev=190405&view=rev Log: Correctly calculate OriginalColumn after multi-line tokens.
Summary: This also unifies the handling of escaped newlines for all tokens. Reviewers: djasper Reviewed By: djasper CC: cfe-commits, klimek Differential Revision: http://llvm-reviews.chandlerc.com/D1638 Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp cfe/trunk/lib/Format/ContinuationIndenter.h cfe/trunk/lib/Format/Format.cpp cfe/trunk/unittests/Format/FormatTest.cpp Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=190405&r1=190404&r2=190405&view=diff ============================================================================== --- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original) +++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Tue Sep 10 07:29:48 2013 @@ -611,9 +611,8 @@ unsigned ContinuationIndenter::moveState return Penalty; } -unsigned -ContinuationIndenter::addMultilineStringLiteral(const FormatToken &Current, - LineState &State) { +unsigned ContinuationIndenter::addMultilineToken(const FormatToken &Current, + LineState &State) { // Break before further function parameters on all levels. for (unsigned i = 0, e = State.Stack.size(); i != e; ++i) State.Stack[i].BreakBeforeParameter = true; @@ -631,6 +630,11 @@ ContinuationIndenter::addMultilineString unsigned ContinuationIndenter::breakProtrudingToken(const FormatToken &Current, LineState &State, bool DryRun) { + // Don't break multi-line tokens other than block comments. Instead, just + // update the state. + if (Current.Type != TT_BlockComment && Current.IsMultiline) + return addMultilineToken(Current, State); + if (!Current.isOneOf(tok::string_literal, tok::comment)) return 0; @@ -639,12 +643,6 @@ unsigned ContinuationIndenter::breakProt if (Current.is(tok::string_literal) && Current.Type != TT_ImplicitStringLiteral) { - // Don't break string literals with (in case of non-raw strings, escaped) - // newlines. As clang-format must not change the string's content, it is - // unlikely that we'll end up with a better format. - if (Current.IsMultiline) - return addMultilineStringLiteral(Current, State); - // Only break up default narrow strings. if (!Current.TokenText.startswith("\"")) return 0; @@ -662,16 +660,6 @@ unsigned ContinuationIndenter::breakProt } else if (Current.Type == TT_LineComment && (Current.Previous == NULL || Current.Previous->Type != TT_ImplicitStringLiteral)) { - // Don't break line comments with escaped newlines. These look like - // separate line comments, but in fact contain a single line comment with - // multiple lines including leading whitespace and the '//' markers. - // - // FIXME: If we want to handle them correctly, we'll need to adjust - // leading whitespace in consecutive lines when changing indentation of - // the first line similar to what we do with block comments. - if (Current.IsMultiline) - return 0; - Token.reset(new BreakableLineComment( Current, StartColumn, State.Line->InPPDirective, Encoding, Style)); } else { Modified: cfe/trunk/lib/Format/ContinuationIndenter.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.h?rev=190405&r1=190404&r2=190405&view=diff ============================================================================== --- cfe/trunk/lib/Format/ContinuationIndenter.h (original) +++ cfe/trunk/lib/Format/ContinuationIndenter.h Tue Sep 10 07:29:48 2013 @@ -84,13 +84,12 @@ private: unsigned breakProtrudingToken(const FormatToken &Current, LineState &State, bool DryRun); - /// \brief Adds a multiline string literal to the \p State. + /// \brief Adds a multiline token to the \p State. /// /// \returns Extra penalty for the first line of the literal: last line is /// handled in \c addNextStateToQueue, and the penalty for other lines doesn't /// matter, as we don't change them. - unsigned addMultilineStringLiteral(const FormatToken &Current, - LineState &State); + unsigned addMultilineToken(const FormatToken &Current, LineState &State); /// \brief Returns \c true if the next token starts a multiline string /// literal. Modified: cfe/trunk/lib/Format/Format.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=190405&r1=190404&r2=190405&view=diff ============================================================================== --- cfe/trunk/lib/Format/Format.cpp (original) +++ cfe/trunk/lib/Format/Format.cpp Tue Sep 10 07:29:48 2013 @@ -656,8 +656,6 @@ private: // In case the token starts with escaped newlines, we want to // take them into account as whitespace - this pattern is quite frequent // in macro definitions. - // FIXME: What do we want to do with other escaped spaces, and escaped - // spaces or newlines in the middle of tokens? // FIXME: Add a more explicit test. while (FormatTok->TokenText.size() > 1 && FormatTok->TokenText[0] == '\\' && FormatTok->TokenText[1] == '\n') { @@ -692,26 +690,27 @@ private: StringRef Text = FormatTok->TokenText; size_t FirstNewlinePos = Text.find('\n'); - if (FirstNewlinePos != StringRef::npos) { + if (FirstNewlinePos == StringRef::npos) { + // FIXME: ColumnWidth actually depends on the start column, we need to + // take this into account when the token is moved. + FormatTok->ColumnWidth = + encoding::columnWidthWithTabs(Text, Column, Style.TabWidth, Encoding); + Column += FormatTok->ColumnWidth; + } else { FormatTok->IsMultiline = true; + // FIXME: ColumnWidth actually depends on the start column, we need to + // take this into account when the token is moved. + FormatTok->ColumnWidth = encoding::columnWidthWithTabs( + Text.substr(0, FirstNewlinePos), Column, Style.TabWidth, Encoding); + // The last line of the token always starts in column 0. // Thus, the length can be precomputed even in the presence of tabs. FormatTok->LastLineColumnWidth = encoding::columnWidthWithTabs( Text.substr(Text.find_last_of('\n') + 1), 0, Style.TabWidth, Encoding); - Text = Text.substr(0, FirstNewlinePos); + Column = FormatTok->LastLineColumnWidth; } - // FIXME: ColumnWidth actually depends on the start column, we need to - // take this into account when the token is moved. - FormatTok->ColumnWidth = - encoding::columnWidthWithTabs(Text, Column, Style.TabWidth, Encoding); - - // FIXME: For multi-line tokens this should be LastLineColumnWidth. - // For line comments this should probably be zero. But before changing, - // we need good tests for this. - Column += FormatTok->ColumnWidth; - return FormatTok; } Modified: cfe/trunk/unittests/Format/FormatTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=190405&r1=190404&r2=190405&view=diff ============================================================================== --- cfe/trunk/unittests/Format/FormatTest.cpp (original) +++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Sep 10 07:29:48 2013 @@ -5736,9 +5736,6 @@ TEST_F(FormatTest, ConfigurableUseOfTab) "};", Tab); - // FIXME: To correctly count mixed whitespace we need to - // also correctly count mixed whitespace in front of the comment. - Tab.TabWidth = 8; Tab.IndentWidth = 8; EXPECT_EQ("/*\n" @@ -5795,6 +5792,39 @@ TEST_F(FormatTest, ConfigurableUseOfTab) "}")); } +TEST_F(FormatTest, CalculatesOriginalColumn) { + EXPECT_EQ("\"qqqqqqqqqqqqqqqqqqqqqqqqqq\\\n" + "q\"; /* some\n" + " comment */", + format(" \"qqqqqqqqqqqqqqqqqqqqqqqqqq\\\n" + "q\"; /* some\n" + " comment */", + getLLVMStyle())); + EXPECT_EQ("// qqqqqqqqqqqqqqqqqqqqqqqqqq\n" + "/* some\n" + " comment */", + format("// qqqqqqqqqqqqqqqqqqqqqqqqqq\n" + " /* some\n" + " comment */", + getLLVMStyle())); + EXPECT_EQ("// qqqqqqqqqqqqqqqqqqqqqqqqqq\\\n" + "qqq\n" + "/* some\n" + " comment */", + format("// qqqqqqqqqqqqqqqqqqqqqqqqqq\\\n" + "qqq\n" + " /* some\n" + " comment */", + getLLVMStyle())); + EXPECT_EQ("inttt qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq\\\n" + "wwww; /* some\n" + " comment */", + format(" inttt qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq\\\n" + "wwww; /* some\n" + " comment */", + getLLVMStyle())); +} + TEST_F(FormatTest, ConfigurableSpaceAfterControlStatementKeyword) { FormatStyle NoSpace = getLLVMStyle(); NoSpace.SpaceAfterControlStatementKeyword = false; _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
