-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Update MSRV to 1.85.0 #10785
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
Update MSRV to 1.85.0 #10785
Conversation
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.
👍 but I guess there's a few extra warnings to handle now.
This makes us eligible to update to the 2024 Rust edition but I'm planning on deferring that to a separate follow-up change. prtest:full
@@ -114,7 +114,7 @@ impl TestFileCompiler { | |||
unsafe extern "C" { | |||
safe fn cosf(f: f32) -> f32; | |||
} | |||
let f = 1.2_f32; | |||
let f = std::hint::black_box(1.2_f32); |
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.
Digging into this, it looks like LLVM sees in the assertion below assert_eq!(cosf(x), cosf(x))
and then LLVM has built-in knowledge that cosf
is pure so it's optimized out entirely. That means libm doesn't get linked at all, meaning later dlsym
calls fail.
Using black_box
here is apparently enough, but unsure how long it'll last necessarily.
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.
For future posterity this is roughly what I was looking at. Basically we just need to preserve the reference to the cosf
symbol. (and if anyone knows of a better way to do that let's definitely land that instead!)
Now possible after bytecodealliance#10785
Now possible after #10785
This makes us eligible to update to the 2024 Rust edition but I'm planning on deferring that to a separate follow-up change.