Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build fails on macOS 10.15: ThreadLocal.h: error: no matching constructor for initialization of 'std::function<LocalRefCount ()>' #2266

Open
barracuda156 opened this issue Jul 25, 2024 · 4 comments

Comments

@barracuda156
Copy link

In file included from /opt/local/var/macports/build/_opt_CatalinaPorts_devel_folly/folly/work/folly-v2024.07.22.00/folly/Singleton.cpp:17:
In file included from /opt/local/var/macports/build/_opt_CatalinaPorts_devel_folly/folly/work/folly-v2024.07.22.00/folly/Singleton.h:134:
In file included from /opt/local/var/macports/build/_opt_CatalinaPorts_devel_folly/folly/work/folly-v2024.07.22.00/folly/concurrency/CoreCachedSharedPtr.h:27:
In file included from /opt/local/var/macports/build/_opt_CatalinaPorts_devel_folly/folly/work/folly-v2024.07.22.00/folly/synchronization/Hazptr.h:20:
In file included from /opt/local/var/macports/build/_opt_CatalinaPorts_devel_folly/folly/work/folly-v2024.07.22.00/folly/synchronization/HazptrDomain.h:29:
In file included from /opt/local/var/macports/build/_opt_CatalinaPorts_devel_folly/folly/work/folly-v2024.07.22.00/folly/synchronization/HazptrThrLocal.h:27:
In file included from /opt/local/var/macports/build/_opt_CatalinaPorts_devel_folly/folly/work/folly-v2024.07.22.00/folly/SingletonThreadLocal.h:25:
/opt/local/var/macports/build/_opt_CatalinaPorts_devel_folly/folly/work/folly-v2024.07.22.00/folly/ThreadLocal.h:74:9: error: no matching constructor for initialization of 'std::function<LocalRefCount ()>'
   74 |       : constructor_(std::forward<F>(constructor)) {}
      |         ^            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/local/var/macports/build/_opt_CatalinaPorts_devel_folly/folly/work/folly-v2024.07.22.00/folly/concurrency/memory/TLRefCount.h:30:9: note: in instantiation of function template specialization 'folly::ThreadLocal<folly::TLRefCount::LocalRefCount, folly::TLRefCount>::ThreadLocal<(lambda at /opt/local/var/macports/build/_opt_CatalinaPorts_devel_folly/folly/work/folly-v2024.07.22.00/folly/concurrency/memory/TLRefCount.h:30:21), 0>' requested here
   30 |       : localCount_([&]() { return LocalRefCount(*this); }),
      |         ^
/opt/local/include/libcxx/v1/functional:2366:5: note: candidate constructor not viable: no known conversion from '(lambda at /opt/local/var/macports/build/_opt_CatalinaPorts_devel_folly/folly/work/folly-v2024.07.22.00/folly/concurrency/memory/TLRefCount.h:30:21)' to 'nullptr_t' (aka 'std::nullptr_t') for 1st argument
 2366 |     function(nullptr_t) _NOEXCEPT {}
      |     ^        ~~~~~~~~~
/opt/local/include/libcxx/v1/functional:2367:5: note: candidate constructor not viable: no known conversion from '(lambda at /opt/local/var/macports/build/_opt_CatalinaPorts_devel_folly/folly/work/folly-v2024.07.22.00/folly/concurrency/memory/TLRefCount.h:30:21)' to 'const function<LocalRefCount ()>' for 1st argument
 2367 |     function(const function&);
      |     ^        ~~~~~~~~~~~~~~~
/opt/local/include/libcxx/v1/functional:2368:5: note: candidate constructor not viable: no known conversion from '(lambda at /opt/local/var/macports/build/_opt_CatalinaPorts_devel_folly/folly/work/folly-v2024.07.22.00/folly/concurrency/memory/TLRefCount.h:30:21)' to 'function<LocalRefCount ()>' for 1st argument
 2368 |     function(function&&) _NOEXCEPT;
      |     ^        ~~~~~~~~~~
/opt/local/include/libcxx/v1/functional:2370:5: note: candidate template ignored: requirement '__callable<(lambda at /opt/local/var/macports/build/_opt_CatalinaPorts_devel_folly/folly/work/folly-v2024.07.22.00/folly/concurrency/memory/TLRefCount.h:30:21) &, true>::value' was not satisfied [with _Fp = (lambda at /opt/local/var/macports/build/_opt_CatalinaPorts_devel_folly/folly/work/folly-v2024.07.22.00/folly/concurrency/memory/TLRefCount.h:30:21)]
 2370 |     function(_Fp);
      |     ^
/opt/local/include/libcxx/v1/functional:2364:5: note: candidate constructor not viable: requires 0 arguments, but 1 was provided
 2364 |     function() _NOEXCEPT { }
      |     ^
1 error generated.
@barracuda156
Copy link
Author

@yfeldblum Could you please take a look? This commit has broken the build: 99767aa

(Apparently the breakage is specific to libc++ or Clang. I did not need to revert this commit when building with gcc on 10.6.)

P. S. There is another unfixed bug, but it shows up later in the build: #2124

@barracuda156
Copy link
Author

