Index: cfe/trunk/lib/Format/BreakableToken.cpp =================================================================== --- cfe/trunk/lib/Format/BreakableToken.cpp +++ cfe/trunk/lib/Format/BreakableToken.cpp @@ -67,10 +67,11 @@ unsigned ContentStartColumn, unsigned ColumnLimit, unsigned TabWidth, - encoding::Encoding Encoding) { - LLVM_DEBUG(llvm::dbgs() << "Comment split: \"" << Text << ", " << ColumnLimit - << "\", Content start: " << ContentStartColumn - << "\n"); + encoding::Encoding Encoding, + const FormatStyle &Style) { + LLVM_DEBUG(llvm::dbgs() << "Comment split: \"" << Text + << "\", Column limit: " << ColumnLimit + << ", Content start: " << ContentStartColumn << "\n"); if (ColumnLimit <= ContentStartColumn + 1) return BreakableToken::Split(StringRef::npos, 0); @@ -95,6 +96,13 @@ if (SpaceOffset != StringRef::npos && kNumberedListRegexp->match(Text.substr(SpaceOffset).ltrim(Blanks))) SpaceOffset = Text.find_last_of(Blanks, SpaceOffset); + // In JavaScript, some @tags can be followed by {, and machinery that parses + // these comments will fail to understand the comment if followed by a line + // break. So avoid ever breaking before a {. + if (Style.Language == FormatStyle::LK_JavaScript && + SpaceOffset != StringRef::npos && SpaceOffset + 1 < Text.size() && + Text[SpaceOffset + 1] == '{') + SpaceOffset = Text.find_last_of(Blanks, SpaceOffset); if (SpaceOffset == StringRef::npos || // Don't break at leading whitespace. @@ -109,6 +117,12 @@ Blanks, std::max(MaxSplitBytes, FirstNonWhitespace)); } if (SpaceOffset != StringRef::npos && SpaceOffset != 0) { + // adaptStartOfLine will break after lines starting with /** if the comment + // is broken anywhere. Avoid emitting this break twice here. + // Example: in /** longtextcomesherethatbreaks */ (with ColumnLimit 20) will + // insert a break after /**, so this code must not insert the same break. + if (SpaceOffset == 1 && Text[SpaceOffset - 1] == '*') + return BreakableToken::Split(StringRef::npos, 0); StringRef BeforeCut = Text.substr(0, SpaceOffset).rtrim(Blanks); StringRef AfterCut = Text.substr(SpaceOffset).ltrim(Blanks); return BreakableToken::Split(BeforeCut.size(), @@ -260,7 +274,7 @@ return Split(StringRef::npos, 0); return getCommentSplit(Content[LineIndex].substr(TailOffset), ContentStartColumn, ColumnLimit, Style.TabWidth, - Encoding); + Encoding, Style); } void BreakableComment::compressWhitespace( @@ -620,6 +634,8 @@ if (DelimitersOnNewline) { // Since we're breaking at index 1 below, the break position and the // break length are the same. + // Note: this works because getCommentSplit is careful never to split at + // the beginning of a line. size_t BreakLength = Lines[0].substr(1).find_first_not_of(Blanks); if (BreakLength != StringRef::npos) insertBreak(LineIndex, 0, Split(1, BreakLength), /*ContentIndent=*/0, Index: cfe/trunk/unittests/Format/FormatTestComments.cpp =================================================================== --- cfe/trunk/unittests/Format/FormatTestComments.cpp +++ cfe/trunk/unittests/Format/FormatTestComments.cpp @@ -1254,6 +1254,12 @@ " */", getLLVMStyleWithColumns(20))); + // This reproduces a crashing bug where both adaptStartOfLine and + // getCommentSplit were trying to wrap after the "/**". + EXPECT_EQ("/** multilineblockcommentwithnowrapopportunity */", + format("/** multilineblockcommentwithnowrapopportunity */", + getLLVMStyleWithColumns(20))); + EXPECT_EQ("/*\n" "\n" "\n" Index: cfe/trunk/unittests/Format/FormatTestJS.cpp =================================================================== --- cfe/trunk/unittests/Format/FormatTestJS.cpp +++ cfe/trunk/unittests/Format/FormatTestJS.cpp @@ -191,31 +191,32 @@ // Break a single line long jsdoc comment pragma. EXPECT_EQ("/**\n" - " * @returns\n" - " * {string}\n" - " * jsdoc line 12\n" + " * @returns {string} jsdoc line 12\n" " */", format("/** @returns {string} jsdoc line 12 */", getGoogleJSStyleWithColumns(20))); - EXPECT_EQ("/**\n" - " * @returns\n" - " * {string}\n" + " * @returns {string}\n" " * jsdoc line 12\n" " */", + format("/** @returns {string} jsdoc line 12 */", + getGoogleJSStyleWithColumns(25))); + + EXPECT_EQ("/**\n" + " * @returns {string} jsdoc line 12\n" + " */", format("/** @returns {string} jsdoc line 12 */", getGoogleJSStyleWithColumns(20))); // FIXME: this overcounts the */ as a continuation of the 12 when breaking. // Related to the FIXME in BreakableBlockComment::getRangeLength. EXPECT_EQ("/**\n" - " * @returns\n" - " * {string}\n" - " * jsdoc line\n" + " * @returns {string}\n" + " * jsdoc line line\n" " * 12\n" " */", - format("/** @returns {string} jsdoc line 12*/", - getGoogleJSStyleWithColumns(20))); + format("/** @returns {string} jsdoc line line 12*/", + getGoogleJSStyleWithColumns(25))); // Fix a multiline jsdoc comment ending in a comment pragma. EXPECT_EQ("/**\n" @@ -2038,27 +2039,32 @@ TEST_F(FormatTestJS, JSDocAnnotations) { verifyFormat("/**\n" - " * @exports\n" - " * {this.is.a.long.path.to.a.Type}\n" + " * @exports {this.is.a.long.path.to.a.Type}\n" " */", "/**\n" " * @exports {this.is.a.long.path.to.a.Type}\n" " */", getGoogleJSStyleWithColumns(20)); verifyFormat("/**\n" - " * @mods\n" - " * {this.is.a.long.path.to.a.Type}\n" + " * @mods {this.is.a.long.path.to.a.Type}\n" + " */", + "/**\n" + " * @mods {this.is.a.long.path.to.a.Type}\n" + " */", + getGoogleJSStyleWithColumns(20)); + verifyFormat("/**\n" + " * @mods {this.is.a.long.path.to.a.Type}\n" " */", "/**\n" " * @mods {this.is.a.long.path.to.a.Type}\n" " */", getGoogleJSStyleWithColumns(20)); verifyFormat("/**\n" - " * @param\n" - " * {this.is.a.long.path.to.a.Type}\n" + " * @param {canWrap\n" + " * onSpace}\n" " */", "/**\n" - " * @param {this.is.a.long.path.to.a.Type}\n" + " * @param {canWrap onSpace}\n" " */", getGoogleJSStyleWithColumns(20)); verifyFormat("/**\n" @@ -2083,8 +2089,7 @@ " /**\n" " * long long long\n" " * long\n" - " * @param\n" - " * {this.is.a.long.path.to.a.Type}\n" + " * @param {this.is.a.long.path.to.a.Type}\n" " * a\n" " * long long long\n" " * long long\n"