rustc: generalize monomorphic_const_eval to polymorphic constants. #41408
Conversation
|
r? @arielb1 (rust_highfive has picked a reviewer for you, use r? to override) |
eddyb
force-pushed
the
poly-const-eval
branch
2 times, most recently
from
ccbe157 to
faf920a
Compare
|
ping @rust-lang/compiler This fixes a regression in nightly, should merge it before the beta. |
src/librustc_const_eval/eval.rs
Outdated
| Some(ic) => lookup_const_by_id(tcx, ic.def_id, Substs::empty()), | ||
| None => default_value, | ||
| Some(ic) => { | ||
| let substs = tcx.lift(&impl_data.substs).unwrap(); |
There was a problem hiding this comment.
doesn't this need to do the drain_fulfillment_cx_or_panic thing? aka be monomorphize::resolve?
There was a problem hiding this comment.
We can't panic here, but I guess, yeah, it should be using something similar to trans.
There was a problem hiding this comment.
There used to be a drain_fulfillment_cx without the panic part function. Can you reintroduce it? We need to not drop nested obligations on the floor, because of associated types in impls:
trait Foo {
const BAR: usize;
}
impl<U, V> Foo for (U, V) where U: Iterator<Item=V>, V: Foo {
const BAR: usize = V::BAR;
}
impl Foo for u32 { const BAR: usize = 4; }
|
|
||
| // Avoid applying substitutions if they're empty, that'd ICE. | ||
| let base_ty = if cx.substs.is_empty() { | ||
| base_ty |
There was a problem hiding this comment.
Empty substitutions are used to signify "polymorphic evaluation". Providing identity substitutions in the right locations is harder.
shepmaster
added
the
S-waiting-on-author
|
r+ modulo the fulfillment_cx problem. |
|
@bors r+ |
|
📌 Commit 60faa01 has been approved by |
|
⌛ Testing commit 60faa01 with merge 582d85c... |
|
💔 Test failed - status-travis |
|
|
@bors r=arielb1 |
|
📌 Commit 8054377 has been approved by |
|
⌛ Testing commit 8054377 with merge ea3233d... |
|
💔 Test failed - status-appveyor |
rustc: generalize monomorphic_const_eval to polymorphic constants. With the addition of `Substs` to the query key, we can now evaluate *and cache* polymorphic constants. Fixes #23898 by replacing the crippled explicit-discriminant-only local-crate-only `lookup_variant_by_id` with `ConstVal::Variant` which can describe variants irrespective of their discriminant. Fixes #41394 by fixing #23898 (for the original testcase) and by not looping past the first discriminant.
|
☀️ Test successful - status-appveyor, status-travis |
With the addition of
Subststo the query key, we can now evaluate and cache polymorphic constants.Fixes #23898 by replacing the crippled explicit-discriminant-only local-crate-only
lookup_variant_by_idwithConstVal::Variantwhich can describe variants irrespective of their discriminant.Fixes #41394 by fixing #23898 (for the original testcase) and by not looping past the first discriminant.