-
Notifications
You must be signed in to change notification settings - Fork 10.6k
Fix std::span tests on linux #84774
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
base: main
Are you sure you want to change the base?
Fix std::span tests on linux #84774
Conversation
preset=buildbot_linux |
b7bcd95
to
dd34be4
Compare
preset=buildbot_linux |
@swift-ci please test |
This adds a check in CMake for whether the current C++ stdlib contains the 'span' header. If so, the llvm-lit feature 'std_span' is set. Also adds 'REQUIRES: std_span' to interop tests that include 'span'. This means we no longer have to choose between blanket disabling `std::span` tests on all Linux distributions, or listing every Linux distro with a libstdc++ version without `std::span` support as unsupported. rdar://161999160 rdar://161999174 rdar://162106580 rdar://162106619 rdar://162106643 rdar://162106653 rdar://162106722 rdar://162106747
ca22113
to
edf402d
Compare
@swift-ci please smoke test |
endif() | ||
|
||
include(CheckIncludeFileCXX) | ||
check_include_file_cxx("span" HAVE_STD_SPAN) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@etcwilde @egorzhdan Could you check my logic here - is this approach sound? Or does this need to be checked for every test target? Is CMake aware of multiple different SDKs when testing multiple different targets?
Perhaps I'm overthinking it - I never use multiple test targets, so that feature is a bit outside my wheelhouse. I can't think of a case where we would realistically test against one target without std::span
and one with, but I thought I'd highlight it.
No description provided.