-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix emit_inference_failure_err
ICE
#98610
Conversation
Some changes occured in need_type_info.rs cc @lcnr |
(rust-highfive has picked a reviewer for you, use r? to override) |
error[E0282]: type annotations needed | ||
--> $DIR/expr-struct-type-relative-gat.rs:17:9 | ||
| | ||
LL | Self::Output::Simple {}; | ||
| ^^^^^^^^^^^^ cannot infer type for type parameter `T` declared on the associated type `Output` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should point at Output
as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what exactly do you mean with this? This should suggest Self::Output::<T>
but that's part of the FIXME
i've added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant something like
error[E0282]: type annotations needed
--> $DIR/expr-struct-type-relative-gat.rs:17:9
|
LL | type Output<T> = Bar<T>;
| - type parameter `T` that couldn't be infered on `Output`
LL | fn baz() {
LL | Self::Output::Simple {};
| ^^^^^^^^^^^^ cannot infer type for type parameter `T` declared on the associated type `Output`
as a stop gap solution, but ideally all of this would look like this
error[E0282]: type annotations needed
--> $DIR/expr-struct-type-relative-gat.rs:17:9
|
LL | type Output<T> = Bar<T>;
| - type parameter `T` that couldn't be infered on `Output`
LL | fn baz() {
LL | Self::Output::Simple {};
| ^^^^^^^^^^^^ cannot infer type for type parameter `T` declared on the associated type `Output`
help: specify the type parameter on associated type `Output`
|
LL | Self::Output::<T>::Simple {};
| +++++
Edit: although thinking about it for a second, maybe we should suggest other forms, like adding a param to baz
and passing that through, I'm not sure about how GATs will actually be used in the wild to get a sense of what the appropriate suggestion might actually be.
// FIXME: Ideally we would also deal with type relative | ||
// paths here, even if that is quite rare. | ||
// | ||
// See the `need_type_info/expr-struct-type-relative-gat.rs` test | ||
// for an example where that would be needed. | ||
// | ||
// However, the `type_dependent_def_id` for `Self::Output` in an | ||
// impl is currently the `DefId` of `Output` in the trait definition | ||
// which makes this somewhat difficult and prevents us from just | ||
// using `self.path_inferred_subst_iter` here. | ||
hir::ExprKind::Struct(&hir::QPath::Resolved(_self_ty, path), _, _) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we leave the ticket open so that the DefId
of Self::Output
corresponds to the impl assoc type and not the trait assoc type? That seems to be the correct fix, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
opened #98711
if let Some(ty) = self.opt_node_type(expr.hir_id) { | ||
if let ty::Adt(_, substs) = ty.kind() { | ||
return self.path_inferred_subst_iter(expr.hir_id, substs, path); | ||
return Box::new(self.resolved_path_inferred_subst_iter(path, substs)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the filtering above, this change isn't also needed, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it isn't strictly needed, but it feels weird to call path_inferred_subst_iter
if we already know that we're only dealing with resolved paths.
r? @estebank |
@bors r+ |
📌 Commit e043821 has been approved by |
…askrgr Rollup of 9 pull requests Successful merges: - rust-lang#98610 (fix `emit_inference_failure_err` ICE) - rust-lang#98640 (Let rust-analyzer ship on stable, non-preview) - rust-lang#98686 (add ice test for 46511) - rust-lang#98727 (rustdoc: filter '_ lifetimes from ty::PolyTraitRef) - rust-lang#98729 (clarify that ExactSizeIterator::len returns the remaining length) - rust-lang#98733 (Request to be notified of MIR changes) - rust-lang#98734 (Update RELEASES.md) - rust-lang#98745 (Add a `--build-dir` flag to rustbuild) - rust-lang#98749 (Add macro_rules! rustdoc change to 1.62 relnotes) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
fixes #98598
this fix doesn't make me too happy, but 🤷