Skip to content
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.

Sign up for GitHub

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 printing impl trait under binders #98371

Merged
merged 2 commits into from
Jun 26, 2022
Merged

Fix printing impl trait under binders #98371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
20cea3e
Fix printing impl trait under binders
compiler-errors Jun 22, 2022
e80cced
Use write! instead of p! to avoid having to use weird scoping
compiler-errors Jun 22, 2022
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
233 changes: 120 additions & 113 deletions compiler/rustc_middle/src/ty/print/pretty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ pub trait PrettyPrinter<'tcx>:
value.as_ref().skip_binder().print(self)
}

fn wrap_binder<T, F: Fn(&T, Self) -> Result<Self, fmt::Error>>(
fn wrap_binder<T, F: FnOnce(&T, Self) -> Result<Self, fmt::Error>>(
self,
value: &ty::Binder<'tcx, T>,
f: F,
Expand Down Expand Up @@ -773,26 +773,26 @@ pub trait PrettyPrinter<'tcx>:
def_id: DefId,
substs: &'tcx ty::List<ty::GenericArg<'tcx>>,
) -> Result<Self::Type, Self::Error> {
define_scoped_cx!(self);
let tcx = self.tcx();

// Grab the "TraitA + TraitB" from `impl TraitA + TraitB`,
// by looking up the projections associated with the def_id.
let bounds = self.tcx().bound_explicit_item_bounds(def_id);
let bounds = tcx.bound_explicit_item_bounds(def_id);

let mut traits = FxIndexMap::default();
let mut fn_traits = FxIndexMap::default();
let mut is_sized = false;

for predicate in bounds.transpose_iter().map(|e| e.map_bound(|(p, _)| *p)) {
let predicate = predicate.subst(self.tcx(), substs);
let predicate = predicate.subst(tcx, substs);
let bound_predicate = predicate.kind();

match bound_predicate.skip_binder() {
ty::PredicateKind::Trait(pred) => {
let trait_ref = bound_predicate.rebind(pred.trait_ref);

// Don't print + Sized, but rather + ?Sized if absent.
if Some(trait_ref.def_id()) == self.tcx().lang_items().sized_trait() {
if Some(trait_ref.def_id()) == tcx.lang_items().sized_trait() {
is_sized = true;
continue;
}
Expand All @@ -801,7 +801,7 @@ pub trait PrettyPrinter<'tcx>:
}
ty::PredicateKind::Projection(pred) => {
let proj_ref = bound_predicate.rebind(pred);
let trait_ref = proj_ref.required_poly_trait_ref(self.tcx());
let trait_ref = proj_ref.required_poly_trait_ref(tcx);

// Projection type entry -- the def-id for naming, and the ty.
let proj_ty = (proj_ref.projection_def_id(), proj_ref.term());
Expand All @@ -817,148 +817,155 @@ pub trait PrettyPrinter<'tcx>:
}
}

write!(self, "impl ")?;

let mut first = true;
// Insert parenthesis around (Fn(A, B) -> C) if the opaque ty has more than one other trait
let paren_needed = fn_traits.len() > 1 || traits.len() > 0 || !is_sized;

p!("impl");

for (fn_once_trait_ref, entry) in fn_traits {
// Get the (single) generic ty (the args) of this FnOnce trait ref.
let generics = self.tcx().generics_of(fn_once_trait_ref.def_id());
let args =
generics.own_substs_no_defaults(self.tcx(), fn_once_trait_ref.skip_binder().substs);

match (entry.return_ty, args[0].expect_ty()) {
// We can only print `impl Fn() -> ()` if we have a tuple of args and we recorded
// a return type.
(Some(return_ty), arg_tys) if matches!(arg_tys.kind(), ty::Tuple(_)) => {
let name = if entry.fn_trait_ref.is_some() {
"Fn"
} else if entry.fn_mut_trait_ref.is_some() {
"FnMut"
} else {
"FnOnce"
};
write!(self, "{}", if first { "" } else { " + " })?;
write!(self, "{}", if paren_needed { "(" } else { "" })?;

p!(
write("{}", if first { " " } else { " + " }),
write("{}{}(", if paren_needed { "(" } else { "" }, name)
);
self = self.wrap_binder(&fn_once_trait_ref, |trait_ref, mut cx| {
define_scoped_cx!(cx);
// Get the (single) generic ty (the args) of this FnOnce trait ref.
let generics = tcx.generics_of(trait_ref.def_id);
let args = generics.own_substs_no_defaults(tcx, trait_ref.substs);

match (entry.return_ty, args[0].expect_ty()) {
// We can only print `impl Fn() -> ()` if we have a tuple of args and we recorded
// a return type.
(Some(return_ty), arg_tys) if matches!(arg_tys.kind(), ty::Tuple(_)) => {
let name = if entry.fn_trait_ref.is_some() {
"Fn"
} else if entry.fn_mut_trait_ref.is_some() {
"FnMut"
} else {
"FnOnce"
};

for (idx, ty) in arg_tys.tuple_fields().iter().enumerate() {
if idx > 0 {
p!(", ");
p!(write("{}(", name));

for (idx, ty) in arg_tys.tuple_fields().iter().enumerate() {
if idx > 0 {
p!(", ");
}
p!(print(ty));
}
p!(print(ty));
}

p!(")");
if let Term::Ty(ty) = return_ty.skip_binder() {
if !ty.is_unit() {
p!(" -> ", print(return_ty));
p!(")");
if let Term::Ty(ty) = return_ty.skip_binder() {
if !ty.is_unit() {
p!(" -> ", print(return_ty));
}
}
}
p!(write("{}", if paren_needed { ")" } else { "" }));
p!(write("{}", if paren_needed { ")" } else { "" }));

first = false;
}
// If we got here, we can't print as a `impl Fn(A, B) -> C`. Just record the
// trait_refs we collected in the OpaqueFnEntry as normal trait refs.
_ => {
if entry.has_fn_once {
traits.entry(fn_once_trait_ref).or_default().extend(
// Group the return ty with its def id, if we had one.
entry
.return_ty
.map(|ty| (self.tcx().lang_items().fn_once_output().unwrap(), ty)),
);
}
if let Some(trait_ref) = entry.fn_mut_trait_ref {
traits.entry(trait_ref).or_default();
first = false;
}
if let Some(trait_ref) = entry.fn_trait_ref {
traits.entry(trait_ref).or_default();
// If we got here, we can't print as a `impl Fn(A, B) -> C`. Just record the
// trait_refs we collected in the OpaqueFnEntry as normal trait refs.
_ => {
if entry.has_fn_once {
traits.entry(fn_once_trait_ref).or_default().extend(
// Group the return ty with its def id, if we had one.
entry
.return_ty
.map(|ty| (tcx.lang_items().fn_once_output().unwrap(), ty)),
);
}
if let Some(trait_ref) = entry.fn_mut_trait_ref {
traits.entry(trait_ref).or_default();
}
if let Some(trait_ref) = entry.fn_trait_ref {
traits.entry(trait_ref).or_default();
}
}
}
}

Ok(cx)
})?;
}

// Print the rest of the trait types (that aren't Fn* family of traits)
for (trait_ref, assoc_items) in traits {
p!(
write("{}", if first { " " } else { " + " }),
print(trait_ref.skip_binder().print_only_trait_name())
);
write!(self, "{}", if first { "" } else { " + " })?;

self = self.wrap_binder(&trait_ref, |trait_ref, mut cx| {
define_scoped_cx!(cx);
p!(print(trait_ref.print_only_trait_name()));

let generics = self.tcx().generics_of(trait_ref.def_id());
let args = generics.own_substs_no_defaults(self.tcx(), trait_ref.skip_binder().substs);
let generics = tcx.generics_of(trait_ref.def_id);
let args = generics.own_substs_no_defaults(tcx, trait_ref.substs);

if !args.is_empty() || !assoc_items.is_empty() {
let mut first = true;
if !args.is_empty() || !assoc_items.is_empty() {
let mut first = true;

for ty in args {
if first {
p!("<");
first = false;
} else {
p!(", ");
for ty in args {
if first {
p!("<");
first = false;
} else {
p!(", ");
}
p!(print(ty));
}
p!(print(trait_ref.rebind(*ty)));
}

for (assoc_item_def_id, term) in assoc_items {
// Skip printing `<[generator@] as Generator<_>>::Return` from async blocks,
// unless we can find out what generator return type it comes from.
let term = if let Some(ty) = term.skip_binder().ty()
&& let ty::Projection(ty::ProjectionTy { item_def_id, substs }) = ty.kind()
&& Some(*item_def_id) == self.tcx().lang_items().generator_return()
{
if let ty::Generator(_, substs, _) = substs.type_at(0).kind() {
let return_ty = substs.as_generator().return_ty();
if !return_ty.is_ty_infer() {
return_ty.into()
for (assoc_item_def_id, term) in assoc_items {
// Skip printing `<[generator@] as Generator<_>>::Return` from async blocks,
// unless we can find out what generator return type it comes from.
let term = if let Some(ty) = term.skip_binder().ty()
&& let ty::Projection(ty::ProjectionTy { item_def_id, substs }) = ty.kind()
&& Some(*item_def_id) == tcx.lang_items().generator_return()
{
if let ty::Generator(_, substs, _) = substs.type_at(0).kind() {
let return_ty = substs.as_generator().return_ty();
if !return_ty.is_ty_infer() {
return_ty.into()
} else {
continue;
}
} else {
continue;
}
} else {
continue;
}
} else {
term.skip_binder()
};
term.skip_binder()
};

if first {
p!("<");
first = false;
} else {
p!(", ");
}
if first {
p!("<");
first = false;
} else {
p!(", ");
}

p!(write("{} = ", self.tcx().associated_item(assoc_item_def_id).name));
p!(write("{} = ", tcx.associated_item(assoc_item_def_id).name));

match term {
Term::Ty(ty) => {
p!(print(ty))
}
Term::Const(c) => {
p!(print(c));
}
};
}
match term {
Term::Ty(ty) => {
p!(print(ty))
}
Term::Const(c) => {
p!(print(c));
}
};
}

if !first {
p!(">");
if !first {
p!(">");
}
}
}

first = false;
first = false;
Ok(cx)
})?;
}

if !is_sized {
p!(write("{}?Sized", if first { " " } else { " + " }));
write!(self, "{}?Sized", if first { "" } else { " + " })?;
} else if first {
p!(" Sized");
write!(self, "Sized")?;
}

Ok(self)
Expand Down Expand Up @@ -1869,7 +1876,7 @@ impl<'tcx> PrettyPrinter<'tcx> for FmtPrinter<'_, 'tcx> {
self.pretty_in_binder(value)
}

fn wrap_binder<T, C: Fn(&T, Self) -> Result<Self, Self::Error>>(
fn wrap_binder<T, C: FnOnce(&T, Self) -> Result<Self, Self::Error>>(
self,
value: &ty::Binder<'tcx, T>,
f: C,
Expand Down Expand Up @@ -2256,7 +2263,7 @@ impl<'tcx> FmtPrinter<'_, 'tcx> {
Ok(inner)
}

pub fn pretty_wrap_binder<T, C: Fn(&T, Self) -> Result<Self, fmt::Error>>(
pub fn pretty_wrap_binder<T, C: FnOnce(&T, Self) -> Result<Self, fmt::Error>>(
self,
value: &ty::Binder<'tcx, T>,
f: C,
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/async-await/issue-70935-complex-spans.drop_tracking.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ LL | async fn baz<T>(_c: impl FnMut() -> T) where T: Future<Output=()> {
LL | |
LL | | }
| |_^
= note: required because it captures the following types: `ResumeTy`, `impl Future<Output = ()>`, `()`
= note: required because it captures the following types: `ResumeTy`, `impl for<'r, 's, 't0> Future<Output = ()>`, `()`
Copy link
Member

Choose a reason for hiding this comment

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

hmm, is this change expected? I guess it doesn't really matter since I'm going to remove it in #98754, but I wonder if it can come up in diagnostics for generators or something like that?

Copy link
Member Author

Choose a reason for hiding this comment

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

well so the only reason we weren't printing this in the first place was because of another bug... the bound lifetimes are there though

i mean i guess we could filter them out or something 🤷

Copy link
Member

Choose a reason for hiding this comment

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

hmm, sure - I think what's confusing is that the lifetimes don't appear in the impl Future pretty printing, only the for binder.

maybe that's ok though and this gives a hint of which lifetimes are captured? I don't feel super strongly either way

note: required because it's used within this `async` block
--> $DIR/issue-70935-complex-spans.rs:23:16
|
Expand Down
14 changes: 14 additions & 0 deletions src/test/ui/impl-trait/printing-binder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
trait Trait<'a> {}
impl<T> Trait<'_> for T {}
fn whatever() -> impl for<'a> Trait<'a> + for<'b> Trait<'b> {}

fn whatever2() -> impl for<'c> Fn(&'c ()) {
|_: &()| {}
}

fn main() {
let x: u32 = whatever();
//~^ ERROR mismatched types
let x2: u32 = whatever2();
//~^ ERROR mismatched types
}
31 changes: 31 additions & 0 deletions src/test/ui/impl-trait/printing-binder.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
error[E0308]: mismatched types
--> $DIR/printing-binder.rs:10:18
|
LL | fn whatever() -> impl for<'a> Trait<'a> + for<'b> Trait<'b> {}
| ------------------------------------------ the found opaque type
...
LL | let x: u32 = whatever();
| --- ^^^^^^^^^^ expected `u32`, found opaque type
| |
| expected due to this
|
= note: expected type `u32`
found opaque type `impl for<'a> Trait<'a> + for<'b> Trait<'b>`

error[E0308]: mismatched types
--> $DIR/printing-binder.rs:12:19
|
LL | fn whatever2() -> impl for<'c> Fn(&'c ()) {
| ----------------------- the found opaque type
...
LL | let x2: u32 = whatever2();
| --- ^^^^^^^^^^^ expected `u32`, found opaque type
| |
| expected due to this
|
= note: expected type `u32`
found opaque type `impl for<'c> Fn(&'c ())`

error: aborting due to 2 previous errors

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