Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

Backport C++17 type_traits and cuda::std::byte to C++14 #44

Closed
wants to merge 16 commits into from

Conversation

jrhemstad
Copy link
Collaborator

@jrhemstad jrhemstad commented Oct 9, 2020

Broken off from #10

  • Backports <type_traits> features from C++17 to make them available in C++14

  • Backports tests of type traits to make them supported in C++14

  • Backports cuda::std::byte to be available in C++14 (this was necessary to make some tests pass as a result of the <type_traits> changes

@jrhemstad
Copy link
Collaborator Author

jrhemstad commented Oct 9, 2020

@griwes are there <type_traits> tests I'm missing that should also be back ported?

Found them in std/utilities/meta.

Copy link
Collaborator

@griwes griwes left a comment

Choose a reason for hiding this comment

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

Looking at the diff made me realize that you're also enabling the variable templates for the traits, but not the tests around the variable templates, like here: https://github.com/NVIDIA/libcudacxx/blob/main/.upstream-tests/test/std/utilities/meta/meta.unary/meta.unary.prop/is_abstract.pass.cpp#L24-L29. I'll leave it up to you whether you want to enable those in this PR or if you want to do that in a future one to help you folks move to this repo in RAPIDS, since those tests will quite obviously pass.

@jrhemstad
Copy link
Collaborator Author

Looking at the diff made me realize that you're also enabling the variable templates for the traits, but not the tests around the variable templates, like here: https://github.com/NVIDIA/libcudacxx/blob/main/.upstream-tests/test/std/utilities/meta/meta.unary/meta.unary.prop/is_abstract.pass.cpp#L24-L29.

Thanks for noticing that. I didn't see the guards in the tests themselves. I updated them all to test in C++14 and they all passed.

@griwes
Copy link
Collaborator

griwes commented Oct 28, 2020

Kicked off internal CI at CL 29255931.

@griwes
Copy link
Collaborator

griwes commented Oct 29, 2020

It looks like MSVC doesn't accept __is_aggregate in its C++14 mode. You'll need to make its definition go away there. (It seems to me that guarding the definition of _LIBCUDACXX_IS_AGGREGATE in libcxx/include/__config with a language version check should be enough, but I have no Windows dev environment set up to confirm.)

@brycelelbach brycelelbach added this to the 1.4.0 milestone Oct 29, 2020
@jrhemstad
Copy link
Collaborator Author

It looks like MSVC doesn't accept __is_aggregate in its C++14 mode. You'll need to make its definition go away there. (It seems to me that guarding the definition of _LIBCUDACXX_IS_AGGREGATE in libcxx/include/__config with a language version check should be enough, but I have no Windows dev environment set up to confirm.)

I added a MSVC version check for >19.14.

@griwes
Copy link
Collaborator

griwes commented Oct 29, 2020

Re-running internal CI, CL 29261259.

@griwes
Copy link
Collaborator

griwes commented Oct 30, 2020

std/language.support/support.types/byte.pass.cpp passes unexpectedly on GCC in C++14 mode which causes a fail. Other than that it looks like we are good to go. Can you fix its keywords and remove the merge commit that's still in this PR? I'll merge it then.

@jrhemstad jrhemstad force-pushed the type-traits-backport branch from 7bc8681 to 30cdf58 Compare October 30, 2020 21:11
@griwes
Copy link
Collaborator

griwes commented Oct 30, 2020

Clarification: it's the libc++ version of the test that's XPASSing, in libcxx/test, not the libcu++ one.

@jrhemstad jrhemstad force-pushed the type-traits-backport branch from 30cdf58 to 2f212ba Compare October 30, 2020 21:16
I backported std::byte to work in C++14. Because this
test used XFAIL, the test now unexpectedly passes
instead of fails. I updated to use UNSUPPORTED
instead to avoid an unexpected pass.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants