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

fix: std::memory_order::memory_order_relaxed -> std::memory_order_relaxed #28

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

chenrui333
Copy link
Contributor

@chenrui333 chenrui333 commented Dec 10, 2024

seeing build failure as below while building hsd 7.0.0

  npm error ../deps/leveldb/util/env_posix.cc:817:34: error: no member named 'memory_order_relaxed' in 'std::memory_order'; did you mean 'std::memory_order_relaxed'?
  npm error   817 |     env_initialized_.store(true, std::memory_order::memory_order_relaxed);
  npm error       |                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  npm error       |                                  std::memory_order_relaxed
  npm error /Library/Developer/CommandLineTools/SDKs/MacOSX15.sdk/usr/include/c++/v1/__atomic/memory_order.h:43:23: note: 'std::memory_order_relaxed' declared here
  npm error    43 | inline constexpr auto memory_order_relaxed = memory_order::relaxed;
  npm error       |                       ^
  npm error ../deps/leveldb/util/env_posix.cc:834:35: error: no member named 'memory_order_relaxed' in 'std::memory_order'; did you mean 'std::memory_order_relaxed'?
  npm error   834 |     assert(!env_initialized_.load(std::memory_order::memory_order_relaxed));
  npm error       |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  npm error       |                                   std::memory_order_relaxed
  npm error /Library/Developer/CommandLineTools/SDKs/MacOSX15.sdk/usr/include/assert.h:75:25: note: expanded from macro 'assert'
  npm error    75 |     (__builtin_expect(!(e), 0) ? __assert_rtn(__func__, __ASSERT_FILE_NAME, __LINE__, #e) : (void)0)
  npm error       |                         ^
  npm error /Library/Developer/CommandLineTools/SDKs/MacOSX15.sdk/usr/include/c++/v1/__atomic/memory_order.h:43:23: note: 'std::memory_order_relaxed' declared here
  npm error    43 | inline constexpr auto memory_order_relaxed = memory_order::relaxed;
  npm error       |                       ^
  npm error 2 errors generated.

This is due to std::memory_order::memory_order_relaxed is removed since C++ 20: https://en.cppreference.com/w/cpp/atomic/memory_order.

upstream has already patched as google/leveldb#965

relates to Homebrew/homebrew-core#200615

cc @nodech

…relaxed`

Signed-off-by: Rui Chen <rui@chenrui.dev>
@nodech
Copy link
Member

nodech commented Dec 10, 2024

Yeah, I saw this issue on nodejs v23 (that's when std version bump happens) and decided to not update until nodejs v24. I guess brew targets the v23 nodejs ?

Reason was to fully upgrade leveldb instead of patching this one for the next major version. But I think it makes sense to fix this. Thanks for the PR!

Do you want me to release hsd v7.0.1 ?

I will release bdb patch right away.

@nodech nodech merged commit b136252 into bcoin-org:master Dec 10, 2024
13 checks passed
@chenrui333
Copy link
Contributor Author

Yeah, I saw this issue on nodejs v23 (that's when std version bump happens) and decided to not update until nodejs v24. I guess brew targets the v23 nodejs ?

yes, we are targeting the latest node, which is nodejs23 at this point

Do you want me to release hsd v7.0.1 ?

that would be great, thanks!!

@chenrui333 chenrui333 deleted the cpp20 branch December 10, 2024 13:57
@chenrui333
Copy link
Contributor Author

thanks for bsd new release, works fine for my hsd 7.0.0 release build now.

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

Successfully merging this pull request may close these issues.

2 participants