-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Warning about using std::counting_semaphore #5595
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
base: develop
Are you sure you want to change the base?
Conversation
Is it worth sticking a pragma message in there for GCC and Clang versions higher than our targets? |
55ece54
to
8064975
Compare
We discussed this and decided that it is not worth it. Most of the deployments today will not be able to take advantage of this optimisation. |
That's fair. I have experience with features waiting on 'eventual' compiler support ( |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #5595 +/- ##
=========================================
- Coverage 78.8% 78.8% -0.0%
=========================================
Files 814 814
Lines 71261 71262 +1
Branches 8368 8369 +1
=========================================
Hits 56131 56131
- Misses 15130 15131 +1
🚀 New features to boost your workflow:
|
src/xrpld/core/detail/semaphore.h
Outdated
* * GCC PR 104928: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104928 | ||
* * LLVM PR 79265: https://github.com/llvm/llvm-project/pull/79265 | ||
* | ||
* Once the minimum compiler version is updated to GCC 13.3 or Clang 17, we can |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that's fixed in gcc 13.3 , given that its release date is May 2024 and the bug was fixed in June 2025 in https://gcc.gnu.org/cgit/gcc/commit/?id=7be4913b8f8b1e1474751656d45b301aa0c20790 . It might have been fixed in gcc 13.4 (released in June 2025) but we would have to verify that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are correct. Checked and updated.
High Level Overview of Change
Comment added.
Context of Change
Type of Change
.gitignore
, formatting, dropping support for older tooling)API Impact
libxrpl
change (any change that may affectlibxrpl
or dependents oflibxrpl
)