I can confirm that reverting the said commit (with minor adjustment to the code changes) plus the fix from my PR fixes the build on Catalina.

From ea538d3975c94c5d0a3d2eb2b0ab9f98fa182089 Mon Sep 17 00:00:00 2001
From: Sergey Fedorov <[email protected]>
Date: Thu, 25 Jul 2024 11:10:58 +0800
Subject: [PATCH] Revert breaking commit re threadlocal

---
 folly/ThreadLocal.h                   | 8 ++++----
 folly/concurrency/memory/TLRefCount.h | 2 +-
 folly/settings/detail/SettingsImpl.h  | 2 +-
 folly/test/ThreadLocalTest.cpp        | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git folly/ThreadLocal.h folly/ThreadLocal.h
index 114b30f20..a610d3d4f 100644
--- folly/ThreadLocal.h
+++ folly/ThreadLocal.h
@@ -67,9 +67,9 @@ class ThreadLocalPtr;
 template <class T, class Tag = void, class AccessMode = void>
 class ThreadLocal {
  public:
-  constexpr ThreadLocal() : constructor_([]() { return T(); }) {}
+  constexpr ThreadLocal() : constructor_([]() { return new T(); }) {}
 
-  template <typename F, std::enable_if_t<is_invocable_r_v<T, F>, int> = 0>
+  template <typename F, std::enable_if_t<is_invocable_r_v<T*, F>, int> = 0>
   explicit ThreadLocal(F&& constructor)
       : constructor_(std::forward<F>(constructor)) {}
 
@@ -100,13 +100,13 @@ class ThreadLocal {
   ThreadLocal& operator=(const ThreadLocal&) = delete;
 
   FOLLY_NOINLINE T* makeTlp() const {
-    auto const ptr = new T(constructor_());
+    auto const ptr = static_cast<T*>(constructor_());
     tlp_.reset(ptr);
     return ptr;
   }
 
   mutable ThreadLocalPtr<T, Tag, AccessMode> tlp_;
-  std::function<T()> constructor_;
+  std::function<void*()> constructor_;
 };
 
 /*
diff --git folly/concurrency/memory/TLRefCount.h folly/concurrency/memory/TLRefCount.h
index 88ecb2666..f0c24c72a 100644
--- folly/concurrency/memory/TLRefCount.h
+++ folly/concurrency/memory/TLRefCount.h
@@ -27,7 +27,7 @@ class TLRefCount {
   using Int = int64_t;
 
   TLRefCount()
-      : localCount_([&]() { return LocalRefCount(*this); }),
+      : localCount_([&]() { return new LocalRefCount(*this); }),
         collectGuard_(this, [](void*) {}) {}
 
   ~TLRefCount() noexcept {
diff --git folly/settings/detail/SettingsImpl.h folly/settings/detail/SettingsImpl.h
index 42ddb3048..740dfe963 100644
--- folly/settings/detail/SettingsImpl.h
+++ folly/settings/detail/SettingsImpl.h
@@ -387,7 +387,7 @@ class SettingCore : public SettingCoreBase {
         defaultValue_(std::move(defaultValue)),
         trivialStorage_(trivialStorage),
         localValue_([]() {
-          return cacheline_aligned<
+          return new cacheline_aligned<
               std::pair<Version, std::shared_ptr<Contents>>>(
               std::in_place, 0, nullptr);
         }) {
diff --git folly/test/ThreadLocalTest.cpp folly/test/ThreadLocalTest.cpp
index f9482538d..4fa1acd1f 100644
--- folly/test/ThreadLocalTest.cpp
+++ folly/test/ThreadLocalTest.cpp
@@ -261,7 +261,7 @@ TEST(ThreadLocal, NotDefaultConstructible) {
     explicit Object(int v) : value{v} {}
   };
   std::atomic<int> a{};
-  ThreadLocal<Object> o{[&a] { return Object(a++); }};
+  ThreadLocal<Object> o{[&a] { return new Object(a++); }};
   EXPECT_EQ(0, o->value);
   std::thread([&] { EXPECT_EQ(1, o->value); }).join();
 }
svacchanda@Sergeys-Mini ~ % port -v installed folly
The following ports are currently installed:
  folly @2024.07.22.00_0 (active) requested_variants='' platform='darwin 19' archs='x86_64' date='2024-07-25T11:34:22+0800'

But perhaps there can be a proper fix rather than a revert?

@yfeldblum
Copy link
Contributor

We have migrated to C++17, and this uses the new C++17 language feature of syntactic/mandatory copy elision.

Is the compiler here not using C++17?

@barracuda156
Copy link
Author

We have migrated to C++17, and this uses the new C++17 language feature of syntactic/mandatory copy elision.

Is the compiler here not using C++17?

@yfeldblum I am pretty sure we do not force any C++ standard against what is set by the build system. But I need to check what compiler is picked on 10.15.

gcc 14 builds this fine as-is, but it defaults to C++17 even without flags being passed explicitly.

However why they aren’t? CMake has a feature to enforce C++ standard; so, for example, if MacPorts on Catalina uses a compiler which cannot handle C++17, build should fail already at configure, not during compilation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants