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

Add a compatibility type for C++20's std::type_identity. #14915

Merged
merged 3 commits into from Mar 21, 2023

Conversation

bangerth
Copy link
Member

We have long had a identity type. In C++20, there is finally std::type_identity we can built on instead. Import that into namespace std_cxx20 and use it to define identity.

In a follow-up patch, I will switch all uses of identity<T> to std_cxx20::type_identity<T>. (Or, rather, to std_cxx20::type_identity_t<T>.

Copy link
Member

@masterleinad masterleinad left a comment

Choose a reason for hiding this comment

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

I think we should again use a feature test macro in the C++20 check we run while configuring via CMake.

@bangerth
Copy link
Member Author

@masterleinad Maybe like the last commit here?

I think a good question is what we will do when we actually require C++20. Will we continue to check for the existence of features with these feature test macros? What if they are not satisfied? This is a question separate from the current patch, but one I'd be curious to hear how you think about for the long run.

@masterleinad
Copy link
Member

@masterleinad Maybe like the last commit here?

We discussed some time ago (#10340), that it's not a good idea, in general, to use the feature macros directly in our source code but rather while configuring using CMake.

I think a good question is what we will do when we actually require C++20. Will we continue to check for the existence of features with these feature test macros? What if they are not satisfied? This is a question separate from the current patch, but one I'd be curious to hear how you think about for the long run.

We still have checks for C++14 support and I would think that we will continue to keep those for the oldest version supported. Of course, this also heavily depends on our decision for the oldest compilers we promise to support. If all of those support a particular feature anyway, we don't have to test for it (assuming that newer compilers will not drop support), though.

@bangerth
Copy link
Member Author

Ah, thanks for the reminder. How about this commit?

As for the general question: I see these checks as things that are hard to get rid of. If we still have checks for specific C++14 features, then that's likely not because we need them, but because nobody has removed them. But that is not a discussion we need to have here.

@masterleinad
Copy link
Member

As for the general question: I see these checks as things that are hard to get rid of. If we still have checks for specific C++14 features, then that's likely not because we need them, but because nobody has removed them. But that is not a discussion we need to have here.

I would agree that it's easy to forget to remove them but it's still a backard-comaptible change if we do. 🙂
At least w.r.t the oldest C++ standard supported, these checks are mainly there to make the life of users easier and help them figure out why they can't use a specific compiler.

using type = T;
};

using identity = std_cxx20::type_identity<T>;
Copy link
Member

Choose a reason for hiding this comment

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

We should deprecate this properly when we change all uses in the library.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do in the follow-up!

@bangerth
Copy link
Member Author

OK this way?

@masterleinad masterleinad merged commit 2158b15 into dealii:master Mar 21, 2023
11 checks passed
@bangerth bangerth deleted the cxx20-identity branch March 21, 2023 19:34
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.

None yet

2 participants