Skip to content

Point to where clause for GATs to add bound #87478

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

Merged
merged 1 commit into from
Aug 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 94 additions & 4 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2259,9 +2259,99 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
};

let mut err = match *sub {
ty::ReEarlyBound(ty::EarlyBoundRegion { name, .. })
| ty::ReFree(ty::FreeRegion { bound_region: ty::BrNamed(_, name), .. }) => {
#[derive(Debug)]
enum SubOrigin<'hir> {
GAT(&'hir hir::Generics<'hir>),
Impl(&'hir hir::Generics<'hir>),
Trait(&'hir hir::Generics<'hir>),
Fn(&'hir hir::Generics<'hir>),
Unknown,
}
let sub_origin = 'origin: {
match *sub {
ty::ReEarlyBound(ty::EarlyBoundRegion { def_id, .. }) => {
let node = self.tcx.hir().get_if_local(def_id).unwrap();
match node {
Node::GenericParam(param) => {
for h in self.tcx.hir().parent_iter(param.hir_id) {
break 'origin match h.1 {
Node::ImplItem(hir::ImplItem {
kind: hir::ImplItemKind::TyAlias(..),
generics,
..
}) => SubOrigin::GAT(generics),
Node::ImplItem(hir::ImplItem {
kind: hir::ImplItemKind::Fn(..),
generics,
..
}) => SubOrigin::Fn(generics),
Node::TraitItem(hir::TraitItem {
kind: hir::TraitItemKind::Type(..),
generics,
..
}) => SubOrigin::GAT(generics),
Node::TraitItem(hir::TraitItem {
kind: hir::TraitItemKind::Fn(..),
generics,
..
}) => SubOrigin::Fn(generics),
Node::Item(hir::Item {
kind: hir::ItemKind::Trait(_, _, generics, _, _),
..
}) => SubOrigin::Trait(generics),
Node::Item(hir::Item {
kind: hir::ItemKind::Impl(hir::Impl { generics, .. }),
..
}) => SubOrigin::Impl(generics),
Node::Item(hir::Item {
kind: hir::ItemKind::Fn(_, generics, _),
..
}) => SubOrigin::Fn(generics),
_ => continue,
};
}
}
_ => {}
}
}
_ => {}
}
SubOrigin::Unknown
};
debug!(?sub_origin);

