Skip to content

Stop using std::is_trivial #2039

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

Merged
merged 10 commits into from
Mar 18, 2025
Merged

Stop using std::is_trivial #2039

merged 10 commits into from
Mar 18, 2025

Conversation

akukanov
Copy link
Contributor

@akukanov akukanov commented Feb 3, 2025

Since std::is_trivial is deprecated in C++ 26, it makes sense to get rid of it in our code in advance. This is requested in the issue #1961.

In type_traits, we simply inject names into the oneDPL namespace, so the only change there so far is to add a comment.

In glue_memory_impl,h, is_trivial is checked to "optimize" the implementation for trivial types (it is worth checking if that really brings any performance improvement, but let's keep that for later). There we have a few cases:

  • In uninitialized_copy/move, the in-place construction is changed to the "copy" brick, which essentially does an assignment like *result = *input. Therefore, here the check is for the result value type to be trivially default-constructible and trivially assignable from the input reference type.
  • In uninitialized_value_construct, the "optimization" uses the "fill" brick which assigns a given value of the output value type. Therefore the new check is for trivially default-constructible and trivially copyable. The same logic applies to uninitialized_fill, which currently (before the patch) checks for std::is_arithmetic :)
  • In uninitialized_default_construct, construction is fully skipped - so the check should be for trivially default-constructible only.

Update: In a discussion with @rarutyun we come to the conclusion that the triviality of both expected and actual operations should be checked in the condition. The patch is updated accordingly.

@akukanov akukanov added this to the 2022.9.0 milestone Feb 20, 2025
@akukanov akukanov self-assigned this Feb 20, 2025
@akukanov akukanov requested a review from rarutyun March 12, 2025 16:14
The implementation remains semantically correct. Potential optimizations for rvalues "prescribed" in the standard require more code changes.
Copy link
Contributor

@rarutyun rarutyun left a comment

Choose a reason for hiding this comment

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

Looks good to me assuming the clang-format makes things worse and we want to ignore it for this patch

@akukanov
Copy link
Contributor Author

Thanks.
In my opinion, soothing clang-format would make things slightly worse; this is one kind of subjective formatting preferences which are tolerable either way. So, if someone insists, I will "fix" it, otherwise will leave as is.

@akukanov akukanov merged commit cc7aba2 into main Mar 18, 2025
18 of 19 checks passed
@akukanov akukanov deleted the dev/stop-using-is-trivial branch March 18, 2025 23:13
timmiesmith pushed a commit that referenced this pull request Jun 9, 2025
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