Skip to content

Conversation

@llvmbot
Copy link
Member

@llvmbot llvmbot commented Oct 6, 2024

Backport acf92a4

Requested by: @HighCommander4

@llvmbot llvmbot added this to the LLVM 19.X Release milestone Oct 6, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Oct 6, 2024

@hokein What do you think about merging this PR to the release branch?

@llvmbot
Copy link
Member Author

llvmbot commented Oct 6, 2024

@llvm/pr-subscribers-clang-tidy

Author: None (llvmbot)

Changes

Backport acf92a4

Requested by: @HighCommander4


Full diff: https://github.com/llvm/llvm-project/pull/111318.diff

2 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp (+9-9)
  • (modified) clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp (+16)
diff --git a/clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp
index 4022ea0cdaf5ee..e45687fde6d9f6 100644
--- a/clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp
@@ -204,7 +204,7 @@ utils::UseRangesCheck::ReplacerMap UseRangesCheck::getReplacerMap() const {
   ReplacerMap Results;
   static const Signature SingleSig = {{0}};
   static const Signature TwoSig = {{0}, {2}};
-  static const auto AddFrom =
+  const auto AddFrom =
       [&Results](llvm::IntrusiveRefCntPtr<UseRangesCheck::Replacer> Replacer,
                  std::initializer_list<StringRef> Names, StringRef Prefix) {
         llvm::SmallString<64> Buffer;
@@ -214,17 +214,17 @@ utils::UseRangesCheck::ReplacerMap UseRangesCheck::getReplacerMap() const {
         }
       };
 
-  static const auto AddFromStd =
-      [](llvm::IntrusiveRefCntPtr<UseRangesCheck::Replacer> Replacer,
-         std::initializer_list<StringRef> Names) {
+  const auto AddFromStd =
+      [&](llvm::IntrusiveRefCntPtr<UseRangesCheck::Replacer> Replacer,
+          std::initializer_list<StringRef> Names) {
         AddFrom(Replacer, Names, "std");
       };
 
-  static const auto AddFromBoost =
-      [](llvm::IntrusiveRefCntPtr<UseRangesCheck::Replacer> Replacer,
-         std::initializer_list<
-             std::pair<StringRef, std::initializer_list<StringRef>>>
-             NamespaceAndNames) {
+  const auto AddFromBoost =
+      [&](llvm::IntrusiveRefCntPtr<UseRangesCheck::Replacer> Replacer,
+          std::initializer_list<
+              std::pair<StringRef, std::initializer_list<StringRef>>>
+              NamespaceAndNames) {
         for (auto [Namespace, Names] : NamespaceAndNames)
           AddFrom(Replacer, Names,
                   SmallString<64>{"boost", (Namespace.empty() ? "" : "::"),
diff --git a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
index 75a140767035b2..49a94045ea4878 100644
--- a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -262,6 +262,22 @@ TEST_F(LSPTest, ClangTidyRename) {
   EXPECT_EQ(Params, std::vector{llvm::json::Value(std::move(ExpectedEdit))});
 }
 
+TEST_F(LSPTest, ClangTidyCrash_Issue109367) {
+  // This test requires clang-tidy checks to be linked in.
+  if (!CLANGD_TIDY_CHECKS)
+    return;
+  Opts.ClangTidyProvider = [](tidy::ClangTidyOptions &ClangTidyOpts,
+                              llvm::StringRef) {
+    ClangTidyOpts.Checks = {"-*,boost-use-ranges"};
+  };
+  // Check that registering the boost-use-ranges checker's matchers
+  // on two different threads does not cause a crash.
+  auto &Client = start();
+  Client.didOpen("a.cpp", "");
+  Client.didOpen("b.cpp", "");
+  Client.sync();
+}
+
 TEST_F(LSPTest, IncomingCalls) {
   Annotations Code(R"cpp(
     void calle^e(int);

@llvmbot
Copy link
Member Author

llvmbot commented Oct 6, 2024

@llvm/pr-subscribers-clangd

Author: None (llvmbot)

Changes

Backport acf92a4

Requested by: @HighCommander4


Full diff: https://github.com/llvm/llvm-project/pull/111318.diff

2 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp (+9-9)
  • (modified) clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp (+16)
diff --git a/clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp
index 4022ea0cdaf5ee..e45687fde6d9f6 100644
--- a/clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp
@@ -204,7 +204,7 @@ utils::UseRangesCheck::ReplacerMap UseRangesCheck::getReplacerMap() const {
   ReplacerMap Results;
   static const Signature SingleSig = {{0}};
   static const Signature TwoSig = {{0}, {2}};
-  static const auto AddFrom =
+  const auto AddFrom =
       [&Results](llvm::IntrusiveRefCntPtr<UseRangesCheck::Replacer> Replacer,
                  std::initializer_list<StringRef> Names, StringRef Prefix) {
         llvm::SmallString<64> Buffer;
@@ -214,17 +214,17 @@ utils::UseRangesCheck::ReplacerMap UseRangesCheck::getReplacerMap() const {
         }
       };
 
-  static const auto AddFromStd =
-      [](llvm::IntrusiveRefCntPtr<UseRangesCheck::Replacer> Replacer,
-         std::initializer_list<StringRef> Names) {
+  const auto AddFromStd =
+      [&](llvm::IntrusiveRefCntPtr<UseRangesCheck::Replacer> Replacer,
+          std::initializer_list<StringRef> Names) {
         AddFrom(Replacer, Names, "std");
       };
 
-  static const auto AddFromBoost =
-      [](llvm::IntrusiveRefCntPtr<UseRangesCheck::Replacer> Replacer,
-         std::initializer_list<
-             std::pair<StringRef, std::initializer_list<StringRef>>>
-             NamespaceAndNames) {
+  const auto AddFromBoost =
+      [&](llvm::IntrusiveRefCntPtr<UseRangesCheck::Replacer> Replacer,
+          std::initializer_list<
+              std::pair<StringRef, std::initializer_list<StringRef>>>
+              NamespaceAndNames) {
         for (auto [Namespace, Names] : NamespaceAndNames)
           AddFrom(Replacer, Names,
                   SmallString<64>{"boost", (Namespace.empty() ? "" : "::"),
diff --git a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
index 75a140767035b2..49a94045ea4878 100644
--- a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -262,6 +262,22 @@ TEST_F(LSPTest, ClangTidyRename) {
   EXPECT_EQ(Params, std::vector{llvm::json::Value(std::move(ExpectedEdit))});
 }
 
+TEST_F(LSPTest, ClangTidyCrash_Issue109367) {
+  // This test requires clang-tidy checks to be linked in.
+  if (!CLANGD_TIDY_CHECKS)
+    return;
+  Opts.ClangTidyProvider = [](tidy::ClangTidyOptions &ClangTidyOpts,
+                              llvm::StringRef) {
+    ClangTidyOpts.Checks = {"-*,boost-use-ranges"};
+  };
+  // Check that registering the boost-use-ranges checker's matchers
+  // on two different threads does not cause a crash.
+  auto &Client = start();
+  Client.didOpen("a.cpp", "");
+  Client.didOpen("b.cpp", "");
+  Client.sync();
+}
+
 TEST_F(LSPTest, IncomingCalls) {
   Annotations Code(R"cpp(
     void calle^e(int);

@llvmbot
Copy link
Member Author

llvmbot commented Oct 6, 2024

@llvm/pr-subscribers-clang-tools-extra

Author: None (llvmbot)

Changes

Backport acf92a4

Requested by: @HighCommander4


Full diff: https://github.com/llvm/llvm-project/pull/111318.diff

2 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp (+9-9)
  • (modified) clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp (+16)
diff --git a/clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp b/clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp
index 4022ea0cdaf5ee..e45687fde6d9f6 100644
--- a/clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/boost/UseRangesCheck.cpp
@@ -204,7 +204,7 @@ utils::UseRangesCheck::ReplacerMap UseRangesCheck::getReplacerMap() const {
   ReplacerMap Results;
   static const Signature SingleSig = {{0}};
   static const Signature TwoSig = {{0}, {2}};
-  static const auto AddFrom =
+  const auto AddFrom =
       [&Results](llvm::IntrusiveRefCntPtr<UseRangesCheck::Replacer> Replacer,
                  std::initializer_list<StringRef> Names, StringRef Prefix) {
         llvm::SmallString<64> Buffer;
@@ -214,17 +214,17 @@ utils::UseRangesCheck::ReplacerMap UseRangesCheck::getReplacerMap() const {
         }
       };
 
-  static const auto AddFromStd =
-      [](llvm::IntrusiveRefCntPtr<UseRangesCheck::Replacer> Replacer,
-         std::initializer_list<StringRef> Names) {
+  const auto AddFromStd =
+      [&](llvm::IntrusiveRefCntPtr<UseRangesCheck::Replacer> Replacer,
+          std::initializer_list<StringRef> Names) {
         AddFrom(Replacer, Names, "std");
       };
 
-  static const auto AddFromBoost =
-      [](llvm::IntrusiveRefCntPtr<UseRangesCheck::Replacer> Replacer,
-         std::initializer_list<
-             std::pair<StringRef, std::initializer_list<StringRef>>>
-             NamespaceAndNames) {
+  const auto AddFromBoost =
+      [&](llvm::IntrusiveRefCntPtr<UseRangesCheck::Replacer> Replacer,
+          std::initializer_list<
+              std::pair<StringRef, std::initializer_list<StringRef>>>
+              NamespaceAndNames) {
         for (auto [Namespace, Names] : NamespaceAndNames)
           AddFrom(Replacer, Names,
                   SmallString<64>{"boost", (Namespace.empty() ? "" : "::"),
diff --git a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
index 75a140767035b2..49a94045ea4878 100644
--- a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -262,6 +262,22 @@ TEST_F(LSPTest, ClangTidyRename) {
   EXPECT_EQ(Params, std::vector{llvm::json::Value(std::move(ExpectedEdit))});
 }
 
+TEST_F(LSPTest, ClangTidyCrash_Issue109367) {
+  // This test requires clang-tidy checks to be linked in.
+  if (!CLANGD_TIDY_CHECKS)
+    return;
+  Opts.ClangTidyProvider = [](tidy::ClangTidyOptions &ClangTidyOpts,
+                              llvm::StringRef) {
+    ClangTidyOpts.Checks = {"-*,boost-use-ranges"};
+  };
+  // Check that registering the boost-use-ranges checker's matchers
+  // on two different threads does not cause a crash.
+  auto &Client = start();
+  Client.didOpen("a.cpp", "");
+  Client.didOpen("b.cpp", "");
+  Client.sync();
+}
+
 TEST_F(LSPTest, IncomingCalls) {
   Annotations Code(R"cpp(
     void calle^e(int);

@tru tru merged commit 33a5c88 into llvm:release/19.x Oct 11, 2024
4 of 6 checks passed
@github-actions
Copy link

@HighCommander4 (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

4 participants