Restructure how we break tokens.

[klimek]

Summary

This fixes some bugs in the reflowing logic and splits out the concerns
of reflowing from BreakableToken.

Things to do after this patch:

• Refactor the breakProtrudingToken function possibly into a class, so we can split it up into methods that operate on the common state.
• Optimize whitespace compression when reflowing by using the next possible split point instead of the latest possible split point.

[krasimir]

Here're a few nits. I'll need an evening to review the meaty part :)

lib/Format/BreakableToken.cpp

73 ↗

(On Diff #123807)

nit: no braces around single-statement if body.

198 ↗

(On Diff #123807)

How about clients that explicitly pass Length = Line.size() - Offset?

476 ↗

(On Diff #123807)

Could you elaborate?

477 ↗

(On Diff #123807)

nit: postfix.

566 ↗

(On Diff #123807)

nit: af -> at

570 ↗

(On Diff #123807)

nit: no braces around single-statement if bodies.

851 ↗

(On Diff #123807)

could you give an example of what's a non-regular indent?

lib/Format/BreakableToken.h

52 ↗

(On Diff #123807)

Replace are with is.

100 ↗

(On Diff #123807)

Does this include the byte Offset + Length?

107 ↗

(On Diff #123807)

text is ambiguous here: does it refer to the content of the line or to the range defined by the offset and length?

114 ↗

(On Diff #123807)

What is Break used for?

120 ↗

(On Diff #123807)

What is ReflownColumn used for?

142 ↗

(On Diff #123807)

Does the current line refer to the line at LineIndex?

lib/Format/ContinuationIndenter.cpp

1504 ↗

(On Diff #123807)

Does a reflow count as a break?

unittests/Format/FormatTest.cpp

7735 ↗

(On Diff #123807)

TODO?

10603 ↗

(On Diff #123807)

What happened here?

unittests/Format/FormatTestComments.cpp

1104 ↗

(On Diff #123807)

I think this is desirable. The jsdoc folks really wanted to fix-up the */ at the end. I think it has to do with DelimitersOnNewline.
If this already works in Java and js mode, that could be good enough.

2108 ↗

(On Diff #123807)

I thinks that we should be compressing whitespace in reflow mode too.

2149 ↗

(On Diff #123807)

This is like this for cases like lists in comments:

blah-blah-blah:
  1. blah
  2. blah-blah

I think here the block comments behavior might be wrong.

[klimek]

Address review comments.

[klimek]

lib/Format/BreakableToken.cpp

198 ↗

(On Diff #123807)

That is different (I now also went and updated the comment for getRangeLength to explain that).

Generally, Length == Line.size() - Offset is still a portion of the content, as opposed to npos, which has a special meaning. I'm wondering whether I should just pull out a differently named method for it, now that I'm thinking about it.

lib/Format/BreakableToken.h

100 ↗

(On Diff #123807)

Reworded.

107 ↗

(On Diff #123807)

Introduced 'text' in the first paragraph and reworded.

120 ↗

(On Diff #123807)

Argh, actually not called ReflownColumn in the implementations :)
Changed and commented.

142 ↗

(On Diff #123807)

Reworded.

lib/Format/ContinuationIndenter.cpp

1504 ↗

(On Diff #123807)

I do believe so (well, the break in the reflow counts, the reflow itself is not a break, but removing a break :)

unittests/Format/FormatTest.cpp

10603 ↗

(On Diff #123807)

It was wrong, it's now correct :)
Those characters are printed in double-width, and llvm's encoding library correctly returns this. Reflowing previously did not correctly count that, but used the character count instead.
At offset 8 (after a \t), C " pokes one column over the limit (C == double-width character, space == 1, " == 1 -> 4 -> 12 columns.

unittests/Format/FormatTestComments.cpp

1104 ↗

(On Diff #123807)

I don't know whether it does - I only know I didn't break any tests :)

2108 ↗

(On Diff #123807)

Correct, but in a different patch - this patch is a strict improvement regarding whitespace compression, and adding more would make it more complex.

2149 ↗

(On Diff #123807)

Note that on the doc I shared you voted the reverse ;)

[krasimir]

lib/Format/BreakableToken.cpp

198 ↗

(On Diff #123807)

Yes please!

lib/Format/ContinuationIndenter.cpp

1504 ↗

(On Diff #123807)

So, if we add a break then remove a break while reflowing, will BreakInserted be true or false?

unittests/Format/FormatTestComments.cpp

2149 ↗

(On Diff #123807)

Then I should consider re-voting :)

[klimek]

Pull out getRemainingLength.

[klimek]

lib/Format/ContinuationIndenter.cpp

1504 ↗

(On Diff #123807)

True.

[krasimir]

Started the review. It would take a few cycles 💃

lib/Format/ContinuationIndenter.cpp

1518 ↗

(On Diff #124095)

Could you please spell out the invariants that we maintain about TailOffset, RemainingTokenColumns and ContentStartColumn (at least) at the beginning of every main loop iteration below? That would surely make it easier to review.

1533 ↗

(On Diff #124095)

If the previous iteration requested that we try to reflow, how can ContentStartColumn be at the start of the (current) line?

[klimek]

lib/Format/ContinuationIndenter.cpp

1518 ↗

(On Diff #124095)

I'm not sure what you mean - I would literally write what I already wrote in the comment. Can you formulate questions so I can make sure I answer them?

1533 ↗

(On Diff #124095)

After a line break (that was already in the original text), ContentStartColumn is set to where we want the first character of the broken line to start.

[klimek]

Restructure based on code review feedback.

[klimek]

Restructured to make the invariants clearer based on a chat with Krasimir.

[krasimir]

lib/Format/ContinuationIndenter.cpp

1525 ↗

(On Diff #124381)

If we indent here, shouldn't that also change ContentStartColumn?

1557 ↗

(On Diff #124381)

How does the last line's penalty get handled in addNextStateToQueue()?

1565 ↗

(On Diff #124381)

I really dislike this: we shouldn't have the reflow control flow depend on the specific type of the token. Better introduce a new virtual method that enables this branch and override it appropriately.

1578 ↗

(On Diff #124381)

nit: BreakableToken::Split NextSplit = Token->getSplit(...)

1578 ↗

(On Diff #124381)

Hm, right now ContentStartColumn + ToSplitColumns points to the column where the character at offset TailOffset + Split.first is supposed to be. Then NextSplit is relative to the offset TailOffset + Split.first + Split.second, so IMO it shouldn't use ContentStartColumn + ToSplitColumns as a start column. I think that ToSplitColumns needs to be computed as follows:

unsigned ToSplitColumns = Token->getRangeLength(LineIndex, TailOffset, Split.first + Split.second, ContentStartColumn);

Also, a comment of the intended meaning of ToSplitColumns would be helpful.

1579 ↗

(On Diff #124381)

nit: comlumns

[klimek]

Address review comments.

[klimek]

lib/Format/ContinuationIndenter.cpp

1525 ↗

(On Diff #124381)

Nope, this will exactly adapt the start of the line to ContentStartColumn. ContentStartColumn is where the line thinks it wants to be, not where it is.

1557 ↗

(On Diff #124381)

By the State's Column being above the column limit.

1578 ↗

(On Diff #124381)

Good catch, but the solution is differnet. Using ContentStartColumn + ToSplitColumns is incorrect, we need ContentStartColumn + ToSplitColumns + 1 (as Split.second just contains the whitespace, but we want to consider that whitespace compressed).

I tried to write a test for this, and convinced myself that while +1 is correct, it is currently impossible to change behavior based on the missing +1.

[krasimir]

lib/Format/BreakableToken.cpp

178 ↗

(On Diff #124581)

Offtopic: Should add a FIXME that this doesn't really work in the presence of tabs.

lib/Format/BreakableToken.h

154 ↗

(On Diff #124581)

nit: the comment doesn't make sense anymore.

lib/Format/ContinuationIndenter.cpp

1625 ↗

(On Diff #124581)

nit: no braces around single-statement if body

1707 ↗

(On Diff #124581)

When we're reflowing we replace the reflow split with a space, so IMO this should be:

RemainingTokenColumns = Token->getRemainingLength(
              NextLineIndex, TailOffset, ContentStartColumn + 1);

1709 ↗

(On Diff #124581)

IMO this should be ContentStartColumn + RemainingTokenColumns + 1, accounting for the space that replaces the reflow split.

1721 ↗

(On Diff #124581)

Looks like ContentStartColumn is consistently used as the start column of the reflown content on the next line.
I suggest to ++ContentStartColumn at the beginning of the body of this if statement (and adjust below appropriately).

[krasimir]

Re: "I tried to write a test for this, and convinced myself that while +1 is correct, it is currently impossible to change behavior based on the missing +1.":
In order to have different outcome based on the start column, you could use tabs. Consider the content "aaa\tb" with 4 spaces of tabstop. This takes up 5 columns (say, under the column limit) if it starts at column 0, and 8 columns (say, over the column limit) if it starts at column 1.

[klimek]

Address review comments.

[klimek]

lib/Format/ContinuationIndenter.cpp

1707 ↗

(On Diff #124581)

Actually, ContentStartColumn is just calculated incorrectly above. Added tests, and added the +1 above, which makes it +1 for all code below.

1721 ↗

(On Diff #124581)

Yep, that's what I also figured out - that also led to removing ++ContentStartColumn in the reflow case below, which was what made this work at all.
Added tests to ReflowsCommentsPrecise, which flow through all of the corner cases of the if's here.

[krasimir]

lib/Format/ContinuationIndenter.cpp

1749 ↗

(On Diff #124709)

nit: Maybe change this to if (Reflow) and switch the if-else bodies.

1777 ↗

(On Diff #124709)

Shouldn't we be resetting NewBreakBefore to false here?

[klimek]

lib/Format/ContinuationIndenter.cpp

1749 ↗

(On Diff #124709)

I had that first, but found that harder to follow when re-reading the code - if you feel strongly, I'm also happy to turn it around again :)

1777 ↗

(On Diff #124709)

NewBreakBefore is always reset at the start of the loop, so resetting it here wouldn't matter.

[krasimir]

LGTM

cfe/trunk/lib/Format/BreakableToken.h

 namespace format {
 
 /// \brief Checks if \p Token switches formatting, like /* clang-format off */.
 /// \p Token must be a comment.
 bool switchesFormatting(const FormatToken &Token);
 
 struct FormatStyle;
 
-/// \brief Base class for strategies on how to break tokens.
+/// \brief Base class for tokens / ranges of tokens that can allow breaking
+/// within the tokens - for example, to avoid whitespace beyond the column
+/// limit, or to reflow text.
+///
+/// Generally, a breakable token consists of logical lines, addressed by a line
+/// index. For example, in a sequence of line comments, each line comment is its
+/// own logical line; similarly, for a block comment, each line in the block
+/// comment is on its own logical line.
+///
+/// There are two methods to compute the layout of the token:
+/// - getRangeLength measures the number of columns needed for a range of text
+///   within a logical line, and
+/// - getContentStartColumn returns the start column at which we want the
+///   content of a logical line to start (potentially after introducing a line
+///   break).
+///
+/// The mechanism to adapt the layout of the breakable token is organised
+/// around the concept of a \c Split, which is a whitespace range that signifies
+/// a position of the content of a token where a reformatting might be done.
 ///
-/// This is organised around the concept of a \c Split, which is a whitespace
-/// range that signifies a position of the content of a token where a
-/// reformatting might be done. Operating with splits is divided into 3
-/// operations:
+/// Operating with splits is divided into two operations:
 /// - getSplit, for finding a split starting at a position,
-/// - getLineLengthAfterSplit, for calculating the size in columns of the rest
-///   of the content after a split has been used for breaking, and
 /// - insertBreak, for executing the split using a whitespace manager.
 ///
 /// There is a pair of operations that are used to compress a long whitespace
-/// range with a single space if that will bring the line lenght under the
+/// range with a single space if that will bring the line length under the
 /// column limit:
 /// - getLineLengthAfterCompression, for calculating the size in columns of the
 ///   line after a whitespace range has been compressed, and
 /// - compressWhitespace, for executing the whitespace compression using a
 ///   whitespace manager; note that the compressed whitespace may be in the
 ///   middle of the original line and of the reformatted line.
 ///
 /// For tokens where the whitespace before each line needs to be also
 /// reformatted, for example for tokens supporting reflow, there are analogous
 /// operations that might be executed before the main line breaking occurs:
-/// - getSplitBefore, for finding a split such that the content preceding it
+/// - getReflowSplit, for finding a split such that the content preceding it
 ///   needs to be specially reflown,
+/// - reflow, for executing the split using a whitespace manager,
 /// - introducesBreakBefore, for checking if reformatting the beginning
 ///   of the content introduces a line break before it,
-/// - getLineLengthAfterSplitBefore, for calculating the line length in columns
-///   of the remainder of the content after the beginning of the content has
-///   been reformatted, and
-/// - replaceWhitespaceBefore, for executing the reflow using a whitespace
+/// - adaptStartOfLine, for executing the reflow using a whitespace
 ///   manager.
 ///
 /// For tokens that require the whitespace after the last line to be
 /// reformatted, for example in multiline jsdoc comments that require the
 /// trailing '*/' to be on a line of itself, there are analogous operations
 /// that might be executed after the last line has been reformatted:
 /// - getSplitAfterLastLine, for finding a split after the last line that needs
 ///   to be reflown,
-/// - getLineLengthAfterSplitAfterLastLine, for calculating the line length in
-///   columns of the remainder of the token, and
 /// - replaceWhitespaceAfterLastLine, for executing the reflow using a
 ///   whitespace manager.
 ///
-/// FIXME: The interface seems set in stone, so we might want to just pull the
-/// strategy into the class, instead of controlling it from the outside.
 class BreakableToken {
 public:
   /// \brief Contains starting character index and length of split.
   typedef std::pair<StringRef::size_type, unsigned> Split;
 
   virtual ~BreakableToken() {}
 
   /// \brief Returns the number of lines in this token in the original code.
   virtual unsigned getLineCount() const = 0;
 
-  /// \brief Returns the number of columns required to format the piece of line
-  /// at \p LineIndex, from byte offset \p TailOffset with length \p Length.
+  /// \brief Returns the number of columns required to format the text in the
+  /// byte range [\p Offset, \p Offset \c + \p Length).
   ///
-  /// Note that previous breaks are not taken into account. \p TailOffset is
-  /// always specified from the start of the (original) line.
-  /// \p Length can be set to StringRef::npos, which means "to the end of line".
-  virtual unsigned
-  getLineLengthAfterSplit(unsigned LineIndex, unsigned TailOffset,
-                          StringRef::size_type Length) const = 0;
+  /// \p Offset is the byte offset from the start of the content of the line
+  ///    at \p LineIndex.
+  ///
+  /// \p StartColumn is the column at which the text starts in the formatted
+  ///    file, needed to compute tab stops correctly.
+  virtual unsigned getRangeLength(unsigned LineIndex, unsigned Offset,
+                                  StringRef::size_type Length,
+                                  unsigned StartColumn) const = 0;
+
+  /// \brief Returns the number of columns required to format the text following
+  /// the byte \p Offset in the line \p LineIndex, including potentially
+  /// unbreakable sequences of tokens following after the end of the token.
+  ///
+  /// \p Offset is the byte offset from the start of the content of the line
+  ///    at \p LineIndex.
+  ///
+  /// \p StartColumn is the column at which the text starts in the formatted
+  ///    file, needed to compute tab stops correctly.
+  ///
+  /// For breakable tokens that never use extra space at the end of a line, this
+  /// is equivalent to getRangeLength with a Length of StringRef::npos.
+  virtual unsigned getRemainingLength(unsigned LineIndex, unsigned Offset,
+                                      unsigned StartColumn) const {
+    return getRangeLength(LineIndex, Offset, StringRef::npos, StartColumn);
+  }
+
+  /// \brief Returns the column at which content in line \p LineIndex starts,
+  /// assuming no reflow.
+  ///
+  /// If \p Break is true, returns the column at which the line should start
+  /// after the line break.
+  /// If \p Break is false, returns the column at which the line itself will
+  /// start.
+  virtual unsigned getContentStartColumn(unsigned LineIndex,
+                                         bool Break) const = 0;
 
   /// \brief Returns a range (offset, length) at which to break the line at
   /// \p LineIndex, if previously broken at \p TailOffset. If possible, do not
-  /// violate \p ColumnLimit.
+  /// violate \p ColumnLimit, assuming the text starting at \p TailOffset in
+  /// the token is formatted starting at ContentStartColumn in the reformatted
+  /// file.
   virtual Split getSplit(unsigned LineIndex, unsigned TailOffset,
-                         unsigned ColumnLimit,
+                         unsigned ColumnLimit, unsigned ContentStartColumn,
                          llvm::Regex &CommentPragmasRegex) const = 0;
 
   /// \brief Emits the previously retrieved \p Split via \p Whitespaces.
   virtual void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
-                           WhitespaceManager &Whitespaces) = 0;
+                           WhitespaceManager &Whitespaces) const = 0;
 
-  /// \brief Returns the number of columns required to format the piece of line
-  /// at \p LineIndex, from byte offset \p TailOffset after the whitespace range
-  /// \p Split has been compressed into a single space.
-  unsigned getLineLengthAfterCompression(unsigned RemainingTokenColumns,
-                                         Split Split) const;
+  /// \brief Returns the number of columns needed to format
+  /// \p RemainingTokenColumns, assuming that Split is within the range measured
+  /// by \p RemainingTokenColumns, and that the whitespace in Split is reduced
+  /// to a single space.
+  unsigned getLengthAfterCompression(unsigned RemainingTokenColumns,
 
   /// \brief Replaces the whitespace range described by \p Split with a single
   /// space.
   virtual void compressWhitespace(unsigned LineIndex, unsigned TailOffset,
                                   Split Split,
-                                  WhitespaceManager &Whitespaces) = 0;
+                                  WhitespaceManager &Whitespaces) const = 0;
 
-  /// \brief Returns a whitespace range (offset, length) of the content at
-  /// \p LineIndex such that the content preceding this range needs to be
-  /// reformatted before any breaks are made to this line.
+  /// \brief Returns whether the token supports reflowing text.
+  virtual bool supportsReflow() const { return false; }
+
+  /// \brief Returns a whitespace range (offset, length) of the content at \p
+  /// LineIndex such that the content of that line is reflown to the end of the
+  /// previous one.
   ///
-  /// \p PreviousEndColumn is the end column of the previous line after
-  /// formatting.
+  /// Returning (StringRef::npos, 0) indicates reflowing is not possible.
+  ///
+  /// The range will include any whitespace preceding the specified line's
+  /// content.
   ///
-  /// A result having offset == StringRef::npos means that no piece of the line
-  /// needs to be reformatted before any breaks are made.
-  virtual Split getSplitBefore(unsigned LineIndex, unsigned PreviousEndColumn,
-                               unsigned ColumnLimit,
+  /// If the split is not contained within one token, for example when reflowing
+  /// line comments, returns (0, <length>).
+  virtual Split getReflowSplit(unsigned LineIndex,
                                llvm::Regex &CommentPragmasRegex) const {
     return Split(StringRef::npos, 0);
   }
 
+  /// \brief Reflows the current line into the end of the previous one.
+  virtual void reflow(unsigned LineIndex,
+                      WhitespaceManager &Whitespaces) const {}
+
   /// \brief Returns whether there will be a line break at the start of the
   /// token.
   virtual bool introducesBreakBeforeToken() const {
     return false;
   }
 
-  /// \brief Returns the number of columns required to format the piece of line
-  /// at \p LineIndex after the content preceding the whitespace range specified
-  /// \p SplitBefore has been reformatted, but before any breaks are made to
-  /// this line.
-  virtual unsigned getLineLengthAfterSplitBefore(unsigned LineIndex,
-                                                 unsigned TailOffset,
-                                                 unsigned PreviousEndColumn,
-                                                 unsigned ColumnLimit,
-                                                 Split SplitBefore) const {
-    return getLineLengthAfterSplit(LineIndex, TailOffset, StringRef::npos);
-  }
-
   /// \brief Replaces the whitespace between \p LineIndex-1 and \p LineIndex.
-  /// Performs a reformatting of the content at \p LineIndex preceding the
-  /// whitespace range \p SplitBefore.
-  virtual void replaceWhitespaceBefore(unsigned LineIndex,
-                                       unsigned PreviousEndColumn,
-                                       unsigned ColumnLimit, Split SplitBefore,
-                                       WhitespaceManager &Whitespaces) {}
+  virtual void adaptStartOfLine(unsigned LineIndex,
+                                WhitespaceManager &Whitespaces) const {}
 
   /// \brief Returns a whitespace range (offset, length) of the content at
   /// the last line that needs to be reformatted after the last line has been
   /// reformatted.
   ///
   /// A result having offset == StringRef::npos means that no reformat is
   /// necessary.
-  virtual Split getSplitAfterLastLine(unsigned TailOffset,
-                                      unsigned ColumnLimit) const {
+  virtual Split getSplitAfterLastLine(unsigned TailOffset) const {
     return Split(StringRef::npos, 0);
   }
 
-  /// \brief Returns the number of columns required to format the piece token
-  /// after the last line after a reformat of the whitespace range \p
-  /// \p SplitAfterLastLine on the last line has been performed.
-  virtual unsigned
-  getLineLengthAfterSplitAfterLastLine(unsigned TailOffset,
-                                       Split SplitAfterLastLine) const {
-    return getLineLengthAfterSplit(getLineCount() - 1,
-                                   TailOffset + SplitAfterLastLine.first +
-                                       SplitAfterLastLine.second,
-                                   StringRef::npos);
-  }
-
   /// \brief Replaces the whitespace from \p SplitAfterLastLine on the last line
   /// after the last line has been formatted by performing a reformatting.
-  virtual void replaceWhitespaceAfterLastLine(unsigned TailOffset,
-                                              Split SplitAfterLastLine,
-                                              WhitespaceManager &Whitespaces) {
+  void replaceWhitespaceAfterLastLine(unsigned TailOffset,
+                                      WhitespaceManager &Whitespaces) const {
     insertBreak(getLineCount() - 1, TailOffset, SplitAfterLastLine,
                 Whitespaces);
   }
 
   /// \brief Updates the next token of \p State to the next token after this
   /// one. This can be used when this token manages a set of underlying tokens
   /// as a unit and is responsible for the formatting of the them.
   virtual void updateNextToken(LineState &State) const {}
 
 protected:
   BreakableToken(const FormatToken &Tok, bool InPPDirective,
                  encoding::Encoding Encoding, const FormatStyle &Style)
       : Tok(Tok), InPPDirective(InPPDirective), Encoding(Encoding),
         Style(Style) {}
 
   const FormatToken &Tok;
   const bool InPPDirective;
   const encoding::Encoding Encoding;
   const FormatStyle &Style;
 };
 
-/// \brief Base class for single line tokens that can be broken.
-///
-/// \c getSplit() needs to be implemented by child classes.
-class BreakableSingleLineToken : public BreakableToken {
-public:
-  unsigned getLineCount() const override;
-  unsigned getLineLengthAfterSplit(unsigned LineIndex, unsigned TailOffset,
-                                   StringRef::size_type Length) const override;
-
-protected:
-  BreakableSingleLineToken(const FormatToken &Tok, unsigned StartColumn,
-                           StringRef Prefix, StringRef Postfix,
-                           bool InPPDirective, encoding::Encoding Encoding,
-                           const FormatStyle &Style);
-
-  // The column in which the token starts.
-  unsigned StartColumn;
-  // The prefix a line needs after a break in the token.
-  StringRef Prefix;
-  // The postfix a line needs before introducing a break.
-  StringRef Postfix;
-  // The token text excluding the prefix and postfix.
-  StringRef Line;
-};
-
-class BreakableStringLiteral : public BreakableSingleLineToken {
+class BreakableStringLiteral : public BreakableToken {
 public:
   /// \brief Creates a breakable token for a single line string literal.
   ///
   /// \p StartColumn specifies the column in which the token will start
   /// after formatting.
   BreakableStringLiteral(const FormatToken &Tok, unsigned StartColumn,
                          StringRef Prefix, StringRef Postfix,
                          bool InPPDirective, encoding::Encoding Encoding,
                          const FormatStyle &Style);
 
   Split getSplit(unsigned LineIndex, unsigned TailOffset, unsigned ColumnLimit,
+                 unsigned ReflowColumn,
                  llvm::Regex &CommentPragmasRegex) const override;
   void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
-                   WhitespaceManager &Whitespaces) override;
+                   WhitespaceManager &Whitespaces) const override;
   void compressWhitespace(unsigned LineIndex, unsigned TailOffset, Split Split,
-                          WhitespaceManager &Whitespaces) override {}
+                          WhitespaceManager &Whitespaces) const override {}
+  unsigned getLineCount() const override;
+  unsigned getRangeLength(unsigned LineIndex, unsigned Offset,
+                          StringRef::size_type Length,
+                          unsigned StartColumn) const override;
+  unsigned getRemainingLength(unsigned LineIndex, unsigned Offset,
+                              unsigned StartColumn) const override;
+  unsigned getContentStartColumn(unsigned LineIndex, bool Break) const override;
+
+protected:
+  // The column in which the token starts.
+  unsigned StartColumn;
+  // The prefix a line needs after a break in the token.
+  StringRef Prefix;
+  // The postfix a line needs before introducing a break.
+  StringRef Postfix;
+  // The token text excluding the prefix and postfix.
+  StringRef Line;
+  // Length of the sequence of tokens after this string literal that cannot
+  // contain line breaks.
+  unsigned UnbreakableTailLength;
 };
 
 class BreakableComment : public BreakableToken {
 protected:
   /// \brief Creates a breakable token for a comment.
   ///
   /// \p StartColumn specifies the column in which the comment will start after
   /// formatting.
   BreakableComment(const FormatToken &Token, unsigned StartColumn,
                    bool InPPDirective, encoding::Encoding Encoding,
                    const FormatStyle &Style);
 
 public:
+  bool supportsReflow() const override { return true; }
   unsigned getLineCount() const override;
   Split getSplit(unsigned LineIndex, unsigned TailOffset, unsigned ColumnLimit,
+                 unsigned ReflowColumn,
                  llvm::Regex &CommentPragmasRegex) const override;
   void compressWhitespace(unsigned LineIndex, unsigned TailOffset, Split Split,
-                          WhitespaceManager &Whitespaces) override;
+                          WhitespaceManager &Whitespaces) const override;
 
 protected:
-  virtual unsigned getContentStartColumn(unsigned LineIndex,
-                                         unsigned TailOffset) const = 0;
-
-  // Returns a split that divides Text into a left and right parts, such that
-  // the left part is suitable for reflowing after PreviousEndColumn.
-  Split getReflowSplit(StringRef Text, StringRef ReflowPrefix,
-                       unsigned PreviousEndColumn, unsigned ColumnLimit) const;
-
   // Returns the token containing the line at LineIndex.
   const FormatToken &tokenAt(unsigned LineIndex) const;
 
   // Checks if the content of line LineIndex may be reflown with the previous
   // line.
   virtual bool mayReflow(unsigned LineIndex,
                          llvm::Regex &CommentPragmasRegex) const = 0;
 

 
 class BreakableBlockComment : public BreakableComment {
 public:
   BreakableBlockComment(const FormatToken &Token, unsigned StartColumn,
                         unsigned OriginalStartColumn, bool FirstInLine,
                         bool InPPDirective, encoding::Encoding Encoding,
                         const FormatStyle &Style);
 
-  unsigned getLineLengthAfterSplit(unsigned LineIndex, unsigned TailOffset,
-                                   StringRef::size_type Length) const override;
+  unsigned getRangeLength(unsigned LineIndex, unsigned Offset,
+                          StringRef::size_type Length,
+                          unsigned StartColumn) const override;
+  unsigned getRemainingLength(unsigned LineIndex, unsigned Offset,
+                              unsigned StartColumn) const override;
+  unsigned getContentStartColumn(unsigned LineIndex, bool Break) const override;
   void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
-                   WhitespaceManager &Whitespaces) override;
-  Split getSplitBefore(unsigned LineIndex, unsigned PreviousEndColumn,
-                       unsigned ColumnLimit,
+                   WhitespaceManager &Whitespaces) const override;
+  Split getReflowSplit(unsigned LineIndex,
                        llvm::Regex &CommentPragmasRegex) const override;
+  void reflow(unsigned LineIndex,
+              WhitespaceManager &Whitespaces) const override;
   bool introducesBreakBeforeToken() const override;
-  unsigned getLineLengthAfterSplitBefore(unsigned LineIndex,
-                                         unsigned TailOffset,
-                                         unsigned PreviousEndColumn,
-                                         unsigned ColumnLimit,
-                                         Split SplitBefore) const override;
-  void replaceWhitespaceBefore(unsigned LineIndex, unsigned PreviousEndColumn,
-                               unsigned ColumnLimit, Split SplitBefore,
-                               WhitespaceManager &Whitespaces) override;
-  Split getSplitAfterLastLine(unsigned TailOffset,
-                              unsigned ColumnLimit) const override;
+  void adaptStartOfLine(unsigned LineIndex,
+                        WhitespaceManager &Whitespaces) const override;
+  Split getSplitAfterLastLine(unsigned TailOffset) const override;
 
   bool mayReflow(unsigned LineIndex,
                  llvm::Regex &CommentPragmasRegex) const override;
 
 private:
   // Rearranges the whitespace between Lines[LineIndex-1] and Lines[LineIndex].
   //
   // Updates Content[LineIndex-1] and Content[LineIndex] by stripping off
   // leading and trailing whitespace.
   //
   // Sets ContentColumn to the intended column in which the text at
   // Lines[LineIndex] starts (note that the decoration, if present, is not
   // considered part of the text).
   void adjustWhitespace(unsigned LineIndex, int IndentDelta);
 
-  // Computes the end column if the full Content from LineIndex gets reflown
-  // after PreviousEndColumn.
-  unsigned getReflownColumn(StringRef Content, unsigned LineIndex,
-                            unsigned PreviousEndColumn) const;
-
-  unsigned getContentStartColumn(unsigned LineIndex,
-                                 unsigned TailOffset) const override;
-
   // The column at which the text of a broken line should start.
   // Note that an optional decoration would go before that column.
   // IndentAtLineBreak is a uniform position for all lines in a block comment,
   // regardless of their relative position.
   // FIXME: Revisit the decision to do this; the main reason was to support
   // patterns like
   // /**************//**
   //  * Comment

 
 class BreakableLineCommentSection : public BreakableComment {
 public:
   BreakableLineCommentSection(const FormatToken &Token, unsigned StartColumn,
                               unsigned OriginalStartColumn, bool FirstInLine,
                               bool InPPDirective, encoding::Encoding Encoding,
                               const FormatStyle &Style);
 
-  unsigned getLineLengthAfterSplit(unsigned LineIndex, unsigned TailOffset,
-                                   StringRef::size_type Length) const override;
+  unsigned getRangeLength(unsigned LineIndex, unsigned Offset,
+                          StringRef::size_type Length,
+                          unsigned StartColumn) const override;
+  unsigned getContentStartColumn(unsigned LineIndex, bool Break) const override;
   void insertBreak(unsigned LineIndex, unsigned TailOffset, Split Split,
-                   WhitespaceManager &Whitespaces) override;
-  Split getSplitBefore(unsigned LineIndex, unsigned PreviousEndColumn,
-                       unsigned ColumnLimit,
+                   WhitespaceManager &Whitespaces) const override;
+  Split getReflowSplit(unsigned LineIndex,
                        llvm::Regex &CommentPragmasRegex) const override;
-  unsigned getLineLengthAfterSplitBefore(unsigned LineIndex,
-                                         unsigned TailOffset,
-                                         unsigned PreviousEndColumn,
-                                         unsigned ColumnLimit,
-                                         Split SplitBefore) const override;
-  void replaceWhitespaceBefore(unsigned LineIndex, unsigned PreviousEndColumn,
-                               unsigned ColumnLimit, Split SplitBefore,
-                               WhitespaceManager &Whitespaces) override;
+  void reflow(unsigned LineIndex,
+              WhitespaceManager &Whitespaces) const override;
+  void adaptStartOfLine(unsigned LineIndex,
+                        WhitespaceManager &Whitespaces) const override;
   void updateNextToken(LineState &State) const override;
   bool mayReflow(unsigned LineIndex,
                  llvm::Regex &CommentPragmasRegex) const override;
 
 private:
-  unsigned getContentStartColumn(unsigned LineIndex,
-                                 unsigned TailOffset) const override;
-
   // OriginalPrefix[i] contains the original prefix of line i, including
   // trailing whitespace before the start of the content. The indentation
   // preceding the prefix is not included.
   // For example, if the line is:
   // // content
   // then the original prefix is "// ".
   SmallVector<StringRef, 16> OriginalPrefix;
 

cfe/trunk/lib/Format/BreakableToken.cpp

static StringRef getLineCommentIndentPrefix(StringRef Comment,
   return LongestPrefix;
 }
 
 static BreakableToken::Split getCommentSplit(StringRef Text,
                                              unsigned ContentStartColumn,
                                              unsigned ColumnLimit,
                                              unsigned TabWidth,
                                              encoding::Encoding Encoding) {
+  DEBUG(llvm::dbgs() << "Comment split: \"" << Text << ", " << ColumnLimit
+                     << "\", Content start: " << ContentStartColumn << "\n");
   if (ColumnLimit <= ContentStartColumn + 1)
     return BreakableToken::Split(StringRef::npos, 0);
 
   unsigned MaxSplit = ColumnLimit - ContentStartColumn + 1;
   unsigned MaxSplitBytes = 0;
 
   for (unsigned NumChars = 0;
        NumChars < MaxSplit && MaxSplitBytes < Text.size();) {
bool switchesFormatting(const FormatToken &Token) {
   assert((Token.is(TT_BlockComment) || Token.is(TT_LineComment)) &&
          "formatting regions are switched by comment tokens");
   StringRef Content = Token.TokenText.substr(2).ltrim();
   return Content.startswith("clang-format on") ||
          Content.startswith("clang-format off");
 }
 
 unsigned
-BreakableToken::getLineLengthAfterCompression(unsigned RemainingTokenColumns,
+BreakableToken::getLengthAfterCompression(unsigned RemainingTokenColumns,
                                               Split Split) const {
   // Example: consider the content
   // lala  lala
   // - RemainingTokenColumns is the original number of columns, 10;
   // - Split is (4, 2), denoting the two spaces between the two words;
   //
   // We compute the number of columns when the split is compressed into a single
   // space, like:
   // lala lala
+  //
+  // FIXME: Correctly measure the length of whitespace in Split.second so it
+  // works with tabs.
   return RemainingTokenColumns + 1 - Split.second;
 }
 
-unsigned BreakableSingleLineToken::getLineCount() const { return 1; }
+unsigned BreakableStringLiteral::getLineCount() const { return 1; }
 
-unsigned BreakableSingleLineToken::getLineLengthAfterSplit(
-    unsigned LineIndex, unsigned TailOffset,
-    StringRef::size_type Length) const {
-  return StartColumn + Prefix.size() + Postfix.size() +
-         encoding::columnWidthWithTabs(Line.substr(TailOffset, Length),
-                                       StartColumn + Prefix.size(),
-                                       Style.TabWidth, Encoding);
+unsigned BreakableStringLiteral::getRangeLength(unsigned LineIndex,
+                                                unsigned Offset,
+                                                StringRef::size_type Length,
+                                                unsigned StartColumn) const {
+  assert(false &&
+         "Getting the length of a part of the string literal indicates that "
+         "the code tries to reflow it.");
+}
+
+unsigned
+BreakableStringLiteral::getRemainingLength(unsigned LineIndex, unsigned Offset,
+                                           unsigned StartColumn) const {
+  return UnbreakableTailLength + Postfix.size() +
+         encoding::columnWidthWithTabs(Line.substr(Offset, StringRef::npos),
+                                       StartColumn, Style.TabWidth, Encoding);
+}
+
+unsigned BreakableStringLiteral::getContentStartColumn(unsigned LineIndex,
+                                                       bool Break) const {
+  return StartColumn + Prefix.size();
 }
 
-BreakableSingleLineToken::BreakableSingleLineToken(
+BreakableStringLiteral::BreakableStringLiteral(
     const FormatToken &Tok, unsigned StartColumn, StringRef Prefix,
     StringRef Postfix, bool InPPDirective, encoding::Encoding Encoding,
     const FormatStyle &Style)
     : BreakableToken(Tok, InPPDirective, Encoding, Style),
-      StartColumn(StartColumn), Prefix(Prefix), Postfix(Postfix) {
+      StartColumn(StartColumn), Prefix(Prefix), Postfix(Postfix),
+      UnbreakableTailLength(Tok.UnbreakableTailLength) {
   assert(Tok.TokenText.startswith(Prefix) && Tok.TokenText.endswith(Postfix));
   Line = Tok.TokenText.substr(
       Prefix.size(), Tok.TokenText.size() - Prefix.size() - Postfix.size());
 }
 
-BreakableStringLiteral::BreakableStringLiteral(
-    const FormatToken &Tok, unsigned StartColumn, StringRef Prefix,
-    StringRef Postfix, bool InPPDirective, encoding::Encoding Encoding,
-    const FormatStyle &Style)
-    : BreakableSingleLineToken(Tok, StartColumn, Prefix, Postfix, InPPDirective,
-                               Encoding, Style) {}
-
-BreakableToken::Split
-BreakableStringLiteral::getSplit(unsigned LineIndex, unsigned TailOffset,
-                                 unsigned ColumnLimit,
-                                 llvm::Regex &CommentPragmasRegex) const {
-  return getStringSplit(Line.substr(TailOffset),
-                        StartColumn + Prefix.size() + Postfix.size(),
-                        ColumnLimit, Style.TabWidth, Encoding);
+BreakableToken::Split BreakableStringLiteral::getSplit(
+    unsigned LineIndex, unsigned TailOffset, unsigned ColumnLimit,
+    unsigned ContentStartColumn, llvm::Regex &CommentPragmasRegex) const {
+  return getStringSplit(Line.substr(TailOffset), ContentStartColumn,
+                        ColumnLimit - Postfix.size(), Style.TabWidth, Encoding);
 }
 
 void BreakableStringLiteral::insertBreak(unsigned LineIndex,
                                          unsigned TailOffset, Split Split,
-                                         WhitespaceManager &Whitespaces) {
+                                         WhitespaceManager &Whitespaces) const {
   Whitespaces.replaceWhitespaceInToken(
       Tok, Prefix.size() + TailOffset + Split.first, Split.second, Postfix,
       Prefix, InPPDirective, 1, StartColumn);
 }
 
 BreakableComment::BreakableComment(const FormatToken &Token,
                                    unsigned StartColumn, bool InPPDirective,
                                    encoding::Encoding Encoding,
                                    const FormatStyle &Style)
     : BreakableToken(Token, InPPDirective, Encoding, Style),
       StartColumn(StartColumn) {}
 
 unsigned BreakableComment::getLineCount() const { return Lines.size(); }
 
 BreakableToken::Split
 BreakableComment::getSplit(unsigned LineIndex, unsigned TailOffset,
-                           unsigned ColumnLimit,
+                           unsigned ColumnLimit, unsigned ContentStartColumn,
                            llvm::Regex &CommentPragmasRegex) const {
   // Don't break lines matching the comment pragmas regex.
   if (CommentPragmasRegex.match(Content[LineIndex]))
     return Split(StringRef::npos, 0);
   return getCommentSplit(Content[LineIndex].substr(TailOffset),
-                         getContentStartColumn(LineIndex, TailOffset),
-                         ColumnLimit, Style.TabWidth, Encoding);
+                         ContentStartColumn, ColumnLimit, Style.TabWidth,
+                         Encoding);
 }
 
-void BreakableComment::compressWhitespace(unsigned LineIndex,
-                                          unsigned TailOffset, Split Split,
-                                          WhitespaceManager &Whitespaces) {
+void BreakableComment::compressWhitespace(
+    unsigned LineIndex, unsigned TailOffset, Split Split,
+    WhitespaceManager &Whitespaces) const {
   StringRef Text = Content[LineIndex].substr(TailOffset);
   // Text is relative to the content line, but Whitespaces operates relative to
   // the start of the corresponding token, so compute the start of the Split
   // that needs to be compressed into a single space relative to the start of
   // its token.
   unsigned BreakOffsetInToken =
       Text.data() - tokenAt(LineIndex).TokenText.data() + Split.first;
   unsigned CharsToRemove = Split.second;
   Whitespaces.replaceWhitespaceInToken(
       tokenAt(LineIndex), BreakOffsetInToken, CharsToRemove, "", "",
       /*InPPDirective=*/false, /*Newlines=*/0, /*Spaces=*/1);
 }
 
-BreakableToken::Split
-BreakableComment::getReflowSplit(StringRef Text, StringRef ReflowPrefix,
-                                 unsigned PreviousEndColumn,
-                                 unsigned ColumnLimit) const {
-  unsigned ReflowStartColumn = PreviousEndColumn + ReflowPrefix.size();
-  StringRef TrimmedText = Text.rtrim(Blanks);
-  // This is the width of the resulting line in case the full line of Text gets
-  // reflown up starting at ReflowStartColumn.
-  unsigned FullWidth = ReflowStartColumn + encoding::columnWidthWithTabs(
-                                               TrimmedText, ReflowStartColumn,
-                                               Style.TabWidth, Encoding);
-  // If the full line fits up, we return a reflow split after it,
-  // otherwise we compute the largest piece of text that fits after
-  // ReflowStartColumn.
-  Split ReflowSplit =
-      FullWidth <= ColumnLimit
-          ? Split(TrimmedText.size(), Text.size() - TrimmedText.size())
-          : getCommentSplit(Text, ReflowStartColumn, ColumnLimit,
-                            Style.TabWidth, Encoding);
-
-  // We need to be extra careful here, because while it's OK to keep a long line
-  // if it can't be broken into smaller pieces (like when the first word of a
-  // long line is longer than the column limit), it's not OK to reflow that long
-  // word up. So we recompute the size of the previous line after reflowing and
-  // only return the reflow split if that's under the line limit.
-  if (ReflowSplit.first != StringRef::npos &&
-      // Check if the width of the newly reflown line is under the limit.
-      PreviousEndColumn + ReflowPrefix.size() +
-              encoding::columnWidthWithTabs(Text.substr(0, ReflowSplit.first),
-                                            PreviousEndColumn +
-                                                ReflowPrefix.size(),
-                                            Style.TabWidth, Encoding) <=
-          ColumnLimit) {
-    return ReflowSplit;
-  }
-  return Split(StringRef::npos, 0);
-}
-
 const FormatToken &BreakableComment::tokenAt(unsigned LineIndex) const {
   return Tokens[LineIndex] ? *Tokens[LineIndex] : Tok;
 }
 
 static bool mayReflowContent(StringRef Content) {
   Content = Content.trim(Blanks);
   // Lines starting with '@' commonly have special meaning.
   // Lines starting with '-', '-#', '+' or '*' are bulleted/numbered lists.
void BreakableBlockComment::adjustWhitespace(unsigned LineIndex,
   Content[LineIndex] = Lines[LineIndex].substr(StartOfLine);
 
   // Adjust the start column uniformly across all lines.
   ContentColumn[LineIndex] =
       encoding::columnWidthWithTabs(Whitespace, 0, Style.TabWidth, Encoding) +
       IndentDelta;
 }
 
-unsigned BreakableBlockComment::getLineLengthAfterSplit(
-    unsigned LineIndex, unsigned TailOffset,
-    StringRef::size_type Length) const {
-  unsigned ContentStartColumn = getContentStartColumn(LineIndex, TailOffset);
+unsigned BreakableBlockComment::getRangeLength(unsigned LineIndex,
+                                               unsigned Offset,
+                                               StringRef::size_type Length,
+                                               unsigned StartColumn) const {
   unsigned LineLength =
-      ContentStartColumn + encoding::columnWidthWithTabs(
-                               Content[LineIndex].substr(TailOffset, Length),
-                               ContentStartColumn, Style.TabWidth, Encoding);
+      encoding::columnWidthWithTabs(Content[LineIndex].substr(Offset, Length),
+                                    StartColumn, Style.TabWidth, Encoding);
+  // FIXME: This should go into getRemainingLength instead, but we currently
+  // break tests when putting it there. Investigate how to fix those tests.
   // The last line gets a "*/" postfix.
   if (LineIndex + 1 == Lines.size()) {
     LineLength += 2;
     // We never need a decoration when breaking just the trailing "*/" postfix.
     // Note that checking that Length == 0 is not enough, since Length could
     // also be StringRef::npos.
-    if (Content[LineIndex].substr(TailOffset, Length).empty()) {
+    if (Content[LineIndex].substr(Offset, StringRef::npos).empty()) {
       LineLength -= Decoration.size();
     }
   }
   return LineLength;
 }
 
+unsigned BreakableBlockComment::getRemainingLength(unsigned LineIndex,
+                                                   unsigned Offset,
+                                                   unsigned StartColumn) const {
+  return getRangeLength(LineIndex, Offset, StringRef::npos, StartColumn);
+}
+
+unsigned BreakableBlockComment::getContentStartColumn(unsigned LineIndex,
+                                                      bool Break) const {
+  if (Break)
+    return IndentAtLineBreak;
+  return std::max(0, ContentColumn[LineIndex]);
+}
+
 void BreakableBlockComment::insertBreak(unsigned LineIndex, unsigned TailOffset,
                                         Split Split,
-                                        WhitespaceManager &Whitespaces) {
+                                        WhitespaceManager &Whitespaces) const {
   StringRef Text = Content[LineIndex].substr(TailOffset);
   StringRef Prefix = Decoration;
   // We need this to account for the case when we have a decoration "* " for all
   // the lines except for the last one, where the star in "*/" acts as a
   // decoration.
   unsigned LocalIndentAtLineBreak = IndentAtLineBreak;
   if (LineIndex + 1 == Lines.size() &&
       Text.size() == Split.first + Split.second) {
void BreakableBlockComment::insertBreak(unsigned LineIndex, unsigned TailOffset,
   unsigned CharsToRemove = Split.second;
   assert(LocalIndentAtLineBreak >= Prefix.size());
   Whitespaces.replaceWhitespaceInToken(
       tokenAt(LineIndex), BreakOffsetInToken, CharsToRemove, "", Prefix,
       InPPDirective, /*Newlines=*/1,
       /*Spaces=*/LocalIndentAtLineBreak - Prefix.size());
 }
 
-BreakableToken::Split BreakableBlockComment::getSplitBefore(
-    unsigned LineIndex, unsigned PreviousEndColumn, unsigned ColumnLimit,
+BreakableToken::Split
+BreakableBlockComment::getReflowSplit(unsigned LineIndex,
                                       llvm::Regex &CommentPragmasRegex) const {
   if (!mayReflow(LineIndex, CommentPragmasRegex))
     return Split(StringRef::npos, 0);
-  StringRef TrimmedContent = Content[LineIndex].ltrim(Blanks);
-  Split Result = getReflowSplit(TrimmedContent, ReflowPrefix, PreviousEndColumn,
-                                ColumnLimit);
-  // Result is relative to TrimmedContent. Adapt it relative to
-  // Content[LineIndex].
-  if (Result.first != StringRef::npos)
-    Result.first += Content[LineIndex].size() - TrimmedContent.size();
-  return Result;
-}
 
-unsigned
-BreakableBlockComment::getReflownColumn(StringRef Content, unsigned LineIndex,
-                                        unsigned PreviousEndColumn) const {
-  unsigned StartColumn = PreviousEndColumn + ReflowPrefix.size();
-  // If this is the last line, it will carry around its '*/' postfix.
-  unsigned PostfixLength = (LineIndex + 1 == Lines.size() ? 2 : 0);
-  // The line is composed of previous text, reflow prefix, reflown text and
-  // postfix.
-  unsigned ReflownColumn = StartColumn +
-                           encoding::columnWidthWithTabs(
-                               Content, StartColumn, Style.TabWidth, Encoding) +
-                           PostfixLength;
-  return ReflownColumn;
-}
-
-unsigned BreakableBlockComment::getLineLengthAfterSplitBefore(
-    unsigned LineIndex, unsigned TailOffset, unsigned PreviousEndColumn,
-    unsigned ColumnLimit, Split SplitBefore) const {
-  if (SplitBefore.first == StringRef::npos ||
-      // Block comment line contents contain the trailing whitespace after the
-      // decoration, so the need of left trim. Note that this behavior is
-      // consistent with the breaking of block comments where the indentation of
-      // a broken line is uniform across all the lines of the block comment.
-      SplitBefore.first + SplitBefore.second <
-          Content[LineIndex].ltrim().size()) {
-    // A piece of line, not the whole, gets reflown.
-    return getLineLengthAfterSplit(LineIndex, TailOffset, StringRef::npos);
-  } else {
-    // The whole line gets reflown, need to check if we need to insert a break
-    // for the postfix or not.
-    StringRef TrimmedContent = Content[LineIndex].ltrim(Blanks);
-    unsigned ReflownColumn =
-        getReflownColumn(TrimmedContent, LineIndex, PreviousEndColumn);
-    if (ReflownColumn <= ColumnLimit) {
-      return ReflownColumn;
-    }
-    return getLineLengthAfterSplit(LineIndex, TailOffset, StringRef::npos);
-  }
+  size_t Trimmed = Content[LineIndex].find_first_not_of(Blanks);
+  return Split(0, Trimmed != StringRef::npos ? Trimmed : 0);
 }
 
 bool BreakableBlockComment::introducesBreakBeforeToken() const {
   // A break is introduced when we want delimiters on newline.
   return DelimitersOnNewline &&
          Lines[0].substr(1).find_first_not_of(Blanks) != StringRef::npos;
 }
 
-void BreakableBlockComment::replaceWhitespaceBefore(
-    unsigned LineIndex, unsigned PreviousEndColumn, unsigned ColumnLimit,
-    Split SplitBefore, WhitespaceManager &Whitespaces) {
-  if (LineIndex == 0) {
-    if (DelimitersOnNewline) {
-      // Since we're breaking af index 1 below, the break position and the
-      // break length are the same.
-      size_t BreakLength = Lines[0].substr(1).find_first_not_of(Blanks);
-      if (BreakLength != StringRef::npos) {
-        insertBreak(LineIndex, 0, Split(1, BreakLength), Whitespaces);
-        DelimitersOnNewline = true;
-      }
-    }
-    return;
-  }
+void BreakableBlockComment::reflow(unsigned LineIndex,
+                                   WhitespaceManager &Whitespaces) const {
   StringRef TrimmedContent = Content[LineIndex].ltrim(Blanks);
-  if (SplitBefore.first != StringRef::npos) {
-    // Here we need to reflow.
-    assert(Tokens[LineIndex - 1] == Tokens[LineIndex] &&
-           "Reflowing whitespace within a token");
-    // This is the offset of the end of the last line relative to the start of
-    // the token text in the token.
-    unsigned WhitespaceOffsetInToken = Content[LineIndex - 1].data() +
-                                       Content[LineIndex - 1].size() -
-                                       tokenAt(LineIndex).TokenText.data();
-    unsigned WhitespaceLength = TrimmedContent.data() -
-                                tokenAt(LineIndex).TokenText.data() -
-                                WhitespaceOffsetInToken;
-    Whitespaces.replaceWhitespaceInToken(
-        tokenAt(LineIndex), WhitespaceOffsetInToken,
-        /*ReplaceChars=*/WhitespaceLength, /*PreviousPostfix=*/"",
-        /*CurrentPrefix=*/ReflowPrefix, InPPDirective, /*Newlines=*/0,
-        /*Spaces=*/0);
-    // Check if we need to also insert a break at the whitespace range.
-    // Note that we don't need a penalty for this break, since it doesn't change
-    // the total number of lines.
-    unsigned ReflownColumn =
-        getReflownColumn(TrimmedContent, LineIndex, PreviousEndColumn);
-    if (ReflownColumn > ColumnLimit)
-      insertBreak(LineIndex, 0, SplitBefore, Whitespaces);
-    return;
-  }
 
+void BreakableBlockComment::adaptStartOfLine(
+    unsigned LineIndex, WhitespaceManager &Whitespaces) const {
+  if (LineIndex == 0) {
+    if (DelimitersOnNewline) {
+      // Since we're breaking at index 1 below, the break position and the
+      // break length are the same.
+      size_t BreakLength = Lines[0].substr(1).find_first_not_of(Blanks);
+      if (BreakLength != StringRef::npos)
+        insertBreak(LineIndex, 0, Split(1, BreakLength), Whitespaces);
+    }
+    return;
+  }
   // Here no reflow with the previous line will happen.
   // Fix the decoration of the line at LineIndex.
   StringRef Prefix = Decoration;
   if (Content[LineIndex].empty()) {
     if (LineIndex + 1 == Lines.size()) {
       if (!LastLineNeedsDecoration) {
         // If the last line was empty, we don't need a prefix, as the */ will
         // line up with the decoration (if it exists).
unsigned WhitespaceLength = Content[LineIndex].data() -
                               tokenAt(LineIndex).TokenText.data() -
                               WhitespaceOffsetInToken;
   Whitespaces.replaceWhitespaceInToken(
       tokenAt(LineIndex), WhitespaceOffsetInToken, WhitespaceLength, "", Prefix,
       InPPDirective, /*Newlines=*/1, ContentColumn[LineIndex] - Prefix.size());
 }
 
 BreakableToken::Split
-BreakableBlockComment::getSplitAfterLastLine(unsigned TailOffset,
-                                             unsigned ColumnLimit) const {
+BreakableBlockComment::getSplitAfterLastLine(unsigned TailOffset) const {
   if (DelimitersOnNewline) {
     // Replace the trailing whitespace of the last line with a newline.
     // In case the last line is empty, the ending '*/' is already on its own
     // line.
     StringRef Line = Content.back().substr(TailOffset);
     StringRef TrimmedLine = Line.rtrim(Blanks);
     if (!TrimmedLine.empty())
       return Split(TrimmedLine.size(), Line.size() - TrimmedLine.size());
bool BreakableBlockComment::mayReflow(unsigned LineIndex,
   if (Lines[LineIndex].ltrim(Blanks).startswith("*")) {
     IndentContent = Lines[LineIndex].ltrim(Blanks).substr(1);
   }
   return LineIndex > 0 && !CommentPragmasRegex.match(IndentContent) &&
          mayReflowContent(Content[LineIndex]) && !Tok.Finalized &&
          !switchesFormatting(tokenAt(LineIndex));
 }
 
-unsigned
-BreakableBlockComment::getContentStartColumn(unsigned LineIndex,
-                                             unsigned TailOffset) const {
-  // If we break, we always break at the predefined indent.
-  if (TailOffset != 0)
-    return IndentAtLineBreak;
-  return std::max(0, ContentColumn[LineIndex]);
-}
-
 BreakableLineCommentSection::BreakableLineCommentSection(
     const FormatToken &Token, unsigned StartColumn,
     unsigned OriginalStartColumn, bool FirstInLine, bool InPPDirective,
     encoding::Encoding Encoding, const FormatStyle &Style)
     : BreakableComment(Token, StartColumn, InPPDirective, Encoding, Style) {
   assert(Tok.is(TT_LineComment) &&
          "line comment section must start with a line comment");
   FormatToken *LineTok = nullptr;
if (CurrentTok->Next && !CurrentTok->Next->ContinuesLineCommentSection) {
       //
       // FIXME: Consider putting separate line comment sections as children to
       // the unwrapped line instead.
       break;
     }
   }
 }
 
-unsigned BreakableLineCommentSection::getLineLengthAfterSplit(
-    unsigned LineIndex, unsigned TailOffset,
-    StringRef::size_type Length) const {
-  unsigned ContentStartColumn =
-      (TailOffset == 0 ? ContentColumn[LineIndex]
-                       : OriginalContentColumn[LineIndex]);
-  return ContentStartColumn + encoding::columnWidthWithTabs(
-                                  Content[LineIndex].substr(TailOffset, Length),
-                                  ContentStartColumn, Style.TabWidth, Encoding);
+unsigned
+BreakableLineCommentSection::getRangeLength(unsigned LineIndex, unsigned Offset,
+                                            StringRef::size_type Length,
+                                            unsigned StartColumn) const {
+  return encoding::columnWidthWithTabs(
+      Content[LineIndex].substr(Offset, Length), StartColumn, Style.TabWidth,
+      Encoding);
 }
 
-void BreakableLineCommentSection::insertBreak(unsigned LineIndex,
-                                              unsigned TailOffset, Split Split,
-                                              WhitespaceManager &Whitespaces) {
+unsigned BreakableLineCommentSection::getContentStartColumn(unsigned LineIndex,
+                                                            bool Break) const {
+  if (Break)
+    return OriginalContentColumn[LineIndex];
+  return ContentColumn[LineIndex];
+}
+
+void BreakableLineCommentSection::insertBreak(
+    unsigned LineIndex, unsigned TailOffset, Split Split,
+    WhitespaceManager &Whitespaces) const {
   StringRef Text = Content[LineIndex].substr(TailOffset);
   // Compute the offset of the split relative to the beginning of the token
   // text.
   unsigned BreakOffsetInToken =
       Text.data() - tokenAt(LineIndex).TokenText.data() + Split.first;
   unsigned CharsToRemove = Split.second;
   // Compute the size of the new indent, including the size of the new prefix of
   // the newly broken line.
   unsigned IndentAtLineBreak = OriginalContentColumn[LineIndex] +
                                Prefix[LineIndex].size() -
                                OriginalPrefix[LineIndex].size();
   assert(IndentAtLineBreak >= Prefix[LineIndex].size());
   Whitespaces.replaceWhitespaceInToken(
       tokenAt(LineIndex), BreakOffsetInToken, CharsToRemove, "",
       Prefix[LineIndex], InPPDirective, /*Newlines=*/1,
       /*Spaces=*/IndentAtLineBreak - Prefix[LineIndex].size());
 }
 
-BreakableComment::Split BreakableLineCommentSection::getSplitBefore(
-    unsigned LineIndex, unsigned PreviousEndColumn, unsigned ColumnLimit,
-    llvm::Regex &CommentPragmasRegex) const {
+BreakableComment::Split BreakableLineCommentSection::getReflowSplit(
+    unsigned LineIndex, llvm::Regex &CommentPragmasRegex) const {
   if (!mayReflow(LineIndex, CommentPragmasRegex))
     return Split(StringRef::npos, 0);
-  return getReflowSplit(Content[LineIndex], ReflowPrefix, PreviousEndColumn,
-                        ColumnLimit);
-}
 
-unsigned BreakableLineCommentSection::getLineLengthAfterSplitBefore(
-    unsigned LineIndex, unsigned TailOffset, unsigned PreviousEndColumn,
-    unsigned ColumnLimit, Split SplitBefore) const {
-  if (SplitBefore.first == StringRef::npos ||
-      SplitBefore.first + SplitBefore.second < Content[LineIndex].size()) {
-    // A piece of line, not the whole line, gets reflown.
-    return getLineLengthAfterSplit(LineIndex, TailOffset, StringRef::npos);
-  } else {
-    // The whole line gets reflown.
-    unsigned StartColumn = PreviousEndColumn + ReflowPrefix.size();
-    return StartColumn +
-           encoding::columnWidthWithTabs(Content[LineIndex], StartColumn,
-                                         Style.TabWidth, Encoding);
-  }
+  size_t Trimmed = Content[LineIndex].find_first_not_of(Blanks);
+
+  // In a line comment section each line is a separate token; thus, after a
+  // split we replace all whitespace before the current line comment token
+  // (which does not need to be included in the split), plus the start of the
+  // line up to where the content starts.
+  return Split(0, Trimmed != StringRef::npos ? Trimmed : 0);
 }
 
-void BreakableLineCommentSection::replaceWhitespaceBefore(
-    unsigned LineIndex, unsigned PreviousEndColumn, unsigned ColumnLimit,
-    Split SplitBefore, WhitespaceManager &Whitespaces) {
-  // If this is the first line of a token, we need to inform Whitespace Manager
-  // about it: either adapt the whitespace range preceding it, or mark it as an
-  // untouchable token.
-  // This happens for instance here:
-  // // line 1 \
-  // // line 2
-  if (LineIndex > 0 && Tokens[LineIndex] != Tokens[LineIndex - 1]) {
-    if (SplitBefore.first != StringRef::npos) {
-      // Reflow happens between tokens. Replace the whitespace between the
-      // tokens by the empty string.
-      Whitespaces.replaceWhitespace(
-          *Tokens[LineIndex], /*Newlines=*/0, /*Spaces=*/0,
-          /*StartOfTokenColumn=*/StartColumn, /*InPPDirective=*/false);
-      // Replace the indent and prefix of the token with the reflow prefix.
-      unsigned WhitespaceLength =
-          Content[LineIndex].data() - tokenAt(LineIndex).TokenText.data();
-      Whitespaces.replaceWhitespaceInToken(*Tokens[LineIndex],
-                                           /*Offset=*/0,
-                                           /*ReplaceChars=*/WhitespaceLength,
-                                           /*PreviousPostfix=*/"",
-                                           /*CurrentPrefix=*/ReflowPrefix,
-                                           /*InPPDirective=*/false,
-                                           /*Newlines=*/0,
-                                           /*Spaces=*/0);
-    } else {
-      // This is the first line for the current token, but no reflow with the
-      // previous token is necessary. However, we still may need to adjust the
-      // start column. Note that ContentColumn[LineIndex] is the expected
-      // content column after a possible update to the prefix, hence the prefix
-      // length change is included.
-      unsigned LineColumn =
-          ContentColumn[LineIndex] -
-          (Content[LineIndex].data() - Lines[LineIndex].data()) +
-          (OriginalPrefix[LineIndex].size() - Prefix[LineIndex].size());
+void BreakableLineCommentSection::reflow(unsigned LineIndex,
+                                         WhitespaceManager &Whitespaces) const {
+}
+
+void BreakableLineCommentSection::adaptStartOfLine(
+    unsigned LineIndex, WhitespaceManager &Whitespaces) const {
+  // If this is the first line of a token, we need to inform Whitespace Manager
+  // about it: either adapt the whitespace range preceding it, or mark it as an
+  // untouchable token.
+  // This happens for instance here:
+  // // line 1 \
+  // // line 2
+  if (LineIndex > 0 && Tokens[LineIndex] != Tokens[LineIndex - 1]) {
 
-      // We always want to create a replacement instead of adding an untouchable
-      // token, even if LineColumn is the same as the original column of the
-      // token. This is because WhitespaceManager doesn't align trailing
-      // comments if they are untouchable.
-      Whitespaces.replaceWhitespace(*Tokens[LineIndex],
-                                    /*Newlines=*/1,
-                                    /*Spaces=*/LineColumn,
-                                    /*StartOfTokenColumn=*/LineColumn,
-                                    /*InPPDirective=*/false);
-    }
-  }
   if (OriginalPrefix[LineIndex] != Prefix[LineIndex]) {
     // Adjust the prefix if necessary.
 
     // Take care of the space possibly introduced after a decoration.
     assert(Prefix[LineIndex] == (OriginalPrefix[LineIndex] + " ").str() &&
            "Expecting a line comment prefix to differ from original by at most "
            "a space");
     Whitespaces.replaceWhitespaceInToken(
         tokenAt(LineIndex), OriginalPrefix[LineIndex].size(), 0, "", "",
         /*InPPDirective=*/false, /*Newlines=*/0, /*Spaces=*/1);
   }
-  // Add a break after a reflow split has been introduced, if necessary.
-  // Note that this break doesn't need to be penalized, since it doesn't change
-  // the number of lines.
-  if (SplitBefore.first != StringRef::npos &&
-      SplitBefore.first + SplitBefore.second < Content[LineIndex].size()) {
-    insertBreak(LineIndex, 0, SplitBefore, Whitespaces);
-  }
 }
 
 void BreakableLineCommentSection::updateNextToken(LineState &State) const {
   if (LastLineTok) {
     State.NextToken = LastLineTok->Next;
   }
 }
 
 bool BreakableLineCommentSection::mayReflow(
     unsigned LineIndex, llvm::Regex &CommentPragmasRegex) const {
   // Line comments have the indent as part of the prefix, so we need to
   // recompute the start of the line.
   StringRef IndentContent = Content[LineIndex];
   if (Lines[LineIndex].startswith("//")) {
     IndentContent = Lines[LineIndex].substr(2);
   }
+  // FIXME: Decide whether we want to reflow non-regular indents:
+  // Currently, we only reflow when the OriginalPrefix[LineIndex] matches the
+  // OriginalPrefix[LineIndex-1]. That means we don't reflow
+  // // text that protrudes
+  // //    into text with different indent
+  // We do reflow in that case in block comments.
   return LineIndex > 0 && !CommentPragmasRegex.match(IndentContent) &&
          mayReflowContent(Content[LineIndex]) && !Tok.Finalized &&
          !switchesFormatting(tokenAt(LineIndex)) &&
          OriginalPrefix[LineIndex] == OriginalPrefix[LineIndex - 1];
 }
 
-unsigned
-BreakableLineCommentSection::getContentStartColumn(unsigned LineIndex,
-                                                   unsigned TailOffset) const {
-  if (TailOffset != 0) {
-    return OriginalContentColumn[LineIndex];
-  }
-  return ContentColumn[LineIndex];
-}
-
 } // namespace format
 } // namespace clang

cfe/trunk/lib/Format/ContinuationIndenter.cpp

std::unique_ptr<BreakableToken> ContinuationIndenter::createBreakableToken(
   }
   return nullptr;
 }
 
 unsigned ContinuationIndenter::breakProtrudingToken(const FormatToken &Current,
                                                     LineState &State,
                                                     bool AllowBreak,
                                                     bool DryRun) {
-  std::unique_ptr<BreakableToken> Token =
+  std::unique_ptr<const BreakableToken> Token =
       createBreakableToken(Current, State, AllowBreak);
   if (!Token)
     return 0;
+  assert(Token->getLineCount() > 0);
   unsigned ColumnLimit = getColumnLimit(State);
-  unsigned StartColumn = State.Column - Current.ColumnWidth;
   if (Current.is(TT_LineComment)) {
     // We don't insert backslashes when breaking line comments.
     ColumnLimit = Style.ColumnLimit;
   }
   if (Current.UnbreakableTailLength >= ColumnLimit)
     return 0;
-
+  // ColumnWidth was already accounted into State.Column before calling
+  // breakProtrudingToken.
+  unsigned StartColumn = State.Column - Current.ColumnWidth;
   unsigned NewBreakPenalty = Current.isStringLiteral()
                                  ? Style.PenaltyBreakString
                                  : Style.PenaltyBreakComment;
-  unsigned RemainingSpace = ColumnLimit - Current.UnbreakableTailLength;
+  // Stores whether we introduce a break anywhere in the token.
   bool BreakInserted = Token->introducesBreakBeforeToken();
   // Store whether we inserted a new line break at the end of the previous
   // logical line.
   bool NewBreakBefore = false;
   // We use a conservative reflowing strategy. Reflow starts after a line is
   // broken or the corresponding whitespace compressed. Reflow ends as soon as a
   // line that doesn't get reflown with the previous line is reached.
-  bool ReflowInProgress = false;
-  unsigned Penalty = 0;
-  unsigned RemainingTokenColumns = 0;
+  bool Reflow = false;
+  // Keep track of where we are in the token:
+  // Where we are in the content of the current logical line.
   unsigned TailOffset = 0;
+  // The column number we're currently at.
+  unsigned ContentStartColumn =
+      Token->getContentStartColumn(0, /*Break=*/false);
+  // The number of columns left in the current logical line after TailOffset.
+  unsigned RemainingTokenColumns =
+      Token->getRemainingLength(0, TailOffset, ContentStartColumn);
+  // Adapt the start of the token, for example indent.
+  if (!DryRun)
+    Token->adaptStartOfLine(0, Whitespaces);
+
+  unsigned Penalty = 0;
   DEBUG(llvm::dbgs() << "Breaking protruding token at column " << StartColumn
                      << ".\n");
   for (unsigned LineIndex = 0, EndIndex = Token->getLineCount();
        LineIndex != EndIndex; ++LineIndex) {
-    DEBUG(llvm::dbgs() << "  Line: " << LineIndex
-                       << " (Reflow: " << ReflowInProgress << ")\n");
-    BreakableToken::Split SplitBefore(StringRef::npos, 0);
-    if (ReflowInProgress) {
-      SplitBefore = Token->getSplitBefore(LineIndex, RemainingTokenColumns,
-                                          RemainingSpace, CommentPragmasRegex);
-    }
-    ReflowInProgress = SplitBefore.first != StringRef::npos;
-    DEBUG({
-      if (ReflowInProgress)
-        llvm::dbgs() << "  Reflowing.\n";
-    });
-    TailOffset =
-        ReflowInProgress ? (SplitBefore.first + SplitBefore.second) : 0;
-    // If we found a reflow split and have added a new break before this line,
-    // we are going to remove the line break at the start of the next logical
-    // line.
-    // For example, here we'll add a new line break after 'text', and
-    // subsequently delete the line break between 'that' and 'reflows'.
-    //   // some text that
-    //   // reflows
-    // ->
-    //   // some text
-    //   // that reflows
-    // When adding the line break, we also added the penalty for it, so we need
-    // to subtract that penalty again when we remove the line break due to
-    // reflowing.
-    if (ReflowInProgress && NewBreakBefore) {
-      assert(Penalty >= NewBreakPenalty);
-      Penalty -= NewBreakPenalty;
-    }
+    DEBUG(llvm::dbgs() << "  Line: " << LineIndex << " (Reflow: " << Reflow
+                       << ")\n");
     NewBreakBefore = false;
-    if (!DryRun)
-      Token->replaceWhitespaceBefore(LineIndex, RemainingTokenColumns,
-                                     RemainingSpace, SplitBefore, Whitespaces);
-    RemainingTokenColumns = Token->getLineLengthAfterSplitBefore(
-        LineIndex, TailOffset, RemainingTokenColumns, ColumnLimit, SplitBefore);
-    while (RemainingTokenColumns > RemainingSpace) {
-      DEBUG(llvm::dbgs() << "    Over limit, need: " << RemainingTokenColumns
-                         << ", space: " << RemainingSpace << "\n");
-      BreakableToken::Split Split = Token->getSplit(
-          LineIndex, TailOffset, ColumnLimit, CommentPragmasRegex);
+    // If we did reflow the previous line, we'll try reflowing again. Otherwise
+    // we'll start reflowing if the current line is broken or whitespace is
+    // compressed.
+    bool TryReflow = Reflow;
+    // Break the current token until we can fit the rest of the line.
+    while (ContentStartColumn + RemainingTokenColumns > ColumnLimit) {
+      DEBUG(llvm::dbgs() << "    Over limit, need: "
+                         << (ContentStartColumn + RemainingTokenColumns)
+                         << ", space: " << ColumnLimit
+                         << ", reflown prefix: " << ContentStartColumn
+                         << ", offset in line: " << TailOffset << "\n");
+      // If the current token doesn't fit, find the latest possible split in the
+      // current line so that breaking at it will be under the column limit.
+      // FIXME: Use the earliest possible split while reflowing to correctly
+      // compress whitespace within a line.
+      BreakableToken::Split Split =
+          Token->getSplit(LineIndex, TailOffset, ColumnLimit,
+                          ContentStartColumn, CommentPragmasRegex);
       if (Split.first == StringRef::npos) {
-        // The last line's penalty is handled in addNextStateToQueue().
+        // No break opportunity - update the penalty and continue with the next
+        // logical line.
         if (LineIndex < EndIndex - 1)
+          // The last line's penalty is handled in addNextStateToQueue().
           Penalty += Style.PenaltyExcessCharacter *
-                     (RemainingTokenColumns - RemainingSpace);
+                     (ContentStartColumn + RemainingTokenColumns - ColumnLimit);
         DEBUG(llvm::dbgs() << "    No break opportunity.\n");
         break;
       }
       assert(Split.first != 0);
 
-      // Check if compressing the whitespace range will bring the line length
-      // under the limit. If that is the case, we perform whitespace compression
-      // instead of inserting a line break.
-      unsigned RemainingTokenColumnsAfterCompression =
-          Token->getLineLengthAfterCompression(RemainingTokenColumns, Split);
-      if (RemainingTokenColumnsAfterCompression <= RemainingSpace) {
-        RemainingTokenColumns = RemainingTokenColumnsAfterCompression;
-        ReflowInProgress = true;
-        if (!DryRun)
-          Token->compressWhitespace(LineIndex, TailOffset, Split, Whitespaces);
-        DEBUG(llvm::dbgs() << "    Compressing below limit.\n");
-        break;
-      }
-
-      // Compute both the penalties for:
-      // - not breaking, and leaving excess characters
-      // - adding a new line break
-      assert(RemainingTokenColumnsAfterCompression > RemainingSpace);
-      unsigned ExcessCharactersPenalty =
-          (RemainingTokenColumnsAfterCompression - RemainingSpace) *
+      if (Token->supportsReflow()) {
+        // Check whether the next natural split point after the current one can
+        // still fit the line, either because we can compress away whitespace,
+        // or because the penalty the excess characters introduce is lower than
+        // the break penalty.
+        // We only do this for tokens that support reflowing, and thus allow us
+        // to change the whitespace arbitrarily (e.g. comments).
+        // Other tokens, like string literals, can be broken on arbitrary
+        // positions.
+
+        // First, compute the columns from TailOffset to the next possible split
+        // position.
+        // For example:
+        // ColumnLimit:     |
+        // // Some text   that    breaks
+        //    ^ tail offset
+        //             ^-- split
+        //    ^-------- to split columns
+        //                    ^--- next split
+        //    ^--------------- to next split columns
+        unsigned ToSplitColumns = Token->getRangeLength(
+            LineIndex, TailOffset, Split.first, ContentStartColumn);
+        DEBUG(llvm::dbgs() << "    ToSplit: " << ToSplitColumns << "\n");
+
+        BreakableToken::Split NextSplit = Token->getSplit(
+            LineIndex, TailOffset + Split.first + Split.second, ColumnLimit,
+            ContentStartColumn + ToSplitColumns + 1, CommentPragmasRegex);
+        // Compute the columns necessary to fit the next non-breakable sequence
+        // into the current line.
+        unsigned ToNextSplitColumns = 0;
+        if (NextSplit.first == StringRef::npos) {
+          ToNextSplitColumns = Token->getRemainingLength(LineIndex, TailOffset,
+                                                         ContentStartColumn);
+        } else {
+          ToNextSplitColumns = Token->getRangeLength(
+              LineIndex, TailOffset,
+              Split.first + Split.second + NextSplit.first, ContentStartColumn);
+        }
+        // Compress the whitespace between the break and the start of the next
+        // unbreakable sequence.
+        ToNextSplitColumns =
+            Token->getLengthAfterCompression(ToNextSplitColumns, Split);
+        DEBUG(llvm::dbgs() << "    ContentStartColumn: " << ContentStartColumn
+                           << "\n");
+        DEBUG(llvm::dbgs() << "    ToNextSplit: " << ToNextSplitColumns << "\n");
+        // If the whitespace compression makes us fit, continue on the current
+        // line.
+        bool ContinueOnLine =
+            ContentStartColumn + ToNextSplitColumns <= ColumnLimit;
+        unsigned ExcessCharactersPenalty = 0;
+        if (!ContinueOnLine) {
+          // Similarly, if the excess characters' penalty is lower than the
+          // penalty of introducing a new break, continue on the current line.
+          ExcessCharactersPenalty =
+              (ContentStartColumn + ToNextSplitColumns - ColumnLimit) *
               Style.PenaltyExcessCharacter;
-
-      unsigned BreakPenalty = NewBreakPenalty;
-      unsigned ColumnsUsed =
-          Token->getLineLengthAfterSplit(LineIndex, TailOffset, Split.first);
-      if (ColumnsUsed > ColumnLimit)
-        BreakPenalty +=
-            Style.PenaltyExcessCharacter * (ColumnsUsed - ColumnLimit);
-
-      DEBUG(llvm::dbgs() << "    Penalty excess: " << ExcessCharactersPenalty
-                         << "\n            break : " << BreakPenalty << "\n");
-      // Only continue to add the line break if the penalty of the excess
-      // characters is larger than the penalty of the line break.
-      // FIXME: This does not take into account when we can later remove the
-      // line break again due to a reflow.
-      if (ExcessCharactersPenalty < BreakPenalty) {
+          DEBUG(llvm::dbgs()
+                << "    Penalty excess: " << ExcessCharactersPenalty
+                << "\n            break : " << NewBreakPenalty << "\n");
+          if (ExcessCharactersPenalty < NewBreakPenalty)
+            ContinueOnLine = true;
+        }
+        if (ContinueOnLine) {
+          DEBUG(llvm::dbgs() << "    Continuing on line...\n");
+          // The current line fits after compressing the whitespace - reflow
+          // the next line into it if possible.
+          TryReflow = true;
           if (!DryRun)
-          Token->compressWhitespace(LineIndex, TailOffset, Split, Whitespaces);
-        // Do not set ReflowInProgress: we do not have any space left to
-        // reflow into.
+            Token->compressWhitespace(LineIndex, TailOffset, Split,
+                                      Whitespaces);
+          // When we continue on the same line, leave one space between content.
+          ContentStartColumn += ToSplitColumns + 1;
           Penalty += ExcessCharactersPenalty;
-        break;
+          TailOffset += Split.first + Split.second;
+          RemainingTokenColumns = Token->getRemainingLength(
+              LineIndex, TailOffset, ContentStartColumn);
+          continue;
         }
-
-      unsigned NewRemainingTokenColumns = Token->getLineLengthAfterSplit(
-          LineIndex, TailOffset + Split.first + Split.second, StringRef::npos);
+      }
+      DEBUG(llvm::dbgs() << "    Breaking...\n");
+      ContentStartColumn =
+          Token->getContentStartColumn(LineIndex, /*Break=*/true);
+      unsigned NewRemainingTokenColumns = Token->getRemainingLength(
+          LineIndex, TailOffset + Split.first + Split.second,
+          ContentStartColumn);
 
       // When breaking before a tab character, it may be moved by a few columns,
       // but will still be expanded to the next tab stop, so we don't save any
       // columns.
-      if (NewRemainingTokenColumns == RemainingTokenColumns)
+      if (NewRemainingTokenColumns == RemainingTokenColumns) {
         // FIXME: Do we need to adjust the penalty?
         break;
+      }
       assert(NewRemainingTokenColumns < RemainingTokenColumns);
 
+      DEBUG(llvm::dbgs() << "    Breaking at: " << TailOffset + Split.first
+                         << ", " << Split.second << "\n");
       if (!DryRun)
         Token->insertBreak(LineIndex, TailOffset, Split, Whitespaces);
 
-      Penalty += BreakPenalty;
+      Penalty += NewBreakPenalty;
       TailOffset += Split.first + Split.second;
       RemainingTokenColumns = NewRemainingTokenColumns;
-      ReflowInProgress = true;
       BreakInserted = true;
       NewBreakBefore = true;
     }
+    // In case there's another line, prepare the state for the start of the next
+    // line.
+    if (LineIndex + 1 != EndIndex) {
+      unsigned NextLineIndex = LineIndex + 1;
+      if (NewBreakBefore)
+        // After breaking a line, try to reflow the next line into the current
+        // one once RemainingTokenColumns fits.
+        TryReflow = true;
+      if (TryReflow) {
+        // We decided that we want to try reflowing the next line into the
+        // current one.
+        // We will now adjust the state as if the reflow is successful (in
+        // preparation for the next line), and see whether that works. If we
+        // decide that we cannot reflow, we will later reset the state to the
+        // start of the next line.
+        Reflow = false;
+        // As we did not continue breaking the line, RemainingTokenColumns is
+        // known to fit after ContentStartColumn. Adapt ContentStartColumn to
+        // the position at which we want to format the next line if we do
+        // actually reflow.
+        // When we reflow, we need to add a space between the end of the current
+        // line and the next line's start column.
+        ContentStartColumn += RemainingTokenColumns + 1;
+        // Get the split that we need to reflow next logical line into the end
+        // of the current one; the split will include any leading whitespace of
+        // the next logical line.
+        BreakableToken::Split SplitBeforeNext =
+            Token->getReflowSplit(NextLineIndex, CommentPragmasRegex);
+        DEBUG(llvm::dbgs() << "    Size of reflown text: " << ContentStartColumn
+                           << "\n    Potential reflow split: ");
+        if (SplitBeforeNext.first != StringRef::npos) {
+          DEBUG(llvm::dbgs() << SplitBeforeNext.first << ", "
+                             << SplitBeforeNext.second << "\n");
+          TailOffset = SplitBeforeNext.first + SplitBeforeNext.second;
+          // If the rest of the next line fits into the current line below the
+          // column limit, we can safely reflow.
+          RemainingTokenColumns = Token->getRemainingLength(
+              NextLineIndex, TailOffset, ContentStartColumn);
+          Reflow = true;
+          if (ContentStartColumn + RemainingTokenColumns > ColumnLimit) {
+            DEBUG(llvm::dbgs() << "    Over limit after reflow, need: "
+                               << (ContentStartColumn + RemainingTokenColumns)
+                               << ", space: " << ColumnLimit
+                               << ", reflown prefix: " << ContentStartColumn
+                               << ", offset in line: " << TailOffset << "\n");
+            // If the whole next line does not fit, try to find a point in
+            // the next line at which we can break so that attaching the part
+            // of the next line to that break point onto the current line is
+            // below the column limit.
+            BreakableToken::Split Split =
+                Token->getSplit(NextLineIndex, TailOffset, ColumnLimit,
+                                ContentStartColumn, CommentPragmasRegex);
+            if (Split.first == StringRef::npos) {
+              DEBUG(llvm::dbgs() << "    Did not find later break\n");
+              Reflow = false;
+            } else {
+              // Check whether the first split point gets us below the column
+              // limit. Note that we will execute this split below as part of
+              // the normal token breaking and reflow logic within the line.
+              unsigned ToSplitColumns = Token->getRangeLength(
+                  NextLineIndex, TailOffset, Split.first, ContentStartColumn);
+              if (ContentStartColumn + ToSplitColumns > ColumnLimit) {
+                DEBUG(llvm::dbgs() << "    Next split protrudes, need: "
+                                   << (ContentStartColumn + ToSplitColumns)
+                                   << ", space: " << ColumnLimit);
+                unsigned ExcessCharactersPenalty =
+                    (ContentStartColumn + ToSplitColumns - ColumnLimit) *
+                    Style.PenaltyExcessCharacter;
+                if (NewBreakPenalty < ExcessCharactersPenalty) {
+                  Reflow = false;
+                }
+              }
+            }
+          }
+        } else {
+          DEBUG(llvm::dbgs() << "not found.\n");
+        }
+      }
+      if (!Reflow) {
+        // If we didn't reflow into the next line, the only space to consider is
+        // the next logical line. Reset our state to match the start of the next
+        // line.
+        TailOffset = 0;
+        ContentStartColumn =
+            Token->getContentStartColumn(NextLineIndex, /*Break=*/false);
+        RemainingTokenColumns = Token->getRemainingLength(
+            NextLineIndex, TailOffset, ContentStartColumn);
+        // Adapt the start of the token, for example indent.
+        if (!DryRun)
+          Token->adaptStartOfLine(NextLineIndex, Whitespaces);
+      } else {
+        // If we found a reflow split and have added a new break before the next
+        // line, we are going to remove the line break at the start of the next
+        // logical line. For example, here we'll add a new line break after
+        // 'text', and subsequently delete the line break between 'that' and
+        // 'reflows'.
+        //   // some text that
+        //   // reflows
+        // ->
+        //   // some text
+        //   // that reflows
+        // When adding the line break, we also added the penalty for it, so we
+        // need to subtract that penalty again when we remove the line break due
+        // to reflowing.
+        if (NewBreakBefore) {
+          assert(Penalty >= NewBreakPenalty);
+          Penalty -= NewBreakPenalty;
+        }
+        if (!DryRun)
+          Token->reflow(NextLineIndex, Whitespaces);
+      }
+    }
   }
 
   BreakableToken::Split SplitAfterLastLine =
-      Token->getSplitAfterLastLine(TailOffset, ColumnLimit);
+      Token->getSplitAfterLastLine(TailOffset);
   if (SplitAfterLastLine.first != StringRef::npos) {
     DEBUG(llvm::dbgs() << "Replacing whitespace after last line.\n");
     if (!DryRun)
       Token->replaceWhitespaceAfterLastLine(TailOffset, SplitAfterLastLine,
                                             Whitespaces);
-    RemainingTokenColumns = Token->getLineLengthAfterSplitAfterLastLine(
-        TailOffset, SplitAfterLastLine);
+    ContentStartColumn =
+        Token->getContentStartColumn(Token->getLineCount() - 1, /*Break=*/true);
+    RemainingTokenColumns = Token->getRemainingLength(
+        Token->getLineCount() - 1,
+        TailOffset + SplitAfterLastLine.first + SplitAfterLastLine.second,
+        ContentStartColumn);
   }
 
-  State.Column = RemainingTokenColumns;
+  State.Column = ContentStartColumn + RemainingTokenColumns -
+                 Current.UnbreakableTailLength;
 
   if (BreakInserted) {
     // If we break the token inside a parameter list, we need to break before
     // the next parameter on all levels, so that the next parameter is clearly
     // visible. Line comments already introduce a break.
     if (Current.isNot(TT_LineComment)) {
       for (unsigned i = 0, e = State.Stack.size(); i != e; ++i)
         State.Stack[i].BreakBeforeParameter = true;

cfe/trunk/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, BreaksStringLiterals) {
   AlignLeft.AlignEscapedNewlines = FormatStyle::ENAS_Left;
   EXPECT_EQ("#define A \\\n"
             "  \"some \" \\\n"
             "  \"text \" \\\n"
             "  \"other\";",
             format("#define A \"some text other\";", AlignLeft));
 }
 
+TEST_F(FormatTest, BreaksStringLiteralsAtColumnLimit) {
+  EXPECT_EQ("C a = \"some more \"\n"
+            "      \"text\";",
+            format("C a = \"some more text\";", getLLVMStyleWithColumns(18)));
+}
+
 TEST_F(FormatTest, FullyRemoveEmptyLines) {
   FormatStyle NoEmptyLines = getLLVMStyleWithColumns(80);
   NoEmptyLines.MaxEmptyLinesToKeep = 0;
   EXPECT_EQ("int i = a(b());",
             format("int i=a(\n\n b(\n\n\n )\n\n);", NoEmptyLines));
 }
 
 TEST_F(FormatTest, BreaksStringLiteralsWithTabs) {
EXPECT_EQ("int a; // first line\n"
             "       // line",
             format("int a; // first line\n"
                    "       // second line\n"
                    "       // third line",
                    Style));
 
   Style.PenaltyExcessCharacter = 90;
   verifyFormat("int a; // the comment", Style);
-  EXPECT_EQ("int a; // the\n"
-            "       // comment aa",
+  EXPECT_EQ("int a; // the comment\n"
+            "       // aa",
             format("int a; // the comment aa", Style));
-  EXPECT_EQ("int a; // first line\n"
-            "       // second line\n"
-            "       // third line",
-            format("int a; // first line\n"
-                   "       // second line\n"
-                   "       // third line",
-                   Style));
   EXPECT_EQ("int a; /* first line\n"
             "        * second line\n"
             "        * third line\n"
             "        */",
             format("int a; /* first line\n"
                    "        * second line\n"
                    "        * third line\n"
                    "        */",
                    Style));
+  EXPECT_EQ("int a; // first line\n"
+            "       // second line\n"
+            "       // third line",
+            format("int a; // first line\n"
+                   "       // second line\n"
+                   "       // third line",
+                   Style));
   // FIXME: Investigate why this is not getting the same layout as the test
   // above.
   EXPECT_EQ("int a; /* first line\n"
-            "        * second\n"
-            "        * line third\n"
-            "        * line\n"
+            "        * second line\n"
+            "        * third line\n"
             "        */",
             format("int a; /* first line second line third line"
                    "\n*/",
                    Style));
 
   EXPECT_EQ("// foo bar baz bazfoo\n"
             "// foo bar\n",
             format("// foo bar baz bazfoo\n"
                    "// foo            bar\n",
                    Style));
   EXPECT_EQ("// foo bar baz bazfoo\n"
             "// foo bar\n",
             format("// foo bar baz      bazfoo\n"
                    "// foo            bar\n",
                    Style));
 
   // FIXME: Optimally, we'd keep bazfoo on the first line and reflow bar to the
   // next one.
-  EXPECT_EQ("// foo bar baz\n"
-            "// bazfoo bar foo\n"
-            "// bar\n",
+  EXPECT_EQ("// foo bar baz bazfoo\n"
+            "// bar foo bar\n",
             format("// foo bar baz      bazfoo bar\n"
                    "// foo            bar\n",
                    Style));
 
   EXPECT_EQ("// foo bar baz bazfoo\n"
-            "// foo bar baz\n"
-            "// bazfoo bar foo\n"
-            "// bar\n",
+            "// foo bar baz bazfoo\n"
+            "// bar foo bar\n",
             format("// foo bar baz      bazfoo\n"
                    "// foo bar baz      bazfoo bar\n"
                    "// foo bar\n",
                    Style));
 
-  // FIXME: Optimally, we'd keep 'bar' in the last line at the end of the line,
-  // as it does not actually protrude far enough to make breaking pay off.
-  // Unfortunately, due to how reflowing is currently implemented, we already
-  // check the column limit after the reflowing decision and extend the reflow
-  // range, so we will not take whitespace compression into account.
   EXPECT_EQ("// foo bar baz bazfoo\n"
-            "// foo bar baz\n"
-            "// bazfoo bar foo\n"
-            "// bar\n",
+            "// foo bar baz bazfoo\n"
+            "// bar foo bar\n",
             format("// foo bar baz      bazfoo\n"
                    "// foo bar baz      bazfoo bar\n"
                    "// foo           bar\n",
                    Style));
 }
 
 #define EXPECT_ALL_STYLES_EQUAL(Styles)                                        \
   for (size_t i = 1; i < Styles.size(); ++i)                                   \
EXPECT_EQ("\"Однажды, в \"\n"
             format("\"Однажды, в студёную зимнюю пору,\"",
                    getLLVMStyleWithColumns(13)));
   EXPECT_EQ(
       "\"一 二 三 \"\n"
       "\"四 五六 \"\n"
       "\"七 八 九 \"\n"
       "\"十\"",
       format("\"一 二 三 四 五六 七 八 九 十\"", getLLVMStyleWithColumns(11)));
-  EXPECT_EQ("\"一\t二 \"\n"
-            "\"\t三 \"\n"
-            "\"四 五\t六 \"\n"
-            "\"\t七 \"\n"
+  EXPECT_EQ("\"一\t\"\n"
+            "\"二 \t\"\n"
+            "\"三 四 \"\n"
+            "\"五\t\"\n"
+            "\"六 \t\"\n"
+            "\"七 \"\n"
             "\"八九十\tqq\"",
             format("\"一\t二 \t三 四 五\t六 \t七 八九十\tqq\"",
                    getLLVMStyleWithColumns(11)));
 
   // UTF8 character in an escape sequence.
   EXPECT_EQ("\"aaaaaa\"\n"
             "\"\\\xC2\x8D\"",
             format("\"aaaaaa\\\xC2\x8D\"", getLLVMStyleWithColumns(10)));

cfe/trunk/unittests/Format/FormatTestComments.cpp

EXPECT_EQ("int f(int i) {\n"
                    " #ifdef A\n"
                    "    j++;\n"
                    "#endif\n"
                    "  }\n"
                    "}"));
 }
 
 TEST_F(FormatTestComments, SplitsLongLinesInComments) {
+  // FIXME: Do we need to fix up the "  */" at the end?
+  // It doesn't look like any of our current logic triggers this.
   EXPECT_EQ("/* This is a long\n"
             " * comment that\n"
-            " * doesn't\n"
-            " * fit on one line.\n"
-            " */",
+            " * doesn't fit on\n"
+            " * one line.  */",
             format("/* "
                    "This is a long                                         "
                    "comment that "
                    "doesn't                                    "
                    "fit on one line.  */",
                    getLLVMStyleWithColumns(20)));
   EXPECT_EQ(
       "/* a b c d\n"
TEST_F(FormatTestComments, ReflowsComments) {
   // Keep empty comment lines.
   EXPECT_EQ("/**/", format(" /**/", getLLVMStyleWithColumns(20)));
   EXPECT_EQ("/* */", format(" /* */", getLLVMStyleWithColumns(20)));
   EXPECT_EQ("/*  */", format(" /*  */", getLLVMStyleWithColumns(20)));
   EXPECT_EQ("//", format(" //  ", getLLVMStyleWithColumns(20)));
   EXPECT_EQ("///", format(" ///  ", getLLVMStyleWithColumns(20)));
 }
 
+TEST_F(FormatTestComments, ReflowsCommentsPrecise) {
+  // FIXME: This assumes we do not continue compressing whitespace once we are
+  // in reflow mode. Consider compressing whitespace.
+
+  // Test that we stop reflowing precisely at the column limit.
+  // After reflowing, "// reflows into   foo" does not fit the column limit,
+  // so we compress the whitespace.
+  EXPECT_EQ("// some text that\n"
+            "// reflows into foo\n",
+            format("// some text that reflows\n"
+                   "// into   foo\n",
+                   getLLVMStyleWithColumns(20)));
+  // Given one more column, "// reflows into   foo" does fit the limit, so we
+  // do not compress the whitespace.
+  EXPECT_EQ("// some text that\n"
+            "// reflows into   foo\n",
+            format("// some text that reflows\n"
+                   "// into   foo\n",
+                   getLLVMStyleWithColumns(21)));
+
+  // Make sure that we correctly account for the space added in the reflow case
+  // when making the reflowing decision.
+  // First, when the next line ends precisely one column over the limit, do not
+  // reflow.
+  EXPECT_EQ("// some text that\n"
+            "// reflows\n"
+            "// into1234567\n",
+            format("// some text that reflows\n"
+                   "// into1234567\n",
+                   getLLVMStyleWithColumns(21)));
+  // Secondly, when the next line ends later, but the first word in that line
+  // is precisely one column over the limit, do not reflow.
+  EXPECT_EQ("// some text that\n"
+            "// reflows\n"
+            "// into1234567 f\n",
+            format("// some text that reflows\n"
+                   "// into1234567 f\n",
+                   getLLVMStyleWithColumns(21)));
+}
+
+TEST_F(FormatTestComments, ReflowsCommentsWithExtraWhitespace) {
+  // Baseline.
+  EXPECT_EQ("// some text\n"
+            "// that re flows\n",
+            format("// some text that\n"
+                   "// re flows\n",
+                   getLLVMStyleWithColumns(16)));
+  EXPECT_EQ("// some text\n"
+            "// that re flows\n",
+            format("// some text that\n"
+                   "// re    flows\n",
+                   getLLVMStyleWithColumns(16)));
+  EXPECT_EQ("/* some text\n"
+            " * that re flows\n"
+            " */\n",
+            format("/* some text that\n"
+                   "*      re       flows\n"
+                   "*/\n",
+                   getLLVMStyleWithColumns(16)));
+  // FIXME: We do not reflow if the indent of two subsequent lines differs;
+  // given that this is different behavior from block comments, do we want
+  // to keep this?
+  EXPECT_EQ("// some text\n"
+            "// that\n"
+            "//     re flows\n",
+            format("// some text that\n"
+                   "//     re       flows\n",
+                   getLLVMStyleWithColumns(16)));
+  // Space within parts of a line that fit.
+  // FIXME: Use the earliest possible split while reflowing to compress the
+  // whitespace within the line.
+  EXPECT_EQ("// some text that\n"
+            "// does re   flow\n"
+            "// more  here\n",
+            format("// some text that does\n"
+                   "// re   flow  more  here\n",
+                   getLLVMStyleWithColumns(21)));
+}
+
 TEST_F(FormatTestComments, IgnoresIf0Contents) {
   EXPECT_EQ("#if 0\n"
             "}{)(&*(^%%#%@! fsadj f;ldjs ,:;| <<<>>>][)(][\n"
             "#endif\n"
             "void f() {}",
             format("#if 0\n"
                    "}{)(&*(^%%#%@! fsadj f;ldjs ,:;| <<<>>>][)(][\n"
                    "#endif\n"
EXPECT_EQ("a = f(/* long\n"
                    getLLVMStyleWithColumns(16)));
 
   EXPECT_EQ(
       "a = f(a,\n"
       "      /* long\n"
       "         long */\n"
       "      b);",
       format("a = f(a, /* long long */ b);", getLLVMStyleWithColumns(16)));
+
   EXPECT_EQ(
       "a = f(\n"
       "    a,\n"
       "    /* long\n"
       "       long */\n"
       "    b);",
       format("a = f(a, /* long long */ b);", getLLVMStyleWithColumns(15)));
 
EXPECT_EQ("/*\n"
             format("/*\n"
                    " * long long long long\n"
                    " * long\n"
                    "  *\n"
                    "* long */",
                    getLLVMStyleWithColumns(20)));
 }
 
-TEST_F(FormatTestComments, NoCrush_Bug34236) {
+TEST_F(FormatTestComments, NoCrash_Bug34236) {
   // This is a test case from a crasher reported in:
   // https://bugs.llvm.org/show_bug.cgi?id=34236
   // Temporarily disable formatting for readability.
   // clang-format off
   EXPECT_EQ(
 "/*                                                                */ /*\n"
 "                                                                      *       a\n"
-"                                                                      * b c\n"
-"                                                                      * d*/",
+"                                                                      * b c d*/",
       format(
 "/*                                                                */ /*\n"
 " *       a b\n"
 " *       c     d*/",
           getLLVMStyleWithColumns(80)));
   // clang-format on
 }