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.

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

Include type bindings on string representation of impl Trait #63526

Open
estebank opened this issue Aug 13, 2019 · 13 comments
Open

Include type bindings on string representation of impl Trait #63526

estebank opened this issue Aug 13, 2019 · 13 comments

Comments

@estebank
Copy link
Contributor

@estebank estebank commented Aug 13, 2019

Follow up to #63507 (comment)

Include type arguments for traits when displaying impl Trait. For example, the following:

error[E0282]: type annotations needed for `impl std::future::Future`
  --> $DIR/cannot-infer-async-enabled-impl-trait-bindings.rs:14:9
   |
LL |     let fut = async {
   |         --- consider giving `fut` the explicit type `impl std::future::Future`, with the type parameters specified
LL |         make_unit()?;
   |         ^^^^^^^^^^^^ cannot infer type

should be

error[E0282]: type annotations needed for `impl std::future::Future`
  --> $DIR/cannot-infer-async-enabled-impl-trait-bindings.rs:14:9
   |
LL |     let fut = async {
   |         --- consider giving `fut` the explicit type `impl std::future::Future<Output=_>`, with the type parameters specified
LL |         make_unit()?;
   |         ^^^^^^^^^^^^ cannot infer type
@iluuu1994
Copy link
Contributor

@iluuu1994 iluuu1994 commented Aug 14, 2019

@rustbot claim

Should have time tonight. 🙂

Loading

@rustbot rustbot self-assigned this Aug 14, 2019
@iluuu1994
Copy link
Contributor

@iluuu1994 iluuu1994 commented Aug 14, 2019

I'll probably better wait until #63507 is merged 😊

Loading

@estebank
Copy link
Contributor Author

@estebank estebank commented Aug 14, 2019

@iluuu1994 that should happen sometime today.

Loading

@iluuu1994
Copy link
Contributor

@iluuu1994 iluuu1994 commented Aug 16, 2019

@estebank Would you mind giving me a few pointers? I looked through the source code but it wasn't completely obvious to me on how to achieve this. Is this already supported by the FmtPrinter or something new that has to be implemented?

Loading

@estebank
Copy link
Contributor Author

@estebank estebank commented Aug 19, 2019

I'm not sure if this codepath is using FmtPrinter (I did a quick check changing this to "im pl " and I didn't see any change to this error. I think that what you want to look at is

fn default_print_impl_path(
self,
impl_def_id: DefId,
_substs: &'tcx [Kind<'tcx>],
self_ty: Ty<'tcx>,
impl_trait_ref: Option<ty::TraitRef<'tcx>>,
) -> Result<Self::Path, Self::Error> {
debug!("default_print_impl_path: impl_def_id={:?}, self_ty={}, impl_trait_ref={:?}",
impl_def_id, self_ty, impl_trait_ref);
let key = self.tcx().def_key(impl_def_id);
let parent_def_id = DefId { index: key.parent.unwrap(), ..impl_def_id };
// Decide whether to print the parent path for the impl.
// Logically, since impls are global, it's never needed, but
// users may find it useful. Currently, we omit the parent if
// the impl is either in the same module as the self-type or
// as the trait.
let in_self_mod = match characteristic_def_id_of_type(self_ty) {
None => false,
Some(ty_def_id) => self.tcx().parent(ty_def_id) == Some(parent_def_id),
};
let in_trait_mod = match impl_trait_ref {
None => false,
Some(trait_ref) => self.tcx().parent(trait_ref.def_id) == Some(parent_def_id),
};
if !in_self_mod && !in_trait_mod {
// If the impl is not co-located with either self-type or
// trait-type, then fallback to a format that identifies
// the module more clearly.
self.path_append_impl(
|cx| cx.print_def_path(parent_def_id, &[]),
&key.disambiguated_data,
self_ty,
impl_trait_ref,
)
} else {
// Otherwise, try to give a good form that would be valid language
// syntax. Preferably using associated item notation.
self.path_qualified(self_ty, impl_trait_ref)
}
}
}

and make it use the _substs, but again, I'm not 100% sure that is the case.

Loading

@iluuu1994
Copy link
Contributor

@iluuu1994 iluuu1994 commented Aug 19, 2019

I will try that, thanks @estebank 🙂

Loading

@estebank
Copy link
Contributor Author

@estebank estebank commented Aug 19, 2019

It's less than ideal, but using RUSTC_LOG=rustc::ty::print=debug causes a stack overflow, which makes it harder to figure out what's going on:


thread 'rustc' has overflowed its stack
fatal runtime error: stack overflow
Abort trap: 6

Loading

@estebank
Copy link
Contributor Author

@estebank estebank commented Aug 21, 2019

@iluuu1994 this is the code you want to modify:

