Author: mydeveloperday Date: 2020-05-15T21:00:55+01:00 New Revision: e8ea35e63f50486fe497d8565abb8cd5b2820a96
URL: https://github.com/llvm/llvm-project/commit/e8ea35e63f50486fe497d8565abb8cd5b2820a96 DIFF: https://github.com/llvm/llvm-project/commit/e8ea35e63f50486fe497d8565abb8cd5b2820a96.diff LOG: [clang-format] [PR44345] Long namespace closing comment is duplicated endlessly Summary: https://bugs.llvm.org/show_bug.cgi?id=44345 When namespaces get long the namespace end comment wraps onto the next line ``` namespace would::it::save::you::a::lot::of::time::if_::i::just::gave::up::and_:: went::mad::now { void foo(); void bar(); } // namespace // would::it::save::you::a::lot::of::time::if_::i::just::gave::up::and_::went::mad::now ``` If clang-format it applied successively it will duplicate the end comment ``` namespace would::it::save::you::a::lot::of::time::if_::i::just::gave::up::and_:: went::mad::now { void foo(); void bar(); } // namespace // would::it::save::you::a::lot::of::time::if_::i::just::gave::up::and_::went::mad::now // would::it::save::you::a::lot::of::time::if_::i::just::gave::up::and_::went::mad::now ``` This revision checks to ensure the end comment is not on the next line before adding yet another comment Reviewed By: krasimir Subscribers: cfe-commits Tags: #clang, #clang-format Differential Revision: https://reviews.llvm.org/D79935 Added: Modified: clang/lib/Format/NamespaceEndCommentsFixer.cpp clang/unittests/Format/FormatTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Format/NamespaceEndCommentsFixer.cpp b/clang/lib/Format/NamespaceEndCommentsFixer.cpp index 20b424f86077..92707150fcdb 100644 --- a/clang/lib/Format/NamespaceEndCommentsFixer.cpp +++ b/clang/lib/Format/NamespaceEndCommentsFixer.cpp @@ -121,7 +121,25 @@ bool validEndComment(const FormatToken *RBraceTok, StringRef NamespaceName, // Named namespace comments must not mention anonymous namespace. if (!NamespaceName.empty() && !AnonymousInComment.empty()) return false; - return NamespaceNameInComment == NamespaceName; + if (NamespaceNameInComment == NamespaceName) + return true; + + // Has namespace comment flowed onto the next line. + // } // namespace + // // verylongnamespacenamethatdidnotfitonthepreviouscommentline + if (!(Comment->Next && Comment->Next->is(TT_LineComment))) + return false; + + static const llvm::Regex CommentPattern = llvm::Regex( + "^/[/*] *( +([a-zA-Z0-9:_]+))?\\.? *(\\*/)?$", llvm::Regex::IgnoreCase); + + // Pull out just the comment text. + if (!CommentPattern.match(Comment->Next->TokenText, &Groups)) { + return false; + } + NamespaceNameInComment = Groups.size() > 2 ? Groups[2] : ""; + + return (NamespaceNameInComment == NamespaceName); } void addEndComment(const FormatToken *RBraceTok, StringRef EndCommentText, diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index f6f4ea825086..a421ca9f131c 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -16238,6 +16238,74 @@ TEST_F(FormatTest, OperatorSpacing) { verifyFormat("operator&&(int(&&)(), class Foo);", Style); } +TEST_F(FormatTest, VeryLongNamespaceCommentSplit) { + // These tests are not in NamespaceFixer because that doesn't + // test its interaction with line wrapping + FormatStyle Style = getLLVMStyle(); + Style.ColumnLimit = 80; + verifyFormat("namespace {\n" + "int i;\n" + "int j;\n" + "} // namespace", + Style); + + verifyFormat("namespace AAA {\n" + "int i;\n" + "int j;\n" + "} // namespace AAA", + Style); + + EXPECT_EQ("namespace Averyveryveryverylongnamespace {\n" + "int i;\n" + "int j;\n" + "} // namespace Averyveryveryverylongnamespace", + format("namespace Averyveryveryverylongnamespace {\n" + "int i;\n" + "int j;\n" + "}", + Style)); + + EXPECT_EQ( + "namespace " + "would::it::save::you::a::lot::of::time::if_::i::just::gave::up::and_::\n" + " went::mad::now {\n" + "int i;\n" + "int j;\n" + "} // namespace\n" + " // " + "would::it::save::you::a::lot::of::time::if_::i::just::gave::up::and_::" + "went::mad::now", + format("namespace " + "would::it::save::you::a::lot::of::time::if_::i::" + "just::gave::up::and_::went::mad::now {\n" + "int i;\n" + "int j;\n" + "}", + Style)); + + // This used to duplicate the comment again and again on subsequent runs + EXPECT_EQ( + "namespace " + "would::it::save::you::a::lot::of::time::if_::i::just::gave::up::and_::\n" + " went::mad::now {\n" + "int i;\n" + "int j;\n" + "} // namespace\n" + " // " + "would::it::save::you::a::lot::of::time::if_::i::just::gave::up::and_::" + "went::mad::now", + format("namespace " + "would::it::save::you::a::lot::of::time::if_::i::" + "just::gave::up::and_::went::mad::now {\n" + "int i;\n" + "int j;\n" + "} // namespace\n" + " // " + "would::it::save::you::a::lot::of::time::if_::i::just::gave::up::" + "and_::went::mad::now", + Style)); +} + } // namespace } // namespace format } // namespace clang _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
