Index: lib/Format/BreakableToken.h =================================================================== --- lib/Format/BreakableToken.h +++ lib/Format/BreakableToken.h @@ -212,7 +212,7 @@ // StartOfLineColumn[i] is the target column at which Line[i] should be. // Note that this excludes a leading "* " or "*" in case all lines have // a "*" prefix. - SmallVector StartOfLineColumn; + SmallVector StartOfLineColumn; // The column at which the text of a broken line should start. // Note that an optional decoration would go before that column. Index: lib/Format/BreakableToken.cpp =================================================================== --- lib/Format/BreakableToken.cpp +++ lib/Format/BreakableToken.cpp @@ -350,10 +350,9 @@ Lines[LineIndex].begin() - Lines[LineIndex - 1].end(); // Adjust the start column uniformly across all lines. - StartOfLineColumn[LineIndex] = std::max( - 0, + StartOfLineColumn[LineIndex] = encoding::columnWidthWithTabs(Whitespace, 0, Style.TabWidth, Encoding) + - IndentDelta); + IndentDelta; } unsigned BreakableBlockComment::getLineCount() const { return Lines.size(); } @@ -437,7 +436,6 @@ unsigned WhitespaceOffsetInToken = Lines[LineIndex].data() - Tok.TokenText.data() - LeadingWhitespace[LineIndex]; - assert(StartOfLineColumn[LineIndex] >= Prefix.size()); Whitespaces.replaceWhitespaceInToken( Tok, WhitespaceOffsetInToken, LeadingWhitespace[LineIndex], "", Prefix, InPPDirective, 1, IndentLevel, @@ -450,7 +448,7 @@ // If we break, we always break at the predefined indent. if (TailOffset != 0) return IndentAtLineBreak; - return StartOfLineColumn[LineIndex]; + return std::max(0, StartOfLineColumn[LineIndex]); } } // namespace format Index: lib/Format/WhitespaceManager.h =================================================================== --- lib/Format/WhitespaceManager.h +++ lib/Format/WhitespaceManager.h @@ -63,6 +63,12 @@ /// (in this order) at \p Offset inside \p Tok, replacing \p ReplaceChars /// characters. /// + /// Note: \p Spaces can be negative to retain information about initial + /// relative column offset between a line of a block comment and the start of + /// the comment. This negative offset may be compensated by trailing comment + /// alignment here. In all other cases negative \p Spaces will be truncated to + /// 0. + /// /// When \p InPPDirective is true, escaped newlines are inserted. \p Spaces is /// used to align backslashes correctly. void replaceWhitespaceInToken(const FormatToken &Tok, unsigned Offset, @@ -70,7 +76,7 @@ StringRef PreviousPostfix, StringRef CurrentPrefix, bool InPPDirective, unsigned Newlines, unsigned IndentLevel, - unsigned Spaces); + int Spaces); /// \brief Returns all the \c Replacements created during formatting. const tooling::Replacements &generateReplacements(); @@ -101,7 +107,7 @@ /// \p StartOfTokenColumn and \p InPPDirective will be used to lay out /// trailing comments and escaped newlines. Change(bool CreateReplacement, const SourceRange &OriginalWhitespaceRange, - unsigned IndentLevel, unsigned Spaces, unsigned StartOfTokenColumn, + unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn, unsigned NewlinesBefore, StringRef PreviousLinePostfix, StringRef CurrentLinePrefix, tok::TokenKind Kind, bool ContinuesPPDirective); @@ -128,7 +134,7 @@ // The number of spaces in front of the token or broken part of the token. // This will be adapted when aligning tokens. - unsigned Spaces; + int Spaces; // \c IsTrailingComment, \c TokenLength, \c PreviousEndOfTokenColumn and // \c EscapedNewlineColumn will be calculated in @@ -137,6 +143,9 @@ unsigned TokenLength; unsigned PreviousEndOfTokenColumn; unsigned EscapedNewlineColumn; + + const Change *StartOfBlockComment; + int IndentationOffset; }; /// \brief Calculate \c IsTrailingComment, \c TokenLength for the last tokens Index: lib/Format/WhitespaceManager.cpp =================================================================== --- lib/Format/WhitespaceManager.cpp +++ lib/Format/WhitespaceManager.cpp @@ -28,7 +28,7 @@ WhitespaceManager::Change::Change( bool CreateReplacement, const SourceRange &OriginalWhitespaceRange, - unsigned IndentLevel, unsigned Spaces, unsigned StartOfTokenColumn, + unsigned IndentLevel, int Spaces, unsigned StartOfTokenColumn, unsigned NewlinesBefore, StringRef PreviousLinePostfix, StringRef CurrentLinePrefix, tok::TokenKind Kind, bool ContinuesPPDirective) : CreateReplacement(CreateReplacement), @@ -69,14 +69,14 @@ void WhitespaceManager::replaceWhitespaceInToken( const FormatToken &Tok, unsigned Offset, unsigned ReplaceChars, StringRef PreviousPostfix, StringRef CurrentPrefix, bool InPPDirective, - unsigned Newlines, unsigned IndentLevel, unsigned Spaces) { + unsigned Newlines, unsigned IndentLevel, int Spaces) { if (Tok.Finalized) return; + SourceLocation Start = Tok.getStartOfNonWhitespace().getLocWithOffset(Offset); Changes.push_back(Change( - true, SourceRange(Tok.getStartOfNonWhitespace().getLocWithOffset(Offset), - Tok.getStartOfNonWhitespace().getLocWithOffset( - Offset + ReplaceChars)), - IndentLevel, Spaces, Spaces, Newlines, PreviousPostfix, CurrentPrefix, + true, SourceRange(Start, Start.getLocWithOffset(ReplaceChars)), + IndentLevel, Spaces, std::max(0, Spaces), Newlines, PreviousPostfix, + CurrentPrefix, // If we don't add a newline this change doesn't start a comment. Thus, // when we align line comments, we don't need to treat this change as one. // FIXME: We still need to take this change in account to properly @@ -122,6 +122,23 @@ // cases, setting TokenLength of the last token to 0 is wrong. Changes.back().TokenLength = 0; Changes.back().IsTrailingComment = Changes.back().Kind == tok::comment; + + const WhitespaceManager::Change *LastBlockComment = nullptr; + for (auto &Change : Changes) { + Change.StartOfBlockComment = nullptr; + if (Change.Kind == tok::comment) { + LastBlockComment = &Change; + } else if (Change.Kind == tok::unknown) { + Change.StartOfBlockComment = LastBlockComment; + } else { + LastBlockComment = nullptr; + } + Change.IndentationOffset = + Change.StartOfBlockComment + ? Change.StartOfTokenColumn - + Change.StartOfBlockComment->StartOfTokenColumn + : 0; + } } void WhitespaceManager::alignTrailingComments() { @@ -131,58 +148,61 @@ bool BreakBeforeNext = false; unsigned Newlines = 0; for (unsigned i = 0, e = Changes.size(); i != e; ++i) { + if (Changes[i].StartOfBlockComment) + continue; + Newlines += Changes[i].NewlinesBefore; + if (!Changes[i].IsTrailingComment) + continue; + unsigned ChangeMinColumn = Changes[i].StartOfTokenColumn; // FIXME: Correctly handle ChangeMaxColumn in PP directives. unsigned ChangeMaxColumn = Style.ColumnLimit - Changes[i].TokenLength; - Newlines += Changes[i].NewlinesBefore; - if (Changes[i].IsTrailingComment) { - // If this comment follows an } in column 0, it probably documents the - // closing of a namespace and we don't want to align it. - bool FollowsRBraceInColumn0 = i > 0 && Changes[i].NewlinesBefore == 0 && - Changes[i - 1].Kind == tok::r_brace && - Changes[i - 1].StartOfTokenColumn == 0; - bool WasAlignedWithStartOfNextLine = false; - if (Changes[i].NewlinesBefore == 1) { // A comment on its own line. - for (unsigned j = i + 1; j != e; ++j) { - if (Changes[j].Kind != tok::comment) { // Skip over comments. - // The start of the next token was previously aligned with the - // start of this comment. - WasAlignedWithStartOfNextLine = - (SourceMgr.getSpellingColumnNumber( - Changes[i].OriginalWhitespaceRange.getEnd()) == - SourceMgr.getSpellingColumnNumber( - Changes[j].OriginalWhitespaceRange.getEnd())); - break; - } + // If this comment follows an } in column 0, it probably documents the + // closing of a namespace and we don't want to align it. + bool FollowsRBraceInColumn0 = i > 0 && Changes[i].NewlinesBefore == 0 && + Changes[i - 1].Kind == tok::r_brace && + Changes[i - 1].StartOfTokenColumn == 0; + bool WasAlignedWithStartOfNextLine = false; + if (Changes[i].NewlinesBefore == 1) { // A comment on its own line. + for (unsigned j = i + 1; j != e; ++j) { + if (Changes[j].Kind != tok::comment) { // Skip over comments. + // The start of the next token was previously aligned with the + // start of this comment. + WasAlignedWithStartOfNextLine = + (SourceMgr.getSpellingColumnNumber( + Changes[i].OriginalWhitespaceRange.getEnd()) == + SourceMgr.getSpellingColumnNumber( + Changes[j].OriginalWhitespaceRange.getEnd())); + break; } } - if (!Style.AlignTrailingComments || FollowsRBraceInColumn0) { - alignTrailingComments(StartOfSequence, i, MinColumn); - MinColumn = ChangeMinColumn; - MaxColumn = ChangeMinColumn; - StartOfSequence = i; - } else if (BreakBeforeNext || Newlines > 1 || - (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) || - // Break the comment sequence if the previous line did not end - // in a trailing comment. - (Changes[i].NewlinesBefore == 1 && i > 0 && - !Changes[i - 1].IsTrailingComment) || - WasAlignedWithStartOfNextLine) { - alignTrailingComments(StartOfSequence, i, MinColumn); - MinColumn = ChangeMinColumn; - MaxColumn = ChangeMaxColumn; - StartOfSequence = i; - } else { - MinColumn = std::max(MinColumn, ChangeMinColumn); - MaxColumn = std::min(MaxColumn, ChangeMaxColumn); - } - BreakBeforeNext = - (i == 0) || (Changes[i].NewlinesBefore > 1) || - // Never start a sequence with a comment at the beginning of - // the line. - (Changes[i].NewlinesBefore == 1 && StartOfSequence == i); - Newlines = 0; } + if (!Style.AlignTrailingComments || FollowsRBraceInColumn0) { + alignTrailingComments(StartOfSequence, i, MinColumn); + MinColumn = ChangeMinColumn; + MaxColumn = ChangeMinColumn; + StartOfSequence = i; + } else if (BreakBeforeNext || Newlines > 1 || + (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) || + // Break the comment sequence if the previous line did not end + // in a trailing comment. + (Changes[i].NewlinesBefore == 1 && i > 0 && + !Changes[i - 1].IsTrailingComment) || + WasAlignedWithStartOfNextLine) { + alignTrailingComments(StartOfSequence, i, MinColumn); + MinColumn = ChangeMinColumn; + MaxColumn = ChangeMaxColumn; + StartOfSequence = i; + } else { + MinColumn = std::max(MinColumn, ChangeMinColumn); + MaxColumn = std::min(MaxColumn, ChangeMaxColumn); + } + BreakBeforeNext = + (i == 0) || (Changes[i].NewlinesBefore > 1) || + // Never start a sequence with a comment at the beginning of + // the line. + (Changes[i].NewlinesBefore == 1 && StartOfSequence == i); + Newlines = 0; } alignTrailingComments(StartOfSequence, Changes.size(), MinColumn); } @@ -190,15 +210,20 @@ void WhitespaceManager::alignTrailingComments(unsigned Start, unsigned End, unsigned Column) { for (unsigned i = Start; i != End; ++i) { + int Shift = 0; if (Changes[i].IsTrailingComment) { - assert(Column >= Changes[i].StartOfTokenColumn); - Changes[i].Spaces += Column - Changes[i].StartOfTokenColumn; - if (i + 1 != End) { - Changes[i + 1].PreviousEndOfTokenColumn += - Column - Changes[i].StartOfTokenColumn; - } - Changes[i].StartOfTokenColumn = Column; + Shift = Column - Changes[i].StartOfTokenColumn; + } + if (Changes[i].StartOfBlockComment) { + Shift = Changes[i].IndentationOffset + + Changes[i].StartOfBlockComment->StartOfTokenColumn - + Changes[i].StartOfTokenColumn; } + assert(Shift >= 0); + Changes[i].Spaces += Shift; + if (i + 1 != End) + Changes[i + 1].PreviousEndOfTokenColumn += Shift; + Changes[i].StartOfTokenColumn += Shift; } } @@ -245,8 +270,8 @@ C.PreviousEndOfTokenColumn, C.EscapedNewlineColumn); else appendNewlineText(ReplacementText, C.NewlinesBefore); - appendIndentText(ReplacementText, C.IndentLevel, C.Spaces, - C.StartOfTokenColumn - C.Spaces); + appendIndentText(ReplacementText, C.IndentLevel, std::max(0, C.Spaces), + C.StartOfTokenColumn - std::max(0, C.Spaces)); ReplacementText.append(C.CurrentLinePrefix); storeReplacement(C.OriginalWhitespaceRange, ReplacementText); } Index: unittests/Format/FormatTest.cpp =================================================================== --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -1013,6 +1013,30 @@ format("int i; /* Comment with empty...\n" " *\n" " * line. */")); + EXPECT_EQ("int foobar = 0; /* comment */\n" + "int bar = 0; /* multiline\n" + " comment 1 */\n" + "int baz = 0; /* multiline\n" + " comment 2 */\n" + "int bzz = 0; /* multiline\n" + " comment 3 */", + format("int foobar = 0; /* comment */\n" + "int bar = 0; /* multiline\n" + " comment 1 */\n" + "int baz = 0; /* multiline\n" + " comment 2 */\n" + "int bzz = 0; /* multiline\n" + " comment 3 */")); + EXPECT_EQ("int foobar = 0; /* comment */\n" + "int bar = 0; /* multiline\n" + " comment */\n" + "int baz = 0; /* multiline\n" + "comment */", + format("int foobar = 0; /* comment */\n" + "int bar = 0; /* multiline\n" + "comment */\n" + "int baz = 0; /* multiline\n" + "comment */")); } TEST_F(FormatTest, CorrectlyHandlesLengthOfBlockComments) {