ty::Opaque(def_id, substs) => {
// FIXME(eddyb) print this with `print_def_path`.
if self.tcx().sess.verbose() {
p!(write("Opaque({:?}, {:?})", def_id, substs));
return Ok(self);
}
let def_key = self.tcx().def_key(def_id);
if let Some(name) = def_key.disambiguated_data.data.get_opt_name() {
p!(write("{}", name));
let mut substs = substs.iter();
// FIXME(eddyb) print this with `print_def_path`.
if let Some(first) = substs.next() {
p!(write("::<"));
p!(print(first));
for subst in substs {
p!(write(", "), print(subst));
}
p!(write(">"));
}
return Ok(self);
}
// Grab the "TraitA + TraitB" from `impl TraitA + TraitB`,
// by looking up the projections associated with the def_id.
let bounds = self.tcx().predicates_of(def_id).instantiate(self.tcx(), substs);
let mut first = true;
let mut is_sized = false;
p!(write("impl"));
for predicate in bounds.predicates {
if let Some(trait_ref) = predicate.to_opt_poly_trait_ref() {
// Don't print +Sized, but rather +?Sized if absent.
if Some(trait_ref.def_id()) == self.tcx().lang_items().sized_trait() {
is_sized = true;
continue;
}
p!(
write("{}", if first { " " } else { "+" }),
print(trait_ref));
first = false;
}
}
if !is_sized {
p!(write("{}?Sized", if first { " " } else { "+" }));
} else if first {
p!(write(" Sized"));
}
}

probably looking at how associated types are printed for dyn types should help:

fn pretty_print_dyn_existential(
mut self,
predicates: &'tcx ty::List<ty::ExistentialPredicate<'tcx>>,
) -> Result<Self::DynExistential, Self::Error> {
define_scoped_cx!(self);
// Generate the main trait ref, including associated types.
let mut first = true;
if let Some(principal) = predicates.principal() {
p!(print_def_path(principal.def_id, &[]));
let mut resugared = false;
// Special-case `Fn(...) -> ...` and resugar it.
let fn_trait_kind = self.tcx().lang_items().fn_trait_kind(principal.def_id);
if !self.tcx().sess.verbose() && fn_trait_kind.is_some() {
if let ty::Tuple(ref args) = principal.substs.type_at(0).sty {
let mut projections = predicates.projection_bounds();
if let (Some(proj), None) = (projections.next(), projections.next()) {
let tys: Vec<_> = args.iter().map(|k| k.expect_ty()).collect();
p!(pretty_fn_sig(&tys, false, proj.ty));
resugared = true;
}
}
}
// HACK(eddyb) this duplicates `FmtPrinter`'s `path_generic_args`,
// in order to place the projections inside the `<...>`.
if !resugared {
// Use a type that can't appear in defaults of type parameters.
let dummy_self = self.tcx().mk_ty_infer(ty::FreshTy(0));
let principal = principal.with_self_ty(self.tcx(), dummy_self);
let args = self.generic_args_to_print(
self.tcx().generics_of(principal.def_id),
principal.substs,
);
// Don't print `'_` if there's no unerased regions.
let print_regions = args.iter().any(|arg| {
match arg.unpack() {
UnpackedKind::Lifetime(r) => *r != ty::ReErased,
_ => false,
}
});
let mut args = args.iter().cloned().filter(|arg| {
match arg.unpack() {
UnpackedKind::Lifetime(_) => print_regions,
_ => true,
}
});
let mut projections = predicates.projection_bounds();
let arg0 = args.next();
let projection0 = projections.next();
if arg0.is_some() || projection0.is_some() {
let args = arg0.into_iter().chain(args);
let projections = projection0.into_iter().chain(projections);
p!(generic_delimiters(|mut cx| {
cx = cx.comma_sep(args)?;
if arg0.is_some() && projection0.is_some() {
write!(cx, ", ")?;
}
cx.comma_sep(projections)
}));
}
}
first = false;
}

Loading

@iluuu1994
Copy link
Contributor

@iluuu1994 iluuu1994 commented Aug 21, 2019

Thanks! Sorry I've just not gotten around to it. Hopefully tonight 😊

Loading

@oliviabrown9
Copy link

@oliviabrown9 oliviabrown9 commented Sep 27, 2019

@iluuu1994 Are you still working on this? If not, I can grab it :)

Loading

@iluuu1994
Copy link
Contributor

@iluuu1994 iluuu1994 commented Sep 27, 2019

@oliviabrown9 Nope, feel free to take over.

Loading

@eddyb
Copy link
Member

@eddyb eddyb commented Sep 27, 2019

This issue seems to be about associated type bindings, but the title mentions type parameters.
The whole to_opt_poly_trait_ref check results in hiding all the bounds that aren't trait bounds.

Due to impl Foo + Bar existing, I think this is harder than the dyn Trait case, because you have to pair up projection bounds to the right trait, kind of like how rustdoc does for displaying something nicer.

This makes me think E-easy is not the right label, but I'm not sure yet.
In the future please cc me on all ty::print-related issues/PRs.

cc @nikomatsakis @matthewjasper

Loading

@estebank estebank removed the E-easy label Nov 12, 2019
@JohnTitor
Copy link
Member

@JohnTitor JohnTitor commented Apr 17, 2020

@rustbot release-assignment

Loading

@rustbot rustbot removed their assignment Apr 17, 2020
@estebank estebank changed the title Include type parameters on string representation of impl Trait Include type bindings on string representation of impl Trait Jul 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants