-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
interpret: do not make const-eval query result depend on tcx.sess #129613
Conversation
Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri The Miri subtree was changed cc @rust-lang/miri |
// FIXME: currently we do not detect this UB, since we don't want the result of const-eval | ||
// to depend on `tcx.sess` which can differ between crates in a crate graph. |
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.
How could we fix this? Wouldn't we have to make the const
ABI the same across the entire crate graph? And since we don't have a global coordination, wouldn't we have to ban everything except the minimum target ABI?
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.
We could also just say that this is not UB during const. 🤷 The vendor intrinsics this lets you call are anyway all non-const, and very unlikely to ever be const.
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.
and very unlikely to ever be const.
Are we sure libs/libs-api knows that we expect that?
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.
I don't know... at least I don't recall anyone suggesting we make them const. There's also a lot of them.
Portable SIMD APIs are much more likely to be implemented in const.
After reviewing the play, I think I'm okay with this. I'm not particularly excited about having compiler code that relies on libs not making something const, but I think we have already done a lot of that, and I don't think we've tracked it very well, but this PR doesn't change any of that. r? saethlin |
…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#129421 (add repr to the allowlist for naked functions) - rust-lang#129480 (docs: correct panic conditions for rem_euclid and similar functions) - rust-lang#129551 (ub_checks intrinsics: fall back to cfg(ub_checks)) - rust-lang#129608 (const-eval: do not make UbChecks behavior depend on current crate's flags) - rust-lang#129613 (interpret: do not make const-eval query result depend on tcx.sess) - rust-lang#129641 (rustdoc: fix missing resource suffix on `crates.js`) - rust-lang#129657 (Rename `BikeshedIntrinsicFrom` to `TransmuteFrom`) - rust-lang#129666 (interpret: add missing alignment check in raw_eq) - rust-lang#129667 (Rustc driver cleanup) - rust-lang#129668 (Fix Pin::set bounds regression) - rust-lang#129686 (coverage: Rename `CodeRegion` to `SourceRegion`) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#129421 (add repr to the allowlist for naked functions) - rust-lang#129480 (docs: correct panic conditions for rem_euclid and similar functions) - rust-lang#129551 (ub_checks intrinsics: fall back to cfg(ub_checks)) - rust-lang#129608 (const-eval: do not make UbChecks behavior depend on current crate's flags) - rust-lang#129613 (interpret: do not make const-eval query result depend on tcx.sess) - rust-lang#129641 (rustdoc: fix missing resource suffix on `crates.js`) - rust-lang#129657 (Rename `BikeshedIntrinsicFrom` to `TransmuteFrom`) - rust-lang#129666 (interpret: add missing alignment check in raw_eq) - rust-lang#129667 (Rustc driver cleanup) - rust-lang#129668 (Fix Pin::set bounds regression) - rust-lang#129686 (coverage: Rename `CodeRegion` to `SourceRegion`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#129613 - RalfJung:interpret-target-feat, r=saethlin interpret: do not make const-eval query result depend on tcx.sess The check against calling functions with missing target features uses `tcx.sess` to determine which target features are available. However, this can differ between different crates in a crate graph, so the same const-eval query can come to different conclusions about whether a constant evaluates successfully or not -- which is bad, we should consistently get the same result everywhere.
The check against calling functions with missing target features uses
tcx.sess
to determine which target features are available. However, this can differ between different crates in a crate graph, so the same const-eval query can come to different conclusions about whether a constant evaluates successfully or not -- which is bad, we should consistently get the same result everywhere.