let mut err = match (*sub, sub_origin) {
// In the case of GATs, we have to be careful. If we a type parameter `T` on an impl,
// but a lifetime `'a` on an associated type, then we might need to suggest adding
// `where T: 'a`. Importantly, this is on the GAT span, not on the `T` declaration.
(ty::ReEarlyBound(ty::EarlyBoundRegion { name: _, .. }), SubOrigin::GAT(generics)) => {
// Does the required lifetime have a nice name we can print?
let mut err = struct_span_err!(
self.tcx.sess,
span,
E0309,
"{} may not live long enough",
labeled_user_string
);
let pred = format!("{}: {}", bound_kind, sub);
let suggestion = format!(
"{} {}",
if !generics.where_clause.predicates.is_empty() { "," } else { " where" },
pred,
);
err.span_suggestion(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make the suggestion a bit easier to read, given that it points at a span right in the middle of a larger span:

Suggested change
err.span_suggestion(
err.span_suggestion_verbose(

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although that would require rewording the other span label...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I leave it to your decision what to do here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, now that the associated type points to the type itself, not the whole defintion, they don't overlap anymore.

I do think the verbose suggestion would be nice, but I'll leave that as a followup.

generics.where_clause.tail_span_for_suggestion(),
"consider adding a where clause".into(),
suggestion,
Applicability::MaybeIncorrect,
);
err
}
(
ty::ReEarlyBound(ty::EarlyBoundRegion { name, .. })
| ty::ReFree(ty::FreeRegion { bound_region: ty::BrNamed(_, name), .. }),
_,
) => {
// Does the required lifetime have a nice name we can print?
let mut err = struct_span_err!(
self.tcx.sess,
Expand All @@ -2278,7 +2368,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
err
}

ty::ReStatic => {
(ty::ReStatic, _) => {
// Does the required lifetime have a nice name we can print?
let mut err = struct_span_err!(
self.tcx.sess,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_infer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#![feature(in_band_lifetimes)]
#![feature(control_flow_enum)]
#![feature(min_specialization)]
#![feature(label_break_value)]
#![recursion_limit = "512"] // For rustdoc

#[macro_use]
Expand Down
22 changes: 22 additions & 0 deletions src/test/ui/generic-associated-types/issue-84931.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#![feature(generic_associated_types)]
// check-fail

trait StreamingIter {
type Item<'a> where Self: 'a;
fn next<'a>(&'a mut self) -> Option<Self::Item::<'a>>;
}

struct StreamingSliceIter<'a, T> {
idx: usize,
data: &'a mut [T],
}

impl<'b, T: 'b> StreamingIter for StreamingSliceIter<'b, T> {
type Item<'a> = &'a mut T;
//~^ the parameter type
fn next(&mut self) -> Option<&mut T> {
loop {}
}
}

fn main() {}
11 changes: 11 additions & 0 deletions src/test/ui/generic-associated-types/issue-84931.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0309]: the parameter type `T` may not live long enough
--> $DIR/issue-84931.rs:15:21
|
LL | type Item<'a> = &'a mut T;
| - ^^^^^^^^^ ...so that the reference type `&'a mut T` does not outlive the data it points at
| |
| help: consider adding a where clause: `where T: 'a`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0309`.
7 changes: 3 additions & 4 deletions src/test/ui/generic-associated-types/issue-86483.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,10 @@ LL | for<'a> T: 'a,
error[E0309]: the parameter type `T` may not live long enough
--> $DIR/issue-86483.rs:9:32
|
LL | pub trait IceIce<T>
| - help: consider adding an explicit lifetime bound...: `T: 'v`
...
LL | type Ice<'v>: IntoIterator<Item = &'v T>;
| ^^^^^^^^^^^^ ...so that the reference type `&'v T` does not outlive the data it points at
| ^^^^^^^^^^^^ - help: consider adding a where clause: `where T: 'v`
| |
| ...so that the reference type `&'v T` does not outlive the data it points at

error: aborting due to 3 previous errors

Expand Down
40 changes: 40 additions & 0 deletions src/test/ui/generic-associated-types/issue-86787.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#![feature(generic_associated_types)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we mark the tests // run-rustfix?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So...no. The suggestion here doesn't actually result in code that compiles, because it causes the where clauses not to match the trait's where clauses. (In fact, in this case, we can't write this where clause in the trait, we would have to use Self)

Unfortunately, that's a whole separate issue with GATs. But applying the current suggestion should help, in the future, to guide the user towards matching the trait's where clauses.

// check-fail

enum Either<L, R> {
Left(L),
Right(R),
}

pub trait HasChildrenOf {
type T;
type TRef<'a>;

fn ref_children<'a>(&'a self) -> Vec<Self::TRef<'a>>;
fn take_children(self) -> Vec<Self::T>;
}

impl<Left, Right> HasChildrenOf for Either<Left, Right>
where
Left: HasChildrenOf,
Right: HasChildrenOf,
{
type T = Either<Left::T, Right::T>;
type TRef<'a>
//~^ the associated type
//~^^ the associated type
where
<Left as HasChildrenOf>::T: 'a,
<Right as HasChildrenOf>::T: 'a
= Either<&'a Left::T, &'a Right::T>;

fn ref_children<'a>(&'a self) -> Vec<Self::TRef<'a>> {
todo!()
}

fn take_children(self) -> Vec<Self::T> {
todo!()
}
}

fn main() {}
29 changes: 29 additions & 0 deletions src/test/ui/generic-associated-types/issue-86787.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
error[E0309]: the associated type `<Left as HasChildrenOf>::T` may not live long enough
--> $DIR/issue-86787.rs:23:5
|
LL | / type TRef<'a>
LL | |
LL | |
LL | | where
LL | | <Left as HasChildrenOf>::T: 'a,
LL | | <Right as HasChildrenOf>::T: 'a
| | - help: consider adding a where clause: `, <Left as HasChildrenOf>::T: 'a`
LL | | = Either<&'a Left::T, &'a Right::T>;
| |________________________________________^ ...so that the type `<Left as HasChildrenOf>::T` will meet its required lifetime bounds

error[E0309]: the associated type `<Right as HasChildrenOf>::T` may not live long enough
--> $DIR/issue-86787.rs:23:5
|
LL | / type TRef<'a>
LL | |
LL | |
LL | | where
LL | | <Left as HasChildrenOf>::T: 'a,
LL | | <Right as HasChildrenOf>::T: 'a
| | - help: consider adding a where clause: `, <Right as HasChildrenOf>::T: 'a`
LL | | = Either<&'a Left::T, &'a Right::T>;
| |________________________________________^ ...so that the type `<Right as HasChildrenOf>::T` will meet its required lifetime bounds

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0309`.