Closed kocsis1david closed 2 years ago
Bisecting points to https://github.com/rust-lang/rust/pull/81172 as the culprit. cc @SimonSapin
searched nightlies: from nightly-2021-02-14 to nightly-2021-05-03 regressed nightly: nightly-2021-02-19 searched commits: from https://github.com/rust-lang/rust/commit/152f6609246558be5e2582e67376194815e6ba0d to https://github.com/rust-lang/rust/commit/0148b971c921a0831fbf3357e5936eec724e3566 regressed commit: https://github.com/rust-lang/rust/commit/d1462d8558cf4551608457f63d9b999185ebf3bf
Reducing a bit more:
#![crate_type = "lib"]
use std::{collections::HashMap, hash::Hash};
struct C;
trait Ctx {
type BindGroupId;
}
impl Ctx for C {
type BindGroupId = u32;
}
pub struct BindGroup {
_id: <C as Ctx>::BindGroupId,
}
pub fn insert(map: &mut HashMap<*const BindGroup, u32>, bind_group: *const BindGroup) {
map.insert(bind_group, 1);
}
However, if I remove insert
and replace the HashMap
with only calling hash()
I don’t reproduce the ICE anymore:
pub fn hash(bind_group: *const BindGroup, state: &mut impl std::hash::Hasher) {
bind_group.hash(state)
}
The panic message points to the second unwrap
in:
… which means that rustc_middle::ty::instance::Instance::resolve
returns Ok(None)
.
Because https://github.com/rust-lang/rust/pull/81172 and a HashMap
are involved, I suspected that it might be <*const BindGroup as Hash>::hash
that fails to be resolved to a concrete function because something fails to resolve <BindGroup as Pointee>::Metadata
through the <C as Ctx>::BindGroupId
associated type. https://github.com/rust-lang/rust/pull/81172/commits/696b239f72350ce2a647ede1a330039d0e0ecfa9#diff-4864ab6ed9e5cec3c6d0d561211554aafbd0094842869d2a711b5451151bc534R2139 is the part of #81172 relevant to that.
However my failed reduction says that might not be it. I’m not sure what to try next. Maybe someone more familiar with compiler internals can comment on how Instance::resolve
returning Ok(None)
.
Nominating for discussion during T-compiler meeting to get more eyes on this (cc: @SimonSapin feel free to jump in)
Some more reduced code examples can be found in this related/duplicate issue. #86469
There’s also a case of hitting a slightly different ICE
error: internal compiler error: compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs:704:14: debuginfo: unexpected type in type_metadata: <Struct as std::ptr::Pointee>::Metadata
thread 'rustc' panicked at 'Box<dyn Any>', compiler/rustc_errors/src/lib.rs:1007:9
here: https://github.com/rust-lang/rust/issues/86469#issuecomment-864674629
Ouch, I forgot about this and the regression has reached Stable :/ However I still don’t know how to proceed as I’m not at all familiar with these compiler internals. Any help would be appreciated.
The slightly different ICE points to:
I think this file is about some other kind of metadata, not https://github.com/rust-lang/rust/issues/81513 pointer metadata? But the underlying issue seems to be similar: some code receives a Ty
that represents an unresolved <Foo as Pointee>::Metadata
associated type projection but expected that to be resolved earlier to a concrete type like ()
or usize
.
Hey @SimonSapin , I self-assigned this and then didn't follow up. I'm looking at it more carefully now.
Looking at the reduction from https://github.com/rust-lang/rust/issues/85447#issuecomment-843879967, I added a bit of instrumentation to the compiler to see which ty
is causing resolution to return Ok(None)
. Answer:
variable | debug output |
---|---|
ty | for<'r, 's> fn(&'r <BindGroup as std::ptr::Pointee>::Metadata, &'s mut std::collections::hash_map::DefaultHasher) {<<BindGroup as std::ptr::Pointee>::Metadata as std::hash::Hash>::hash::<std::collections::hash_map::DefaultHasher>} |
def_id | DefId(2:9096 ~ core[3e18]::hash::Hash::hash) |
substs | [<BindGroup as std::ptr::Pointee>::Metadata, std::collections::hash_map::DefaultHasher] |
If nothing else, this lends evidence to @SimonSapin 's notes above: we're trying to resolve a trait method, Hash::hash
, via a substitution where the Self
type is an associated item.
So my current guess is that we're missing some extra resolution path somewhere to convert the <BindGroup as std::ptr::Pointee>::Metadata
into a more concrete type that can be used for the necessary method resolution.
Update: I mostly added this note because of something that I didn't mention in the text above: The presence of the higher-kinded type for<'r, 's> fn(...)
could be part of the reason that we are unexpectedly encountering an unresolved type here.
Update: I mostly added this note because of something that I didn't mention in the text above: The presence of the higher-kinded type
for<'r, 's> fn(...)
could be part of the reason that we are unexpectedly encountering an unresolved type here.
Hmm, no, this doesn't seem like a sufficient explanation, at least not in this trivialized form. Here's why: I took a whirl at running the same test case on a build immediately prior to PR #81172, and in the debug output from that run, I see successful visit_fn_use
direct resolution calls on higher-kinded types, including the exact an analogous one of interest here:
DEBUG rustc_mir::monomorphize::collector visit_fn_use resolve direct
ty: for<'r, 's> fn(&'r BindGroupRef, &'s mut std::collections::hash_map::DefaultHasher) {<BindGroupRef as std::hash::Hash>::hash::<std::collections::hash_map::DefaultHasher>}
= def_id: DefId(2:7145 ~ core[1f15]::hash::Hash::hash)
with substs: [BindGroupRef, std::collections::hash_map::DefaultHasher]
Update: Of course the ty is not "the exact" one of interest. The whole point is that after this PR, we are seeing attempts to resolve like:
visit_fn_use resolve direct ty: for<'r, 's> fn(&'r <BindGroup as std::ptr::Pointee>::Metadata, &'s mut std::collections::hash_map::DefaultHasher)
, while before PR #81172, it was
visit_fn_use resolve direct ty: for<'r, 's> fn(&'r BindGroupRef, &'s mut std::collections::hash_map::DefaultHasher)
I'm just restating that the HKT alone isn't the reason for the resolution .unwrap().unwrap() failure. (Its entirely possible that the HKT is an issue somewhere upstream. Or maybe the construction from PR #81172 is just not quite right.)
@pnkfelix and I dug into this. The problem is exactly this FIXME here:
In particular, the BindGroup
struct has a single field, and it is a (normalizable) projection:
pub struct BindGroup {
_id: <C as Ctx>::BindGroupId,
}
The Instance::resolve
code expect that, by the time we get to monomorphization, we are able to resolve all associated types -- however, in this case, we are not, because project.rs
sees the tail field as <C as Ctx>::BindGroupId
, doesn't attempt to normalize, and then decides there is no candidate for projection.
Fixing this is not entirely trivial, I am debating the best approach. The problem is that the "assemble candidates" logic isn't really meant to normalize, which is kind of a side-effecting operation that produces subobligations. But it should be possible to do -- I suppose we could look at what select.rs
does here. I think maybe it uses a "probe" to do the normalization and then throw away the result?
(Egads I would like to convert this code to use chalk!)
The problem is exactly this FIXME here:
When writing this comment as part of https://github.com/rust-lang/rust/pull/81172 I remember feeling very much feeling like the "no idea what I’m doing" meme. I mostly imitated what the compiler already did for the DiscriminantKind
trait, which is also automagically implemented for every type and also has an associated type.
Does DiscriminantKind
not have a similar problem?
I think I can remove the I-nominate
, was discussed on Zulip a while ago, T-compiler followed up on this
@rustbot label -I-nominated
Fixed by #92248
Code
Meta
rustc --version --verbose
:This error also occurs on nightly, that last version that it worked on was 1.51. It seems that this only occurs if the crate-type is lib. Even if I remove a
pub
, it can compile successfully.Error output