r190405 - Correctly calculate OriginalColumn after multi-line tokens.
Alexander Kornienko
alexfh at google.com
Tue Sep 10 05:29:48 PDT 2013
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;
More information about the cfe-commits
mailing list