Index: cfe/trunk/lib/Format/BreakableToken.h =================================================================== --- cfe/trunk/lib/Format/BreakableToken.h +++ cfe/trunk/lib/Format/BreakableToken.h @@ -58,6 +58,8 @@ /// operations that might be executed before the main line breaking occurs: /// - getSplitBefore, for finding a split such that the content preceding it /// needs to be specially reflown, +/// - 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 @@ -135,6 +137,12 @@ return Split(StringRef::npos, 0); } + /// \brief Returns if a break before the content at \p LineIndex will be + /// inserted after the whitespace preceding the content has been reformatted. + virtual bool introducesBreakBefore(unsigned LineIndex) 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 @@ -339,6 +347,7 @@ Split getSplitBefore(unsigned LineIndex, unsigned PreviousEndColumn, unsigned ColumnLimit, llvm::Regex &CommentPragmasRegex) const override; + bool introducesBreakBefore(unsigned LineIndex) const override; unsigned getLineLengthAfterSplitBefore(unsigned LineIndex, unsigned TailOffset, unsigned PreviousEndColumn, Index: cfe/trunk/lib/Format/BreakableToken.cpp =================================================================== --- cfe/trunk/lib/Format/BreakableToken.cpp +++ cfe/trunk/lib/Format/BreakableToken.cpp @@ -599,6 +599,12 @@ } } +bool BreakableBlockComment::introducesBreakBefore(unsigned LineIndex) const { + // A break is introduced when we want delimiters on newline. + return LineIndex == 0 && 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) { Index: cfe/trunk/lib/Format/ContinuationIndenter.h =================================================================== --- cfe/trunk/lib/Format/ContinuationIndenter.h +++ cfe/trunk/lib/Format/ContinuationIndenter.h @@ -318,6 +318,9 @@ /// \brief \c true if this line contains a continued for-loop section. bool LineContainsContinuedForLoopSection; + /// \brief \c true if \p NextToken should not continue this line. + bool NoContinuation; + /// \brief The \c NestingLevel at the start of this line. unsigned StartOfLineLevel; @@ -364,6 +367,8 @@ if (LineContainsContinuedForLoopSection != Other.LineContainsContinuedForLoopSection) return LineContainsContinuedForLoopSection; + if (NoContinuation != Other.NoContinuation) + return NoContinuation; if (StartOfLineLevel != Other.StartOfLineLevel) return StartOfLineLevel < Other.StartOfLineLevel; if (LowestLevelOnLine != Other.LowestLevelOnLine) Index: cfe/trunk/lib/Format/ContinuationIndenter.cpp =================================================================== --- cfe/trunk/lib/Format/ContinuationIndenter.cpp +++ cfe/trunk/lib/Format/ContinuationIndenter.cpp @@ -106,6 +106,7 @@ /*AvoidBinPacking=*/false, /*NoLineBreak=*/false)); State.LineContainsContinuedForLoopSection = false; + State.NoContinuation = false; State.StartOfStringLiteral = 0; State.StartOfLineLevel = 0; State.LowestLevelOnLine = 0; @@ -322,6 +323,12 @@ Previous.TokenText == "\'\\n\'")))) return true; + if (Previous.is(TT_BlockComment) && Previous.IsMultiline) + return true; + + if (State.NoContinuation) + return true; + return false; } @@ -331,6 +338,8 @@ const FormatToken &Current = *State.NextToken; assert(!State.Stack.empty()); + State.NoContinuation = false; + if ((Current.is(TT_ImplicitStringLiteral) && (Current.Previous->Tok.getIdentifierInfo() == nullptr || Current.Previous->Tok.getIdentifierInfo()->getPPKeywordID() == @@ -1286,7 +1295,7 @@ return 0; } } else if (Current.is(TT_BlockComment)) { - if (!Current.isTrailingComment() || !Style.ReflowComments || + if (!Style.ReflowComments || // If a comment token switches formatting, like // /* clang-format on */, we don't want to break it further, // but we may still want to adjust its indentation. @@ -1332,6 +1341,7 @@ ReflowInProgress = SplitBefore.first != StringRef::npos; TailOffset = ReflowInProgress ? (SplitBefore.first + SplitBefore.second) : 0; + BreakInserted = BreakInserted || Token->introducesBreakBefore(LineIndex); if (!DryRun) Token->replaceWhitespaceBefore(LineIndex, RemainingTokenColumns, RemainingSpace, SplitBefore, Whitespaces); @@ -1408,6 +1418,9 @@ State.Stack[i].BreakBeforeParameter = true; } + if (Current.is(TT_BlockComment)) + State.NoContinuation = true; + Penalty += Current.isStringLiteral() ? Style.PenaltyBreakString : Style.PenaltyBreakComment; Index: cfe/trunk/unittests/Format/FormatTestComments.cpp =================================================================== --- cfe/trunk/unittests/Format/FormatTestComments.cpp +++ cfe/trunk/unittests/Format/FormatTestComments.cpp @@ -2407,6 +2407,57 @@ getLLVMStyleWithColumns(15))); } +TEST_F(FormatTestComments, BreaksAfterMultilineBlockCommentsInParamLists) { + EXPECT_EQ("a = f(/* long\n" + " long\n" + " */\n" + " a);", + format("a = f(/* long long */ a);", getLLVMStyleWithColumns(15))); + + EXPECT_EQ("a = f(/* long\n" + " long\n" + " */\n" + " a);", + format("a = f(/* long\n" + " long\n" + " */a);", + getLLVMStyleWithColumns(15))); + + EXPECT_EQ("a = f(/* long\n" + " long\n" + " */\n" + " a);", + format("a = f(/* long\n" + " long\n" + " */ a);", + getLLVMStyleWithColumns(15))); + + EXPECT_EQ("a = f(/* long\n" + " long\n" + " */\n" + " (1 + 1));", + format("a = f(/* long\n" + " long\n" + " */ (1 + 1));", + getLLVMStyleWithColumns(15))); + + EXPECT_EQ( + "a = f(a,\n" + " /* long\n" + " long\n" + " */\n" + " b);", + format("a = f(a, /* long long */ b);", getLLVMStyleWithColumns(15))); + + EXPECT_EQ( + "a = f(a,\n" + " /* long\n" + " long\n" + " */\n" + " (1 + 1));", + format("a = f(a, /* long long */ (1 + 1));", getLLVMStyleWithColumns(15))); +} + TEST_F(FormatTestComments, IndentLineCommentsInStartOfBlockAtEndOfFile) { verifyFormat("{\n" " // a\n" @@ -2805,6 +2856,22 @@ getLLVMStyleWithColumns(80))); // clang-format on } + +TEST_F(FormatTestComments, NonTrailingBlockComments) { + verifyFormat("const /** comment comment */ A = B;", + getLLVMStyleWithColumns(40)); + + verifyFormat("const /** comment comment comment */ A =\n" + " B;", + getLLVMStyleWithColumns(40)); + + EXPECT_EQ("const /** comment comment comment\n" + " comment */\n" + " A = B;", + format("const /** comment comment comment comment */\n" + " A = B;", + getLLVMStyleWithColumns(40))); +} } // end namespace } // end namespace format } // end namespace clang Index: cfe/trunk/unittests/Format/FormatTestJS.cpp =================================================================== --- cfe/trunk/unittests/Format/FormatTestJS.cpp +++ cfe/trunk/unittests/Format/FormatTestJS.cpp @@ -65,6 +65,27 @@ TEST_F(FormatTestJS, BlockComments) { verifyFormat("/* aaaaaaaaaaaaa */ aaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); + // Breaks after a single line block comment. + EXPECT_EQ("aaaaa = bbbb.ccccccccccccccc(\n" + " /** @type_{!cccc.rrrrrrr.MMMMMMMMMMMM.LLLLLLLLLLL.lala} */\n" + " mediaMessage);", + format("aaaaa = bbbb.ccccccccccccccc(\n" + " /** " + "@type_{!cccc.rrrrrrr.MMMMMMMMMMMM.LLLLLLLLLLL.lala} */ " + "mediaMessage);", + getGoogleJSStyleWithColumns(70))); + // Breaks after a multiline block comment. + EXPECT_EQ( + "aaaaa = bbbb.ccccccccccccccc(\n" + " /**\n" + " * @type_{!cccc.rrrrrrr.MMMMMMMMMMMM.LLLLLLLLLLL.lala}\n" + " */\n" + " mediaMessage);", + format("aaaaa = bbbb.ccccccccccccccc(\n" + " /**\n" + " * @type_{!cccc.rrrrrrr.MMMMMMMMMMMM.LLLLLLLLLLL.lala}\n" + " */ mediaMessage);", + getGoogleJSStyleWithColumns(70))); } TEST_F(FormatTestJS, JSDocComments) {