Index: lib/Format/BreakableToken.h =================================================================== --- lib/Format/BreakableToken.h +++ lib/Format/BreakableToken.h @@ -138,7 +138,8 @@ /// after formatting. BreakableLineComment(const FormatToken &Token, unsigned IndentLevel, unsigned StartColumn, bool InPPDirective, - encoding::Encoding Encoding, const FormatStyle &Style); + encoding::Encoding Encoding, const FormatStyle &Style, + unsigned PreviousLineCommentOverflow); Split getSplit(unsigned LineIndex, unsigned TailOffset, unsigned ColumnLimit) const override; @@ -148,10 +149,15 @@ WhitespaceManager &Whitespaces) override; void replaceWhitespaceBefore(unsigned LineIndex, WhitespaceManager &Whitespaces) override; - + unsigned getLineLengthAfterSplit(unsigned LineIndex, unsigned TailOffset, + StringRef::size_type Length) const override; + void resetPreviousLineCommentOverflow() { PreviousLineCommentOverflow = 0; } private: // The prefix without an additional space if one was added. StringRef OriginalPrefix; + // When reflowing line comments we need to use the overflow from the + // previous line comment. + unsigned PreviousLineCommentOverflow; }; class BreakableBlockComment : public BreakableToken { Index: lib/Format/BreakableToken.cpp =================================================================== --- lib/Format/BreakableToken.cpp +++ lib/Format/BreakableToken.cpp @@ -199,10 +199,12 @@ BreakableLineComment::BreakableLineComment( const FormatToken &Token, unsigned IndentLevel, unsigned StartColumn, - bool InPPDirective, encoding::Encoding Encoding, const FormatStyle &Style) + bool InPPDirective, encoding::Encoding Encoding, const FormatStyle &Style, + unsigned PreviousLineCommentOverflow) : BreakableSingleLineToken(Token, IndentLevel, StartColumn, getLineCommentIndentPrefix(Token.TokenText), "", - InPPDirective, Encoding, Style) { + InPPDirective, Encoding, Style), + PreviousLineCommentOverflow(PreviousLineCommentOverflow) { OriginalPrefix = Prefix; if (Token.TokenText.size() > Prefix.size() && isAlphanumeric(Token.TokenText[Prefix.size()])) { @@ -213,11 +215,44 @@ } } +unsigned BreakableLineComment::getLineLengthAfterSplit( + unsigned LineIndex, unsigned Offset, StringRef::size_type Length) const { + if (PreviousLineCommentOverflow) { + // If we have a previous line comment overflow - add one char for separating + // whitespace. Since we are merging the current line comment with the + // previous (and the previous tokens already include the prefix and postfix + // size, we can ignore those here). + return PreviousLineCommentOverflow + 1 + + encoding::columnWidthWithTabs(Line.substr(Offset, Length), + /*StartColumn + Prefix.size()*/ 0, + Style.TabWidth, Encoding); + } else + return BreakableSingleLineToken::getLineLengthAfterSplit(LineIndex, Offset, Length); +} + + BreakableToken::Split BreakableLineComment::getSplit(unsigned LineIndex, unsigned TailOffset, unsigned ColumnLimit) const { - return getCommentSplit(Line.substr(TailOffset), StartColumn + Prefix.size(), - ColumnLimit, Style.TabWidth, Encoding); + if (PreviousLineCommentOverflow) { + assert(TailOffset == 0 && "When dealing with previous overflow, we should " + "not be starting at an offset into the current " + "text!"); + if (ColumnLimit == PreviousLineCommentOverflow) + return BreakableToken::Split(0, 0); + + auto split = getCommentSplit(Line, 0/*StartColumn + Prefix.size()*/, + ColumnLimit - PreviousLineCommentOverflow, + Style.TabWidth, Encoding); + + // If we are exceeding the limit, start on a new line. + if (((split.first == StringRef::npos ? Line.size() : split.first) + + PreviousLineCommentOverflow) >= ColumnLimit) + return BreakableToken::Split(0, 0); + return split; + } else + return getCommentSplit(Line.substr(TailOffset), StartColumn + Prefix.size(), + ColumnLimit, Style.TabWidth, Encoding); } void BreakableLineComment::insertBreak(unsigned LineIndex, unsigned TailOffset, @@ -227,7 +262,6 @@ Tok, OriginalPrefix.size() + TailOffset + Split.first, Split.second, Postfix, Prefix, InPPDirective, /*Newlines=*/1, IndentLevel, StartColumn); } - void BreakableLineComment::replaceWhitespace(unsigned LineIndex, unsigned TailOffset, Split Split, WhitespaceManager &Whitespaces) { @@ -240,11 +274,21 @@ void BreakableLineComment::replaceWhitespaceBefore(unsigned LineIndex, WhitespaceManager &Whitespaces) { - if (OriginalPrefix != Prefix) { - Whitespaces.replaceWhitespaceInToken(Tok, OriginalPrefix.size(), 0, "", "", - /*InPPDirective=*/false, - /*Newlines=*/0, /*IndentLevel=*/0, - /*Spaces=*/1); + if (PreviousLineCommentOverflow) { + // Note: The difference between OriginalPrefix and Prefix is no more than a + // space. + Whitespaces.replaceBeginningWhitespaceForReflowingLineComment( + Tok, /*Spaces*/ OriginalPrefix != Prefix ? 1 : 0, StartColumn, + OriginalPrefix == Prefix ? OriginalPrefix.size() - 1 + : OriginalPrefix.size()); + } else { + if (OriginalPrefix != Prefix) { + Whitespaces.replaceWhitespaceInToken(Tok, OriginalPrefix.size(), 0, "", + "", + /*InPPDirective=*/false, + /*Newlines=*/0, /*IndentLevel=*/0, + /*Spaces=*/1); + } } } @@ -451,3 +495,5 @@ } // namespace format } // namespace clang + + Index: lib/Format/ContinuationIndenter.h =================================================================== --- lib/Format/ContinuationIndenter.h +++ lib/Format/ContinuationIndenter.h @@ -303,6 +303,9 @@ /// \brief The number of used columns in the current line. unsigned Column; + /// \brief The start column to be used by all line-comments that are + /// undergoing reflow. + unsigned StartColumnOfReflowingLineComment; /// \brief The token that needs to be next formatted. FormatToken *NextToken; Index: lib/Format/ContinuationIndenter.cpp =================================================================== --- lib/Format/ContinuationIndenter.cpp +++ lib/Format/ContinuationIndenter.cpp @@ -256,6 +256,8 @@ unsigned ExtraSpaces) { FormatToken &Current = *State.NextToken; const FormatToken &Previous = *State.NextToken->Previous; + const bool IsReflowingLineComment = + Current.Type == TT_LineComment && Previous.Type == TT_LineComment; if (Current.is(tok::equal) && (State.Line->First->is(tok::kw_for) || Current.NestingLevel == 0) && State.Stack.back().VariablePos == 0) { @@ -271,13 +273,20 @@ if (Previous.PartOfMultiVariableDeclStmt) State.Stack.back().LastSpace = State.Stack.back().VariablePos; } - - unsigned Spaces = Current.SpacesRequiredBefore + ExtraSpaces; - - if (!DryRun) + // FIXME: Is this really what we want? + unsigned Spaces = IsReflowingLineComment + ? ExtraSpaces + : Current.SpacesRequiredBefore + ExtraSpaces; + assert((!IsReflowingLineComment || Spaces == 0) && + "There should be no extra spaces added to the previous column when " + "reflowing comments!"); + // No need to replace white space when reflowing a line-comment, since that is + // deferred to when the merging is actually being performed and the decision + // to flow it on to the next line or keep it on the current is being made. + if (!DryRun && !IsReflowingLineComment) { Whitespaces.replaceWhitespace(Current, /*Newlines=*/0, /*IndentLevel=*/0, Spaces, State.Column + Spaces); - + } if (Current.Type == TT_SelectorName && !State.Stack.back().ObjCSelectorNameFound) { if (Current.LongestObjCSelectorName == 0) @@ -935,6 +944,11 @@ unsigned ContinuationIndenter::breakProtrudingToken(const FormatToken &Current, LineState &State, bool DryRun) { + const bool IsReflowingLineComments = [&] { + FormatToken *Previous = Current.Previous; + return Previous && Previous->Type == TT_LineComment && + Current.Type == TT_LineComment; + }(); // Don't break multi-line tokens other than block comments. Instead, just // update the state. if (Current.Type != TT_BlockComment && Current.IsMultiline) @@ -949,6 +963,10 @@ std::unique_ptr Token; unsigned StartColumn = State.Column - Current.ColumnWidth; + assert(!IsReflowingLineComments || StartColumn <= Style.ColumnLimit); + // Should we need it, cache the start column of the next reflow comment. + if (!IsReflowingLineComments && Current.Type == TT_LineComment) + State.StartColumnOfReflowingLineComment = StartColumn; unsigned ColumnLimit = getColumnLimit(State); if (Current.isStringLiteral()) { @@ -1001,9 +1019,12 @@ Current.Previous->Type != TT_ImplicitStringLiteral)) { if (CommentPragmasRegex.match(Current.TokenText.substr(2))) return 0; - Token.reset(new BreakableLineComment(Current, State.Line->Level, - StartColumn, /*InPPDirective=*/false, - Encoding, Style)); + Token.reset(new BreakableLineComment( + Current, State.Line->Level, + IsReflowingLineComments ? State.StartColumnOfReflowingLineComment + : StartColumn, + /*InPPDirective=*/false, Encoding, Style, + IsReflowingLineComments ? StartColumn : 0)); // We don't insert backslashes when breaking line comments. ColumnLimit = Style.ColumnLimit; } else { @@ -1011,15 +1032,42 @@ } if (Current.UnbreakableTailLength >= ColumnLimit) return 0; - - unsigned RemainingSpace = ColumnLimit - Current.UnbreakableTailLength; + // FIXME: Remaining Space might not be calculated correctly for reflowing line + // comments. + const unsigned RemainingSpace = ColumnLimit - Current.UnbreakableTailLength; bool BreakInserted = false; unsigned Penalty = 0; unsigned RemainingTokenColumns = 0; + BreakableLineComment *CurLineCommentBeingReflown = + IsReflowingLineComments + ? static_cast(Token.get()) + : nullptr; + for (unsigned LineIndex = 0, EndIndex = Token->getLineCount(); LineIndex != EndIndex; ++LineIndex) { - if (!DryRun) + if (IsReflowingLineComments) { + BreakableToken::Split Split = Token->getSplit(LineIndex, 0, ColumnLimit); + // Since we can not place any portion of the current line-comment's tokens + // into the previous comment's line, ignore the previous comment overflow + // and place the current comment on its own line, aligned with the + // previous coment. + if (Split.first == 0) { + CurLineCommentBeingReflown->resetPreviousLineCommentOverflow(); + if (!DryRun) { + // FIXME: Get rid of this ugly const_cast hack! + Whitespaces.replaceWhitespace( + const_cast(Current), /*NewLines=*/1, + State.Stack.back().IndentLevel, + State.StartColumnOfReflowingLineComment, + State.StartColumnOfReflowingLineComment, State.Line->InPPDirective); + } + } + } + if (!DryRun) { Token->replaceWhitespaceBefore(LineIndex, Whitespaces); + } + // The Offset from the beginning of the current token's line that has been + // processed. unsigned TailOffset = 0; RemainingTokenColumns = Token->getLineLengthAfterSplit(LineIndex, TailOffset, StringRef::npos); @@ -1034,6 +1082,14 @@ break; } assert(Split.first != 0); + // Determine the columns used on the currently "inserted" line. + const unsigned ColumnsUsed = + Token->getLineLengthAfterSplit(LineIndex, TailOffset, Split.first); + // Overflow has to be less than column limit - so ok to reset - since it + // must have all been used in calculating the current split, so should + // not be considered for remaining tokens. + if (CurLineCommentBeingReflown) + CurLineCommentBeingReflown->resetPreviousLineCommentOverflow(); unsigned NewRemainingTokenColumns = Token->getLineLengthAfterSplit( LineIndex, TailOffset + Split.first + Split.second, StringRef::npos); @@ -1052,17 +1108,17 @@ break; assert(NewRemainingTokenColumns < RemainingTokenColumns); - if (!DryRun) + if (!DryRun) { Token->insertBreak(LineIndex, TailOffset, Split, Whitespaces); + } Penalty += Current.SplitPenalty; - unsigned ColumnsUsed = - Token->getLineLengthAfterSplit(LineIndex, TailOffset, Split.first); if (ColumnsUsed > ColumnLimit) { Penalty += Style.PenaltyExcessCharacter * (ColumnsUsed - ColumnLimit); } TailOffset += Split.first + Split.second; RemainingTokenColumns = NewRemainingTokenColumns; BreakInserted = true; + } } @@ -1079,7 +1135,7 @@ Penalty += Current.isStringLiteral() ? Style.PenaltyBreakString : Style.PenaltyBreakComment; - + State.Stack.back().LastSpace = StartColumn; } return Penalty; Index: lib/Format/Format.cpp =================================================================== --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -274,6 +274,8 @@ namespace clang { namespace format { +bool isInEditorMode(); + const std::error_category &getParseCategory() { static ParseErrorCategory C; return C; @@ -562,6 +564,12 @@ ContinuationIndenter *Indenter; }; + +static unsigned getColumnWidthOfLine(const AnnotatedLine *Line) { + FormatToken *Last = Line->Last; + return Last->OriginalColumn + Last->ColumnWidth + 1; +} + class LineJoiner { public: LineJoiner(const FormatStyle &Style) : Style(Style) {} @@ -571,10 +579,73 @@ tryFitMultipleLinesInOne(unsigned Indent, SmallVectorImpl::const_iterator I, SmallVectorImpl::const_iterator E) { - // We can never merge stuff if there are trailing line comments. + const AnnotatedLine *TheLine = *I; - if (TheLine->Last->Type == TT_LineComment) - return 0; + // Determine whether we need to merge line comments for re-flow. + if (TheLine->Last->Type == TT_LineComment) { + auto HasOnlyWhitespaceTokens = [](StringRef Text) { + StringRef RTrimmedCommentText = Text.rtrim(); + for (char c : RTrimmedCommentText) + if (c != '/') + return false; + return true; + }; + FormatToken *CurCommentBeingMerged = TheLine->Last; + if (HasOnlyWhitespaceTokens(CurCommentBeingMerged->TokenText)) + return 0; + const bool IsInEditorMode = true; //clang::format::isInEditorMode(); + unsigned NumOfSequentialLineCommentsToJoin = 0; + + unsigned CurTotalWidthOfChars = 0; + if (!IsInEditorMode) { + // If we are not in Editor mode, then only reflow if the current + // line-comment exceeds its column length. + if ((CurTotalWidthOfChars = getColumnWidthOfLine(TheLine)) <= + Style.ColumnLimit + 1) + return 0; + } + + for (auto *Next = I + 1; Next != E; ++Next) { + const AnnotatedLine *const NextLine = *Next; + const FormatToken *const FirstTok = NextLine->First; + const FormatToken *const LastTok = NextLine->Last; + // Only consider reflowing/merging those comment-containing-lines that + // contain only a line-comment. + if (FirstTok->Type != TT_LineComment || + LastTok->Type != TT_LineComment || FirstTok != LastTok) + break; + // Only consider those comments sequenced directly on the next line. + if (FirstTok->NewlinesBefore > 1) + break; + // Do not merge a comment that contains more than 3 forward slashes in + // sequence + if (FirstTok->TokenText.startswith("////")) + break; + // Do not merge a comment that has no non-whitespace tokens + if (HasOnlyWhitespaceTokens(FirstTok->TokenText)) + break; + + if (IsInEditorMode) + ++NumOfSequentialLineCommentsToJoin; + else { + ++NumOfSequentialLineCommentsToJoin; + // FIXME: add one for space? + CurTotalWidthOfChars += (FirstTok->OriginalColumn + FirstTok->ColumnWidth + 1); + const unsigned NumDivisions = CurTotalWidthOfChars/Style.ColumnLimit; + + // Do not keep joining if when we add to the subsequent line, we have enough + // columns to spare. + // FIXME: This might need some tweaking to be more accurate. + if (NumDivisions == NumOfSequentialLineCommentsToJoin) + break; + // FIXME: Or if we are just one above and our remainder is 0. + // if (NumDivisions == NumOfSequentialLineCommentsToJoin + 1 && + // !(CurTotalWidthOfChars % Style.ColumnLimit)) + // break; + } + } + return NumOfSequentialLineCommentsToJoin; + } if (Style.ColumnLimit > 0 && Indent > Style.ColumnLimit) return 0; @@ -833,6 +904,7 @@ // Merge multiple lines if possible. unsigned MergedLines = Joiner.tryFitMultipleLinesInOne(Indent, I, E); + if (MergedLines > 0 && Style.ColumnLimit == 0) { // Disallow line merging if there is a break at the start of one of the // input lines. @@ -1103,11 +1175,21 @@ if (!Seen.insert(&Node->State).second) // State already examined with lower penalty. continue; - + const bool IsReflowingLineComments = [&] { + const FormatToken *CurrentTok = Node->State.NextToken; + const FormatToken *PrevTok = CurrentTok->Previous; + return PrevTok && CurrentTok && PrevTok->Type == TT_LineComment && + CurrentTok->Type == TT_LineComment; + }(); FormatDecision LastFormat = Node->State.NextToken->Decision; if (LastFormat == FD_Unformatted || LastFormat == FD_Continue) addNextStateToQueue(Penalty, Node, /*NewLine=*/false, &Count, &Queue); - if (LastFormat == FD_Unformatted || LastFormat == FD_Break) + // Don't attempt to analyze a state for adding new lines when reflowing a + // C++ comment. + // FIXME: We can probably make this work if necessary - but currently + // it allows for joining lines it shouldn't. + if ((LastFormat == FD_Unformatted || LastFormat == FD_Break) && + !IsReflowingLineComments) addNextStateToQueue(Penalty, Node, /*NewLine=*/true, &Count, &Queue); } Index: lib/Format/WhitespaceManager.h =================================================================== --- lib/Format/WhitespaceManager.h +++ lib/Format/WhitespaceManager.h @@ -51,6 +51,13 @@ unsigned StartOfTokenColumn, bool InPPDirective = false); + /// \brief Replaces the whitespace in front of \p Tok while reflowing the + /// comment - erases the comment prefix./ Only call once for each + /// \c AnnotatedToken. + void replaceBeginningWhitespaceForReflowingLineComment( + const FormatToken &Tok, unsigned Spaces, unsigned StartOfTokenColumn, + unsigned NumOfForwardSlashesInCommentMarker, bool InPPDirective = false); + /// \brief Adds information about an unchangeable token's whitespace. /// /// Needs to be called for every token for which \c replaceWhitespace Index: lib/Format/WhitespaceManager.cpp =================================================================== --- lib/Format/WhitespaceManager.cpp +++ lib/Format/WhitespaceManager.cpp @@ -43,6 +43,22 @@ Replaces.clear(); } +void WhitespaceManager::replaceBeginningWhitespaceForReflowingLineComment( + const FormatToken &Tok, unsigned Spaces, unsigned StartOfTokenColumn, + unsigned NumOfForwardSlashesInCommentMarker, + bool InPPDirective /*= false*/) { + if (Tok.Finalized) + return; + // Tok.Decision = FD_Continue; + + SourceRange CommentBeginRange = + SourceRange(Tok.WhitespaceRange.getBegin(), + Tok.WhitespaceRange.getEnd().getLocWithOffset( + NumOfForwardSlashesInCommentMarker)); + Changes.push_back(Change(true, CommentBeginRange, /*IndentLevel*/ 0, Spaces, + StartOfTokenColumn, /*Newlines*/ 0, "", "", + Tok.Tok.getKind(), InPPDirective && !Tok.IsFirst)); +} void WhitespaceManager::replaceWhitespace(FormatToken &Tok, unsigned Newlines, unsigned IndentLevel, unsigned Spaces, unsigned StartOfTokenColumn, @@ -223,6 +239,7 @@ if (i + 1 != End) Changes[i + 1].PreviousEndOfTokenColumn += Shift; Changes[i].StartOfTokenColumn += Shift; + } } Index: tools/clang-format/ClangFormat.cpp =================================================================== --- tools/clang-format/ClangFormat.cpp +++ tools/clang-format/ClangFormat.cpp @@ -254,6 +254,10 @@ } return false; } +// FIXME: This is a terrible hack - there should be a better way to identify and +// broadcast that we are in editor mode. +static bool IsInEditorMode; +bool isInEditorMode() { return IsInEditorMode; } } // namespace format } // namespace clang @@ -263,6 +267,7 @@ OS << clang::getClangToolFullVersion("clang-format") << '\n'; } + int main(int argc, const char **argv) { llvm::sys::PrintStackTraceOnErrorSignal(); @@ -297,6 +302,10 @@ llvm::outs() << Config << "\n"; return 0; } + // FIXME: Yuk! This is an unreliable hack to detect if we are in editor mode - + // there must be a way to have all editors identify this? + clang::format::IsInEditorMode = + Offsets.getNumOccurrences() && Lengths.getNumOccurrences(); bool Error = false; switch (FileNames.size()) { @@ -318,3 +327,4 @@ } return Error ? 1 : 0; } +