Inherent associated types: Better type inference #108430
Conversation
rustbot
added
S-waiting-on-review
fmease
force-pushed
the
iat-better-infer
branch
from
2367d7c
to
1789bb3
Compare
| @@ -0,0 +1,16 @@ | |||
| // check-pass | |||
There was a problem hiding this comment.
Not sure if this test is still useful since it's kind of superseded by inference.rs. I've solely kept it to explicitly demonstrate that bugs/ice-substitution.rs is fixed in this PR.
This comment has been minimized.
This comment has been minimized.
fmease
force-pushed
the
iat-better-infer
branch
from
1789bb3
to
4a45d9d
Compare
| } | ||
| } | ||
| } else { | ||
| assert!(!probe_impl_substs.needs_infer()); |
There was a problem hiding this comment.
Not sure if this holds true in general.
fmease
force-pushed
the
iat-better-infer
branch
from
4a45d9d
to
7a19ab7
Compare
There was a problem hiding this comment.
I don't think this is the right approach.
Instead, I think we need to emit something like a ty::Alias(ty::Inherent, ..) instead of eagerly normalizing and registering bounds within astconv. Adding all this machinery to astconv, then having to rip it out later seems like a lot of work for nothing.
More abstractly, I would like to see more design work on this feature rather than incremental tweaks to behavior like this that still leave bigger questions unanswered.
| @@ -516,6 +516,10 @@ impl<'tcx> AstConv<'tcx> for ItemCtxt<'tcx> { | |||
| // There's no place to record types from signatures? | |||
| } | |||
|
|
|||
| fn register_predicate_obligations(&self, _: PredicateObligations<'tcx>) { | |||
| // There's no place to track this, so just let it go. | |||
There was a problem hiding this comment.
This isn't right. We should at least ICE here.
| // FIXME(inherent_associated_types): To fully *confirm* the *probed* candidate, we still | ||
| // need to relate the Self-type with fresh item substs & register region obligations for | ||
| // regionck to prove/disprove. | ||
| // FIXME(fmease, inherent_associated_types): Register WF obligations for the Self type. |
There was a problem hiding this comment.
This would be fixed by representing it as a projection, so we see the Self type during wfcheck.
There was a problem hiding this comment.
I do plan on implementing something akin to InherentProjection at least experimentally as we've already discussed here. Since I haven't looked into it that much yet, I wasn't sure if this covers wf-checking and inference for the Self type or just for the parameters & bounds of an inherent GAT.
|
Closing in favor of a more principled approach |
…compiler-errors Introduce `AliasKind::Inherent` for inherent associated types Allows us to check (possibly generic) inherent associated types for well-formedness. Type inference now also works properly. Follow-up to rust-lang#105961. Supersedes rust-lang#108430. Fixes rust-lang#106722. Fixes rust-lang#108957. Fixes rust-lang#109768. Fixes rust-lang#109789. Fixes rust-lang#109790. ~Not to be merged before rust-lang#108860 (`AliasKind::Weak`).~ CC `@jackh726` r? `@compiler-errors` `@rustbot` label T-types F-inherent_associated_types
…compiler-errors Introduce `AliasKind::Inherent` for inherent associated types Allows us to check (possibly generic) inherent associated types for well-formedness. Type inference now also works properly. Follow-up to rust-lang#105961. Supersedes rust-lang#108430. Fixes rust-lang#106722. Fixes rust-lang#108957. Fixes rust-lang#109768. Fixes rust-lang#109789. Fixes rust-lang#109790. ~Not to be merged before rust-lang#108860 (`AliasKind::Weak`).~ CC `@jackh726` r? `@compiler-errors` `@rustbot` label T-types F-inherent_associated_types
Follow-up to #105961.
If we are in a
FnCtxt, use the providedInferCtxtto re-relate the Self type of the applicable IAT candidate with the one of the impl to constrain and possibly subsequently solve any inference variables in the Self type. Enables type inference for IATs in bodies of functions, closures, and consts.CC @aliemjay: I've reintroduced
normalizetoAstConvand you probably have an opinion about this.@rustbot label F-inherent_associated_types
r? @jackh726 (feel free to reassign though)