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

Traits.h: Use std::is_trivially_relocatable if P1144 is available #2216

Closed

Conversation

Quuxplusone
Copy link
Contributor

Well, llvm/llvm-project#84621 isn't going anywhere in Clang, therefore #2159 will presumably never be mergeable (in the foreseeable future, anyway).

So here's an alternative approach, for your consideration: If P1144's feature-test macro is set, then use std::is_trivially_relocatable<T> in place of std::is_trivially_copyable<T>.
This will produce improved codegen for e.g. fbvector<std::vector<int>>::erase, when compiled on a compiler supporting P1144 semantics. https://godbolt.org/z/s6sa313h3

Compare https://github.com/charles-salvia/std_error/blob/3abc272/all_in_one.hpp#L180-L196

@facebook-github-bot
Copy link
Contributor

@Orvid has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@Orvid merged this pull request in 9327b2a.

facebook-github-bot pushed a commit to facebook/hhvm that referenced this pull request Jun 17, 2024
Summary:
Well, llvm/llvm-project#84621 isn't going anywhere in Clang, therefore facebook/folly#2159 will presumably never be mergeable (in the foreseeable future, anyway).

So here's an alternative approach, for your consideration: If [P1144's feature-test macro](https://open-std.org/jtc1/sc22/wg21/docs/papers/2024/p1144r10.html#wording) is set, then use `std::is_trivially_relocatable<T>` in place of `std::is_trivially_copyable<T>`.
This will produce improved codegen for e.g. `fbvector<std::vector<int>>::erase`, when compiled on a compiler supporting P1144 semantics. https://godbolt.org/z/s6sa313h3

Compare https://github.com/charles-salvia/std_error/blob/3abc272/all_in_one.hpp#L180-L196

X-link: facebook/folly#2216

Reviewed By: Gownta

Differential Revision: D58147899

Pulled By: Orvid

fbshipit-source-id: 671ed21eeb6080f583e6966e776d243c99c9c17f
Quuxplusone added a commit to Quuxplusone/folly that referenced this pull request Oct 12, 2024
Well, this is awkward. facebook#2216 added this `#if` check, but with the
wrong macro: `__cpp_lib_is_trivially_relocatable` should have been
`__cpp_lib_trivially_relocatable`.
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