[PATCH] D157568: [clang-format] Handle NamespaceMacro string arg for FixNamespaceComments

2023-08-16 Thread Owen Pan via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG063c42e919c0: [clang-format] Handle NamespaceMacro string 
arg for FixNamespaceComments (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157568/new/

https://reviews.llvm.org/D157568

Files:
  clang/lib/Format/NamespaceEndCommentsFixer.cpp
  clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -40,6 +40,18 @@
 Code,
 /*Ranges=*/{1, tooling::Range(0, Code.size())}, Style);
   }
+
+  bool isFormatted(StringRef Code, const std::vector &Ranges,
+   const FormatStyle &Style = getLLVMStyle()) const {
+return clang::format::fixNamespaceEndComments(Style, Code, Ranges,
+  "")
+.empty();
+  }
+
+  bool isFormatted(StringRef Code,
+   const FormatStyle &Style = getLLVMStyle()) const {
+return isFormatted(Code, {1, tooling::Range(0, Code.size())}, Style);
+  }
 };
 
 TEST_F(NamespaceEndCommentsFixerTest, AddsEndComment) {
@@ -688,48 +700,34 @@
   FormatStyle Style = getLLVMStyle();
   Style.NamespaceMacros.push_back("TESTSUITE");
 
-  EXPECT_EQ("TESTSUITE() {\n"
-"int i;\n"
-"} // end anonymous TESTSUITE()",
-fixNamespaceEndComments("TESTSUITE() {\n"
-"int i;\n"
-"} // end anonymous TESTSUITE()",
-Style));
-  EXPECT_EQ("TESTSUITE(A) {\n"
-"int i;\n"
-"} /* end of TESTSUITE(A) */",
-fixNamespaceEndComments("TESTSUITE(A) {\n"
-"int i;\n"
-"} /* end of TESTSUITE(A) */",
-Style));
-  EXPECT_EQ("TESTSUITE(A) {\n"
-"int i;\n"
-"}   //   TESTSUITE(A)",
-fixNamespaceEndComments("TESTSUITE(A) {\n"
-"int i;\n"
-"}   //   TESTSUITE(A)",
-Style));
-  EXPECT_EQ("TESTSUITE(A::B) {\n"
-"int i;\n"
-"} // end TESTSUITE(A::B)",
-fixNamespaceEndComments("TESTSUITE(A::B) {\n"
-"int i;\n"
-"} // end TESTSUITE(A::B)",
-Style));
-  EXPECT_EQ("TESTSUITE(A) {\n"
-"int i;\n"
-"}; // end TESTSUITE(A)",
-fixNamespaceEndComments("TESTSUITE(A) {\n"
-"int i;\n"
-"}; // end TESTSUITE(A)",
-Style));
-  EXPECT_EQ("TESTSUITE() {\n"
-"int i;\n"
-"}; /* unnamed TESTSUITE() */",
-fixNamespaceEndComments("TESTSUITE() {\n"
-"int i;\n"
-"}; /* unnamed TESTSUITE() */",
-Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE() {\n"
+  "int i;\n"
+  "} // end anonymous TESTSUITE()",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(A) {\n"
+  "int i;\n"
+  "} /* end of TESTSUITE(A) */",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(A) {\n"
+  "int i;\n"
+  "}   //   TESTSUITE(A)",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(A::B) {\n"
+  "int i;\n"
+  "} // end TESTSUITE(A::B)",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(A) {\n"
+  "int i;\n"
+  "}; // end TESTSUITE(A)",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE() {\n"
+  "int i;\n"
+  "}; /* unnamed TESTSUITE() */",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(\"foo\") {\n"
+  "int i;\n"
+  "} // TESTSUITE(\"foo\")",
+  Style));
 }
 
 TEST_F(NamespaceEndCommentsFixerTest, UpdatesInvalidEndLineComment) {
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -174,7 +174,7 @@
   llvm::Regex::IgnoreCase);
   static const llvm::Regex Namespa

[PATCH] D157568: [clang-format] Handle NamespaceMacro string arg for FixNamespaceComments

2023-08-13 Thread Owen Pan via Phabricator via cfe-commits 
owenpan added inline comments.



Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:177
   llvm::Regex("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
-  "([a-zA-Z0-9_]+)\\(([a-zA-Z0-9:_]*)\\)\\.? *(\\*/)?$",
+  "([a-zA-Z0-9_]+)\\(([a-zA-Z0-9:_]*|\".+\")\\)\\.? *(\\*/)?$",
   llvm::Regex::IgnoreCase);

HazardyKnusperkeks wrote:
> Maybe this?
I don't think we'd need it because the `+` is greedy.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157568/new/

https://reviews.llvm.org/D157568

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157568: [clang-format] Handle NamespaceMacro string arg for FixNamespaceComments

2023-08-13 Thread Björn Schäpers via Phabricator via cfe-commits 
HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:177
   llvm::Regex("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
-  "([a-zA-Z0-9_]+)\\(([a-zA-Z0-9:_]*)\\)\\.? *(\\*/)?$",
+  "([a-zA-Z0-9_]+)\\(([a-zA-Z0-9:_]*|\".+\")\\)\\.? *(\\*/)?$",
   llvm::Regex::IgnoreCase);

Maybe this?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157568/new/

https://reviews.llvm.org/D157568

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157568: [clang-format] Handle NamespaceMacro string arg for FixNamespaceComments

2023-08-09 Thread Owen Pan via Phabricator via cfe-commits 
owenpan updated this revision to Diff 548878.
owenpan added a comment.

Rebased and updated test cases.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157568/new/

https://reviews.llvm.org/D157568

Files:
  clang/lib/Format/NamespaceEndCommentsFixer.cpp
  clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -40,6 +40,18 @@
 Code,
 /*Ranges=*/{1, tooling::Range(0, Code.size())}, Style);
   }
+
+  bool isFormatted(StringRef Code, const std::vector &Ranges,
+   const FormatStyle &Style = getLLVMStyle()) const {
+return clang::format::fixNamespaceEndComments(Style, Code, Ranges,
+  "")
+.empty();
+  }
+
+  bool isFormatted(StringRef Code,
+   const FormatStyle &Style = getLLVMStyle()) const {
+return isFormatted(Code, {1, tooling::Range(0, Code.size())}, Style);
+  }
 };
 
 TEST_F(NamespaceEndCommentsFixerTest, AddsEndComment) {
@@ -688,48 +700,34 @@
   FormatStyle Style = getLLVMStyle();
   Style.NamespaceMacros.push_back("TESTSUITE");
 
-  EXPECT_EQ("TESTSUITE() {\n"
-"int i;\n"
-"} // end anonymous TESTSUITE()",
-fixNamespaceEndComments("TESTSUITE() {\n"
-"int i;\n"
-"} // end anonymous TESTSUITE()",
-Style));
-  EXPECT_EQ("TESTSUITE(A) {\n"
-"int i;\n"
-"} /* end of TESTSUITE(A) */",
-fixNamespaceEndComments("TESTSUITE(A) {\n"
-"int i;\n"
-"} /* end of TESTSUITE(A) */",
-Style));
-  EXPECT_EQ("TESTSUITE(A) {\n"
-"int i;\n"
-"}   //   TESTSUITE(A)",
-fixNamespaceEndComments("TESTSUITE(A) {\n"
-"int i;\n"
-"}   //   TESTSUITE(A)",
-Style));
-  EXPECT_EQ("TESTSUITE(A::B) {\n"
-"int i;\n"
-"} // end TESTSUITE(A::B)",
-fixNamespaceEndComments("TESTSUITE(A::B) {\n"
-"int i;\n"
-"} // end TESTSUITE(A::B)",
-Style));
-  EXPECT_EQ("TESTSUITE(A) {\n"
-"int i;\n"
-"}; // end TESTSUITE(A)",
-fixNamespaceEndComments("TESTSUITE(A) {\n"
-"int i;\n"
-"}; // end TESTSUITE(A)",
-Style));
-  EXPECT_EQ("TESTSUITE() {\n"
-"int i;\n"
-"}; /* unnamed TESTSUITE() */",
-fixNamespaceEndComments("TESTSUITE() {\n"
-"int i;\n"
-"}; /* unnamed TESTSUITE() */",
-Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE() {\n"
+  "int i;\n"
+  "} // end anonymous TESTSUITE()",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(A) {\n"
+  "int i;\n"
+  "} /* end of TESTSUITE(A) */",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(A) {\n"
+  "int i;\n"
+  "}   //   TESTSUITE(A)",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(A::B) {\n"
+  "int i;\n"
+  "} // end TESTSUITE(A::B)",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(A) {\n"
+  "int i;\n"
+  "}; // end TESTSUITE(A)",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE() {\n"
+  "int i;\n"
+  "}; /* unnamed TESTSUITE() */",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(\"foo\") {\n"
+  "int i;\n"
+  "} // TESTSUITE(\"foo\")",
+  Style));
 }
 
 TEST_F(NamespaceEndCommentsFixerTest, UpdatesInvalidEndLineComment) {
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -174,7 +174,7 @@
   llvm::Regex::IgnoreCase);
   static const llvm::Regex NamespaceMacroCommentPattern =
   llvm::Regex("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
-  "([a-zA-Z0-9_]+)\\(([a-zA-Z0

[PATCH] D157568: [clang-format] Handle NamespaceMacro string arg for FixNamespaceComments

2023-08-09 Thread Owen Pan via Phabricator via cfe-commits 
owenpan created this revision.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, MyDeveloperDay.
owenpan requested review of this revision.

Fixes https://github.com/llvm/llvm-project/issues/63795.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157568

Files:
  clang/lib/Format/NamespaceEndCommentsFixer.cpp
  clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp


Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -40,8 +40,30 @@
 Code,
 /*Ranges=*/{1, tooling::Range(0, Code.size())}, Style);
   }
+
+  bool isFormatted(StringRef Code, const std::vector &Ranges,
+   const FormatStyle &Style = getLLVMStyle()) const {
+return clang::format::fixNamespaceEndComments(Style, Code, Ranges,
+  "")
+.empty();
+  }
+
+  bool isFormatted(StringRef Code,
+   const FormatStyle &Style = getLLVMStyle()) const {
+return isFormatted(Code, {1, tooling::Range(0, Code.size())}, Style);
+  }
 };
 
+TEST_F(NamespaceEndCommentsFixerTest, IsFormatted) {
+  auto Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("SUITE");
+  EXPECT_TRUE(isFormatted("SUITE(\"foo\") {\n"
+  "int i;\n"
+  "int j;\n"
+  "} // SUITE(\"foo\")",
+  Style));
+}
+
 TEST_F(NamespaceEndCommentsFixerTest, AddsEndComment) {
   EXPECT_EQ("namespace {\n"
 "int i;\n"
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -174,7 +174,8 @@
   llvm::Regex::IgnoreCase);
   static const llvm::Regex NamespaceMacroCommentPattern =
   llvm::Regex("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
-  "([a-zA-Z0-9_]+)\\(([a-zA-Z0-9:_]*)\\)\\.? *(\\*/)?$",
+  // NamespaceMacro arguments can also be string literals.
+  "([a-zA-Z0-9_]+)\\(([a-zA-Z0-9:_]*|\".+\")\\)\\.? *(\\*/)?$",
   llvm::Regex::IgnoreCase);
 
   SmallVector Groups;


Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -40,8 +40,30 @@
 Code,
 /*Ranges=*/{1, tooling::Range(0, Code.size())}, Style);
   }
+
+  bool isFormatted(StringRef Code, const std::vector &Ranges,
+   const FormatStyle &Style = getLLVMStyle()) const {
+return clang::format::fixNamespaceEndComments(Style, Code, Ranges,
+  "")
+.empty();
+  }
+
+  bool isFormatted(StringRef Code,
+   const FormatStyle &Style = getLLVMStyle()) const {
+return isFormatted(Code, {1, tooling::Range(0, Code.size())}, Style);
+  }
 };
 
+TEST_F(NamespaceEndCommentsFixerTest, IsFormatted) {
+  auto Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("SUITE");
+  EXPECT_TRUE(isFormatted("SUITE(\"foo\") {\n"
+  "int i;\n"
+  "int j;\n"
+  "} // SUITE(\"foo\")",
+  Style));
+}
+
 TEST_F(NamespaceEndCommentsFixerTest, AddsEndComment) {
   EXPECT_EQ("namespace {\n"
 "int i;\n"
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -174,7 +174,8 @@
   llvm::Regex::IgnoreCase);
   static const llvm::Regex NamespaceMacroCommentPattern =
   llvm::Regex("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
-  "([a-zA-Z0-9_]+)\\(([a-zA-Z0-9:_]*)\\)\\.? *(\\*/)?$",
+  // NamespaceMacro arguments can also be string literals.
+  "([a-zA-Z0-9_]+)\\(([a-zA-Z0-9:_]*|\".+\")\\)\\.? *(\\*/)?$",
   llvm::Regex::IgnoreCase);
 
   SmallVector Groups;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits