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

std::memory_order::memory_order_relaxed -> std::memory_order_relaxed #965

Merged
merged 1 commit into from
Jan 8, 2022
Merged

std::memory_order::memory_order_relaxed -> std::memory_order_relaxed #965

merged 1 commit into from
Jan 8, 2022

Conversation

ShawnZhong
Copy link
Contributor

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

Running cmake -DCMAKE_CXX_STANDARD=20 .. && cmake --build . gives the following error:

leveldb/util/env_posix.cc:890:35: error: no member named 'memory_order_relaxed' in 'std::memory_order'; did you mean 'std::memory_order_relaxed'?
    assert(!env_initialized_.load(std::memory_order::memory_order_relaxed));
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                  std::memory_order_relaxed

This PR fixed this issue.

@pwnall pwnall self-requested a review January 8, 2022 19:35
Copy link
Member

@pwnall pwnall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this contribution!

This PR will be merged by a bot after it makes it way through our internal repositories.

I applied the same changes to env_windows.cc and verified that tests still pass in https://github.com/pwnall/leveldb/runs/4749567817. I changed the commit description to a summary of the PR description.

Please don't change your branch -- the bot will merge the commit approved here.

@pwnall pwnall merged commit e4ccaa0 into google:master Jan 8, 2022
@ShawnZhong ShawnZhong deleted the cpp20 branch January 8, 2022 20:57
fanquake added a commit to bitcoin-core/leveldb-subtree that referenced this pull request Mar 2, 2022
93ee8d8 Merge pull request google#965 from ShawnZhong:cpp20 (Victor Costan)

Pull request description:

  This is cherry-picked from upstream and can be dropped when upstream is fully synced to include the commit.

  For now this is needed for two reasons:

  * Consistently use the same symbol names in the whole project.
  * Compiling with C++20 fails.

ACKs for top commit:
  achow101:
    ACK 93ee8d8

Tree-SHA512: 3ab48f003cd01ff9dce2da8d68085fa5ff100c3e6de75a0a4b4dd1b669d78577c85d7663906441a410d8c96965fc059a574ff6ef9b2741f9a49bd31d65acb896
@mikehardy
Copy link

mikehardy commented Oct 24, 2024

@pwnall and maybe @a-sully - many apologies for the direct tag, but this was never released. It is currently causing build failures for us over in react-native-firebase (which wraps Google Firebase) because in a react-native context they have started defining NDEBUG during the build, which opens these non-C++-conformant lines to the compiler and they fail to compile

Is there any way we could get a leveldb 1.24 release with this fix in it 🙏 ?

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.

4 participants