Implement signature type inference#21823
Conversation
Veykril
force-pushed
the
push-zvmkkvrwwppl
branch
6 times, most recently
from
8844613 to
70f825b
Compare
rustbot
added
the
S-waiting-on-review
|
Have to hook this up in analysis_stats still and add some IDE level tests (assuming none cover this yet) but otherwise this should be good to go |
|
This also does not yet hook up inference in variant fields, thats a separate follow up |
|
Oh, and type expectations are missing for generic const argument position |
Veykril
force-pushed
the
push-zvmkkvrwwppl
branch
from
70f825b to
96f12f3
Compare
Veykril
force-pushed
the
push-zvmkkvrwwppl
branch
from
96f12f3 to
b720ba0
Compare
Veykril
force-pushed
the
push-zvmkkvrwwppl
branch
from
b720ba0 to
408301a
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
Veykril
force-pushed
the
push-zvmkkvrwwppl
branch
3 times, most recently
from
a5d80c8 to
d711324
Compare
Veykril
force-pushed
the
push-zvmkkvrwwppl
branch
from
d711324 to
c061236
Compare
| binding_owners, | ||
| block_scopes: block_scopes.into_boxed_slice(), | ||
| ident_hygiene, | ||
| const_expr_origins: const_expr_origins.unwrap_or_default(), |
|
|
||
| fn lower_const_arg_opt(&mut self, arg: Option<ast::ConstArg>) -> ConstRef { | ||
| ConstRef { expr: self.collect_expr_opt(arg.and_then(|it| it.expr())) } | ||
| let const_expr_origins = self.store.const_expr_origins.take(); |
There was a problem hiding this comment.
I think a comment to explain why we take it will be helpful. If I understand correctly, that's because a nested const isn't a root, right? And it will be inferred with its parent.
There was a problem hiding this comment.
Yea, that was my thinking but thats actually wrong. I am planning on changing that to consider nested const args to also be roots but doing so currently breaks some things so that needs to wait for follow up proper const lowering
| )), | ||
| // Path const generic args: determining the expected type requires | ||
| // path resolution. | ||
| // FIXME |
There was a problem hiding this comment.
This is problematic indeed.
The way I see it, we will need to get rid of separate TyLoweringContext and InferenceContext, and instead of having a single place to infer all anon consts, we will need to infer them on demand from the lowered type. I don't know yet whether this will mean a single query for all ty lowering or many queries like now.
How do you expect to solve that?
There was a problem hiding this comment.
My only thought here so far was to infer all anon const with the rest of the body or signature they are in, then fetching the type of the anon const itself will just be a lookup in the main inference context. How to thread the expectations through I have not really thought about yet.
| // the current infer query, except with revealed opaques - is it rare enough to not matter? | ||
| let InternedCoroutine(owner, expr_id) = def_id.0.loc(self.db); | ||
| let body = self.db.body(owner); | ||
| let Some(body_owner) = owner.as_def_with_body() else { |
There was a problem hiding this comment.
This looks incorrect, here and below? Signatures may contain coroutines as well.
There was a problem hiding this comment.
Ah, yea I forgot to look at the remaining as_def_with_body calls
| TyKind::Closure(closure_id, subst) => { | ||
| let owner = db.lookup_intern_closure(closure_id.0).0; | ||
| let infer = InferenceResult::for_body(db, owner); | ||
| let Some(body_owner) = owner.as_def_with_body() else { |
There was a problem hiding this comment.
Here too we should handle signatures.
| TyKind::Closure(id, args) => { | ||
| let def = db.lookup_intern_closure(id.0); | ||
| let infer = InferenceResult::for_body(db, def.0); | ||
| let Some(body_owner) = def.0.as_def_with_body() else { |
3 participants
No description provided.