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
Don't build with stdlib assertions on Linux #33393
Conversation
@swift-ci smoke test |
@swift-ci test centos 7 |
5 similar comments
@swift-ci test centos 7 |
@swift-ci test centos 7 |
@swift-ci test centos 7 |
@swift-ci test centos 7 |
@swift-ci test centos 7 |
@swift-ci test amazon linux 2 |
Build failed |
@swift-ci test amazon linux 2 |
Build failed |
@drexin With this change, it looks like none of the Linux bots test the stdlib with assertions enabled. That disables all the swift reflection tests for ELF. Is there a more targeted option, like disabling stdlib-asserts on a subset of the Linux builders? |
@vedantk what really does not make sense here is that |
@drexin @atrick let me try to explain the problem we're having, and maybe we can come up with a solution together. Basically, we have some runtime functions that return metadata-related information when dealing with Linux (this is due to some constraints in the ELF format). We use these to provide the reflection tests the information they need to run themselves. Because this is used exclusively during testing, and shouldn't do anything on release, these functions have an empty body when assertions are off. So now you see what the problem is from our perspective. Do you have any suggestions on how we can solve this issue? |
Yep, the release build obviously should not enable any runtime verification by default. I'm very unhappy with the build-script defaults, but hopefully this PR fixed your specific problem. |
@atrick the problem is since stdlib's assertions are off, we can't run the reflection tests, since the function functions we added should only run with assertions on. |
Reflection testing would need the build with --stdlib-assertions. There are other stdlib unit tests that also require stdlib asserts enabled. |
Apparently we have been building with stdlib assertions enabled on Linux. A recent change (#33390) caused a massive performance regression on Linux, which brought this to light.