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

C++23 aligned storage #1271

Merged
merged 2 commits into from
Dec 16, 2024
Merged

Conversation

sjoubert
Copy link
Contributor

This removes the usage of std::aligned_storage that is now deprecated in C++23 and replace it with an aligned array of std::byte that is C++17 compliant.

I've also replaced the manual destructor calls with std::destroy_at (replacing placement new with std::construct_at would require C++20 so these are untouched). It is in a separate commit, I can just remove it or make a separate PR if you prefer.

Side note: looking at various examples elsewhere I've seen a lot of usage of std::launder that seem missing here (mostly after each reinterpret_cast). I have no idea how and why it is useful, but you might want to take a look at some point.

Since std::aligned_storage is deprecated in C++23, use `alignas` and an
array of `std::bytes` as a replacement
@sjoubert sjoubert force-pushed the cpp23_aligned_storage branch from 9a55a74 to 7b17e24 Compare December 13, 2024 08:25
Ideally the construction should also use std::construct_at instead of a
placement new but this would require C++20
@sjoubert sjoubert force-pushed the cpp23_aligned_storage branch from 7b17e24 to 6065d6a Compare December 13, 2024 08:30
Copy link

codecov bot commented Dec 13, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 77.42%. Comparing base (ced7d26) to head (6065d6a).
Report is 12 commits behind head on master.

Files with missing lines Patch % Lines
include/pistache/async.h 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1271      +/-   ##
==========================================
+ Coverage   75.72%   77.42%   +1.70%     
==========================================
  Files          59       57       -2     
  Lines        9031     7669    -1362     
==========================================
- Hits         6839     5938     -901     
+ Misses       2192     1731     -461     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sjoubert
Copy link
Contributor Author

  • I've fixed the conventional-commits error on my commits, but I don't know why it insists on checking a commit that is not part of this PR
  • I don't really know how to interpret the abidiff output and if it is actually an issue (given pistache is not at a 1.0 release yet)

@kiplingw
Copy link
Member

Thanks @sjoubert. Looks fine to me. Thank you for those syntactical improvements. If there's no other feedback from my peers by the end of the weekend, remind me to merge if I forget.

@sjoubert
Copy link
Contributor Author

ping @kiplingw

@kiplingw kiplingw merged commit a474d71 into pistacheio:master Dec 16, 2024
132 of 134 checks passed
@sjoubert sjoubert deleted the cpp23_aligned_storage branch December 16, 2024 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants