HazardyKnusperkeks created this revision.
HazardyKnusperkeks added reviewers: owenpan, MyDeveloperDay, rymiel.
HazardyKnusperkeks added a project: clang-format.
Herald added a project: All.
HazardyKnusperkeks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Fixes https://github.com/llvm/llvm-project/issues/57504.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D138263
Files:
clang/lib/Format/WhitespaceManager.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/FormatTestComments.cpp
Index: clang/unittests/Format/FormatTestComments.cpp
===================================================================
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -68,6 +68,20 @@
EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
}
+ void verifyFormat(llvm::StringRef Expected, llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle()) {
+ EXPECT_EQ(Expected.str(), format(Expected, Style))
+ << "Expected code is not stable";
+ EXPECT_EQ(Expected.str(), format(test::messUp(Code), Style));
+ }
+
+ void verifyFormatWithoutMessUp(llvm::StringRef Expected, llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle()) {
+ EXPECT_EQ(Expected.str(), format(Expected, Style))
+ << "Expected code is not stable";
+ EXPECT_EQ(Expected.str(), format(Code, Style));
+ }
+
void verifyGoogleFormat(llvm::StringRef Code) {
verifyFormat(Code, getGoogleStyle());
}
@@ -3076,6 +3090,55 @@
Style));
}
+TEST_F(FormatTestComments, DontAlignNamespaceComments) {
+ FormatStyle Style = getLLVMStyle();
+ Style.NamespaceIndentation = FormatStyle::NI_All;
+ Style.NamespaceMacros.push_back("TESTSUITE");
+ Style.ShortNamespaceLines = 0;
+
+ llvm::StringRef Input = "namespace A {\n"
+ " TESTSUITE(B) {\n"
+ " namespace C {\n"
+ " namespace D {} // namespace D\n"
+ " std::string Foo = Bar; // Comment\n"
+ " } // namespace C\n"
+ " }\n"
+ "} // NaMeSpAcE A";
+
+ EXPECT_EQ(Style.AlignTrailingComments.Kind, FormatStyle::TCAS_Always);
+ verifyFormat("namespace A {\n"
+ " TESTSUITE(B) {\n"
+ " namespace C {\n"
+ " namespace D {} // namespace D\n"
+ " std::string Foo = Bar; // Comment\n"
+ " } // namespace C\n"
+ " } // TESTSUITE(B)\n"
+ "} // NaMeSpAcE A",
+ Input, Style);
+
+ Style.AlignTrailingComments.Kind = FormatStyle::TCAS_Never;
+ verifyFormat("namespace A {\n"
+ " TESTSUITE(B) {\n"
+ " namespace C {\n"
+ " namespace D {} // namespace D\n"
+ " std::string Foo = Bar; // Comment\n"
+ " } // namespace C\n"
+ " } // TESTSUITE(B)\n"
+ "} // NaMeSpAcE A",
+ Input, Style);
+
+ Style.AlignTrailingComments.Kind = FormatStyle::TCAS_Leave;
+ verifyFormatWithoutMessUp("namespace A {\n"
+ " TESTSUITE(B) {\n"
+ " namespace C {\n"
+ " namespace D {} // namespace D\n"
+ " std::string Foo = Bar; // Comment\n"
+ " } // namespace C\n"
+ " }// TESTSUITE(B)\n"
+ "} // NaMeSpAcE A",
+ Input, Style);
+}
+
TEST_F(FormatTestComments, AlignsBlockCommentDecorations) {
EXPECT_EQ("/*\n"
" */",
Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4034,7 +4034,7 @@
"#define HEADER_GUARD\n"
"namespace my_namespace {\n"
"int i;\n"
- "} // my_namespace\n"
+ "} // my_namespace\n"
"#endif // HEADER_GUARD",
format("#ifndef HEADER_GUARD\n"
" #define HEADER_GUARD\n"
@@ -19219,7 +19219,7 @@
" int x;\n"
" };\n"
" } // namespace b\n"
- " } // namespace a",
+ " } // namespace a",
WhitesmithsBraceStyle);
verifyFormat("void f()\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===================================================================
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -14,6 +14,7 @@
#include "WhitespaceManager.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/Regex.h"
#include <algorithm>
namespace clang {
@@ -987,11 +988,30 @@
if (i + 1 != e && Changes[i + 1].ContinuesPPDirective)
ChangeMaxColumn -= 2;
- // 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].Tok->is(tok::r_brace) &&
- Changes[i - 1].StartOfTokenColumn == 0;
+
+ // Check if a given comment is most likely a comment to end a namespace.
+ auto IsNamespaceComment = [this](llvm::StringRef CommentText) {
+ // The Regex is a subset of what is defined in
+ // NamespaceEndCommentsFixer.cpp validEndComment().
+ static const llvm::Regex NamespaceCommentRegex(
+ "// *(end (of )?)? *(anonymous|unnamed)? *(namespace|[A-Za-z0-9_]+)",
+ llvm::Regex::IgnoreCase);
+ SmallVector<StringRef, 5> Groups;
+ auto Matched = NamespaceCommentRegex.match(CommentText, &Groups);
+ if (!Matched)
+ return false;
+ assert(Groups.size() > 4);
+ StringRef NamespaceText = Groups[4];
+ return NamespaceText.equals_insensitive("namespace") ||
+ llvm::is_contained(Style.NamespaceMacros, NamespaceText);
+ };
+
+ // We don't want to align namespace end comments. To detect that we check
+ // that the comments follows a closing brace, and its comment matches our
+ // heuristic.
+ bool DontAlignThisComment = i > 0 && Changes[i].NewlinesBefore == 0 &&
+ Changes[i - 1].Tok->is(tok::r_brace) &&
+ IsNamespaceComment(Changes[i].Tok->TokenText);
bool WasAlignedWithStartOfNextLine = false;
if (Changes[i].NewlinesBefore >= 1) { // A comment on its own line.
unsigned CommentColumn = SourceMgr.getSpellingColumnNumber(
@@ -1011,7 +1031,7 @@
}
}
if (Style.AlignTrailingComments.Kind == FormatStyle::TCAS_Never ||
- FollowsRBraceInColumn0) {
+ DontAlignThisComment) {
alignTrailingComments(StartOfSequence, i, MinColumn);
MinColumn = ChangeMinColumn;
MaxColumn = ChangeMinColumn;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
