fix: Lower const params with a bad id #14932
Conversation
rustbot
added
the
S-waiting-on-review
HKalbasi
force-pushed
the
dev
branch
2 times, most recently
from
c0ddcc9
to
e238389
Compare
|
I think before we try to tackle this issue we should take a step back and consider the possible ways to implement it with advantages and disadvantages in mind (maybe as a zulip discussion would make sense). This looks like it will have some architecture implications so it feels like we wanna get this right (or close to right) in an initial implementation. If I recall, this approach here is a different from the one that was mentioned in the issue itself right? |
HKalbasi
force-pushed
the
dev
branch
3 times, most recently
from
bee5565
to
f6b57e1
Compare
| #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] | ||
| pub struct AnonymousConstId(InternId); | ||
| impl_intern_key!(AnonymousConstId); | ||
|
|
||
| #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] | ||
| pub enum TypeOwnerId { | ||
| ModuleId(ModuleId), |
There was a problem hiding this comment.
how can a module be a TypeOwner?
crates/hir-ty/src/infer.rs
Outdated
| @@ -102,6 +102,10 @@ pub(crate) fn infer_query(db: &dyn HirDatabase, def: DefWithBodyId) -> Arc<Infer | |||
| }, | |||
| }); | |||
| } | |||
| DefWithBodyId::InTypeConstId(_) => { | |||
| // FIXME: We should know the expected type here. | |||
There was a problem hiding this comment.
yeah, but this is one of the hard parts, isn't it... IIRC there's a pretty hacky hack for this in rustc
There was a problem hiding this comment.
🤔 My plan was to make a query on TypeOwnerId to lower all types that it owns, and record the types and other data for these consts in the result of that query. But it would be a massive change and can go wrong in multiple ways.
Another approach that just came to my mind is encoding the output type inside InTypeConstId. It is a bit weird but would work with a minimal change and wouldn't affect the stability of id very much.
|
@bors r+ |
|
☀️ Test successful - checks-actions |
lnicola
changed the title
internal: Give ConstBlockId and InTypeConstId named Location types cc #14932
cc #7434
This PR adds an
InTypeConstIdwhich is aDefWithBodyIdand lower const generic parameters into bodies using it, and evaluate them with the mir interpreter. I think this is the last unimplemented const generic feature relative to rustc stable.But there is a problem: The id used in the
InTypeConstIdis the rawFileAstId, which changes frequently. So these ids and their bodies will be invalidated very frequently, which is bad for incremental analysis.Due this problem, I disabled lowering for local crates (in library crate the id is stable since files won't be changed). This might be overreacting (const generic expressions are usually small, maybe it would be better enabled with bad performance than disabled) but it makes motivation for doing it in the correct way, and it splits the potential panic and breakages that usually comes with const generic PRs in two steps.
Other than the id, I think (at least I hope) other parts are in the right direction.