Skip to content

Commit

Permalink
Rollup merge of rust-lang#118268 - compiler-errors:pretty-print, r=es…
Browse files Browse the repository at this point in the history
…tebank

Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always

It's almost always better, at least in diagnostics, to print `Fn(i32, u32)` instead of `Fn<(i32, u32)>`.

Related to but doesn't fix rust-lang#118225. That needs a separate fix.
  • Loading branch information
matthiaskrgr authored Nov 29, 2023
2 parents f0d9e4f + ecccf33 commit d538d16
Show file tree
Hide file tree
Showing 22 changed files with 104 additions and 69 deletions.
5 changes: 1 addition & 4 deletions compiler/rustc_hir_analysis/src/astconv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1181,10 +1181,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
if let Some(bound_span) = bound_span {
err.span_label(
bound_span,
format!(
"ambiguous `{assoc_name}` from `{}`",
bound.print_only_trait_path(),
),
format!("ambiguous `{assoc_name}` from `{}`", bound.print_trait_sugared(),),
);
if let Some(constraint) = &is_equality {
where_bounds.push(format!(
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir_analysis/src/astconv/object_safety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
trait here instead: `trait NewTrait: {} {{}}`",
regular_traits
.iter()
// FIXME: This should `print_sugared`, but also needs to integrate projection bounds...
.map(|t| t.trait_ref().print_only_trait_path().to_string())
.collect::<Vec<_>>()
.join(" + "),
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_hir_analysis/src/coherence/unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub(super) fn check_item(tcx: TyCtxt<'_>, def_id: LocalDefId) {
tcx.def_span(def_id),
E0199,
"implementing the trait `{}` is not unsafe",
trait_ref.print_only_trait_path()
trait_ref.print_trait_sugared()
)
.span_suggestion_verbose(
item.span.with_hi(item.span.lo() + rustc_span::BytePos(7)),
Expand All @@ -40,13 +40,13 @@ pub(super) fn check_item(tcx: TyCtxt<'_>, def_id: LocalDefId) {
tcx.def_span(def_id),
E0200,
"the trait `{}` requires an `unsafe impl` declaration",
trait_ref.print_only_trait_path()
trait_ref.print_trait_sugared()
)
.note(format!(
"the trait `{}` enforces invariants that the compiler can't check. \
Review the trait documentation and make sure this implementation \
upholds those invariants before adding the `unsafe` keyword",
trait_ref.print_only_trait_path()
trait_ref.print_trait_sugared()
))
.span_suggestion_verbose(
item.span.shrink_to_lo(),
Expand All @@ -69,7 +69,7 @@ pub(super) fn check_item(tcx: TyCtxt<'_>, def_id: LocalDefId) {
"the trait `{}` enforces invariants that the compiler can't check. \
Review the trait documentation and make sure this implementation \
upholds those invariants before adding the `unsafe` keyword",
trait_ref.print_only_trait_path()
trait_ref.print_trait_sugared()
))
.span_suggestion_verbose(
item.span.shrink_to_lo(),
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2288,7 +2288,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ty::Adt(def, _) if def.did().is_local() => {
spans.push_span_label(
self.tcx.def_span(def.did()),
format!("must implement `{}`", pred.trait_ref.print_only_trait_path()),
format!("must implement `{}`", pred.trait_ref.print_trait_sugared()),
);
}
_ => {}
Expand All @@ -2299,7 +2299,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let msg = if preds.len() == 1 {
format!(
"an implementation of `{}` might be missing for `{}`",
preds[0].trait_ref.print_only_trait_path(),
preds[0].trait_ref.print_trait_sugared(),
preds[0].self_ty()
)
} else {
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2221,8 +2221,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
infer::ExistentialProjection(exp_found) => self.expected_found_str(exp_found),
infer::TraitRefs(exp_found) => {
let pretty_exp_found = ty::error::ExpectedFound {
expected: exp_found.expected.print_only_trait_path(),
found: exp_found.found.print_only_trait_path(),
expected: exp_found.expected.print_trait_sugared(),
found: exp_found.found.print_trait_sugared(),
};
match self.expected_found_str(pretty_exp_found) {
Some((expected, found, _, _)) if expected == found => {
Expand All @@ -2233,8 +2233,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
}
infer::PolyTraitRefs(exp_found) => {
let pretty_exp_found = ty::error::ExpectedFound {
expected: exp_found.expected.print_only_trait_path(),
found: exp_found.found.print_only_trait_path(),
expected: exp_found.expected.print_trait_sugared(),
found: exp_found.found.print_trait_sugared(),
};
match self.expected_found_str(pretty_exp_found) {
Some((expected, found, _, _)) if expected == found => {
Expand Down
46 changes: 45 additions & 1 deletion compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2640,6 +2640,23 @@ impl<'tcx> fmt::Debug for TraitRefPrintOnlyTraitPath<'tcx> {
}
}

/// Wrapper type for `ty::TraitRef` which opts-in to pretty printing only
/// the trait path, and additionally tries to "sugar" `Fn(...)` trait bounds.
#[derive(Copy, Clone, TypeFoldable, TypeVisitable, Lift)]
pub struct TraitRefPrintSugared<'tcx>(ty::TraitRef<'tcx>);

impl<'tcx> rustc_errors::IntoDiagnosticArg for TraitRefPrintSugared<'tcx> {
fn into_diagnostic_arg(self) -> rustc_errors::DiagnosticArgValue<'static> {
self.to_string().into_diagnostic_arg()
}
}

impl<'tcx> fmt::Debug for TraitRefPrintSugared<'tcx> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
fmt::Display::fmt(self, f)
}
}

/// Wrapper type for `ty::TraitRef` which opts-in to pretty printing only
/// the trait name. That is, it will print `Trait` instead of
/// `<T as Trait<U>>`.
Expand All @@ -2657,6 +2674,10 @@ impl<'tcx> ty::TraitRef<'tcx> {
TraitRefPrintOnlyTraitPath(self)
}

pub fn print_trait_sugared(self) -> TraitRefPrintSugared<'tcx> {
TraitRefPrintSugared(self)
}

pub fn print_only_trait_name(self) -> TraitRefPrintOnlyTraitName<'tcx> {
TraitRefPrintOnlyTraitName(self)
}
Expand All @@ -2666,6 +2687,10 @@ impl<'tcx> ty::Binder<'tcx, ty::TraitRef<'tcx>> {
pub fn print_only_trait_path(self) -> ty::Binder<'tcx, TraitRefPrintOnlyTraitPath<'tcx>> {
self.map_bound(|tr| tr.print_only_trait_path())
}

pub fn print_trait_sugared(self) -> ty::Binder<'tcx, TraitRefPrintSugared<'tcx>> {
self.map_bound(|tr| tr.print_trait_sugared())
}
}

#[derive(Copy, Clone, TypeFoldable, TypeVisitable, Lift)]
Expand Down Expand Up @@ -2745,6 +2770,7 @@ forward_display_to_print! {
ty::PolyExistentialTraitRef<'tcx>,
ty::Binder<'tcx, ty::TraitRef<'tcx>>,
ty::Binder<'tcx, TraitRefPrintOnlyTraitPath<'tcx>>,
ty::Binder<'tcx, TraitRefPrintSugared<'tcx>>,
ty::Binder<'tcx, ty::FnSig<'tcx>>,
ty::Binder<'tcx, ty::TraitPredicate<'tcx>>,
ty::Binder<'tcx, TraitPredPrintModifiersAndPath<'tcx>>,
Expand Down Expand Up @@ -2844,6 +2870,24 @@ define_print_and_forward_display! {
p!(print_def_path(self.0.def_id, self.0.args));
}

TraitRefPrintSugared<'tcx> {
if !with_no_queries()
&& let Some(kind) = cx.tcx().fn_trait_kind_from_def_id(self.0.def_id)
&& let ty::Tuple(args) = self.0.args.type_at(1).kind()
{
p!(write("{}", kind.as_str()), "(");
for (i, arg) in args.iter().enumerate() {
if i > 0 {
p!(", ");
}
p!(print(arg));
}
p!(")");
} else {
p!(print_def_path(self.0.def_id, self.0.args));
}
}

TraitRefPrintOnlyTraitName<'tcx> {
p!(print_def_path(self.0.def_id, &[]));
}
Expand Down Expand Up @@ -2892,7 +2936,7 @@ define_print_and_forward_display! {
if let ty::ImplPolarity::Negative = self.polarity {
p!("!");
}
p!(print(self.trait_ref.print_only_trait_path()))
p!(print(self.trait_ref.print_trait_sugared()))
}

ty::ProjectionPredicate<'tcx> {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ fn overlap<'tcx>(
let trait_ref = infcx.resolve_vars_if_possible(trait_ref);
format!(
"of `{}` for `{}`",
trait_ref.print_only_trait_path(),
trait_ref.print_trait_sugared(),
trait_ref.self_ty()
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,14 +184,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
flags.push((sym::cause, Some("MainFunctionType".to_string())));
}

if let Some(kind) = self.tcx.fn_trait_kind_from_def_id(trait_ref.def_id)
&& let ty::Tuple(args) = trait_ref.args.type_at(1).kind()
{
let args = args.iter().map(|ty| ty.to_string()).collect::<Vec<_>>().join(", ");
flags.push((sym::Trait, Some(format!("{}({args})", kind.as_str()))));
} else {
flags.push((sym::Trait, Some(trait_ref.print_only_trait_path().to_string())));
}
flags.push((sym::Trait, Some(trait_ref.print_trait_sugared().to_string())));

// Add all types without trimmed paths or visible paths, ensuring they end up with
// their "canonical" def path.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
span.shrink_to_hi(),
format!(
"the trait `{}` is implemented for fn pointer `{}`, try casting using `as`",
cand.print_only_trait_path(),
cand.print_trait_sugared(),
cand.self_ty(),
),
format!(" as {}", cand.self_ty()),
Expand Down Expand Up @@ -1785,7 +1785,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
ct_op: |ct| ct.normalize(self.tcx, ty::ParamEnv::empty()),
});
err.highlighted_help(vec![
(format!("the trait `{}` ", cand.print_only_trait_path()), Style::NoStyle),
(format!("the trait `{}` ", cand.print_trait_sugared()), Style::NoStyle),
("is".to_string(), Style::Highlight),
(" implemented for `".to_string(), Style::NoStyle),
(cand.self_ty().to_string(), Style::Highlight),
Expand Down Expand Up @@ -1821,7 +1821,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
_ => (" implemented for `", ""),
};
err.highlighted_help(vec![
(format!("the trait `{}` ", cand.print_only_trait_path()), Style::NoStyle),
(format!("the trait `{}` ", cand.print_trait_sugared()), Style::NoStyle),
("is".to_string(), Style::Highlight),
(desc.to_string(), Style::NoStyle),
(cand.self_ty().to_string(), Style::Highlight),
Expand Down Expand Up @@ -1854,7 +1854,7 @@ impl<'tcx> InferCtxtPrivExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
let end = if candidates.len() <= 9 { candidates.len() } else { 8 };
err.help(format!(
"the following {other}types implement trait `{}`:{}{}",
trait_ref.print_only_trait_path(),
trait_ref.print_trait_sugared(),
candidates[..end].join(""),
if candidates.len() > 9 {
format!("\nand {} others", candidates.len() - 8)
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl<'tcx> IntercrateAmbiguityCause<'tcx> {
IntercrateAmbiguityCause::DownstreamCrate { trait_ref, self_ty } => {
format!(
"downstream crates may implement trait `{trait_desc}`{self_desc}",
trait_desc = trait_ref.print_only_trait_path(),
trait_desc = trait_ref.print_trait_sugared(),
self_desc = if let Some(self_ty) = self_ty {
format!(" for type `{self_ty}`")
} else {
Expand All @@ -90,7 +90,7 @@ impl<'tcx> IntercrateAmbiguityCause<'tcx> {
format!(
"upstream crates may add a new impl of trait `{trait_desc}`{self_desc} \
in future versions",
trait_desc = trait_ref.print_only_trait_path(),
trait_desc = trait_ref.print_trait_sugared(),
self_desc = if let Some(self_ty) = self_ty {
format!(" for type `{self_ty}`")
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ fn report_conflicting_impls<'tcx>(
let msg = DelayDm(|| {
format!(
"conflicting implementations of trait `{}`{}{}",
overlap.trait_ref.print_only_trait_path(),
overlap.trait_ref.print_trait_sugared(),
overlap.self_ty.map_or_else(String::new, |ty| format!(" for type `{ty}`")),
match used_to_be_allowed {
Some(FutureCompatOverlapErrorKind::Issue33140) => ": (E0119)",
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/future_not_send.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl<'tcx> LateLintPass<'tcx> for FutureNotSend {
db.note(format!(
"`{}` doesn't implement `{}`",
trait_pred.self_ty(),
trait_pred.trait_ref.print_only_trait_path(),
trait_pred.trait_ref.print_trait_sugared(),
));
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/error-codes/E0401.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ error[E0283]: type annotations needed
LL | bfnr(x);
| ^^^^ cannot infer type of the type parameter `W` declared on the function `bfnr`
|
= note: multiple `impl`s satisfying `_: Fn<()>` found in the following crates: `alloc`, `core`:
= note: multiple `impl`s satisfying `_: Fn()` found in the following crates: `alloc`, `core`:
- impl<A, F> Fn<A> for &F
where A: Tuple, F: Fn<A>, F: ?Sized;
- impl<Args, F, A> Fn<Args> for Box<F, A>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error[E0308]: mismatched types
LL | foo(bar, "string", |s| s.len() == 5);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
|
= note: expected trait `for<'a, 'b> FnOnce<(&'a &'b str,)>`
found trait `for<'a> FnOnce<(&'a &str,)>`
= note: expected trait `for<'a, 'b> FnOnce(&'a &'b str)`
found trait `for<'a> FnOnce(&'a &str)`
note: this closure does not fulfill the lifetime requirements
--> $DIR/issue-71955.rs:45:24
|
Expand All @@ -23,8 +23,8 @@ error[E0308]: mismatched types
LL | foo(bar, "string", |s| s.len() == 5);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
|
= note: expected trait `for<'a, 'b> FnOnce<(&'a &'b str,)>`
found trait `for<'a> FnOnce<(&'a &str,)>`
= note: expected trait `for<'a, 'b> FnOnce(&'a &'b str)`
found trait `for<'a> FnOnce(&'a &str)`
note: this closure does not fulfill the lifetime requirements
--> $DIR/issue-71955.rs:45:24
|
Expand All @@ -42,8 +42,8 @@ error[E0308]: mismatched types
LL | foo(baz, "string", |s| s.0.len() == 5);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
|
= note: expected trait `for<'a, 'b> FnOnce<(&'a Wrapper<'b>,)>`
found trait `for<'a> FnOnce<(&'a Wrapper<'_>,)>`
= note: expected trait `for<'a, 'b> FnOnce(&'a Wrapper<'b>)`
found trait `for<'a> FnOnce(&'a Wrapper<'_>)`
note: this closure does not fulfill the lifetime requirements
--> $DIR/issue-71955.rs:48:24
|
Expand All @@ -61,8 +61,8 @@ error[E0308]: mismatched types
LL | foo(baz, "string", |s| s.0.len() == 5);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
|
= note: expected trait `for<'a, 'b> FnOnce<(&'a Wrapper<'b>,)>`
found trait `for<'a> FnOnce<(&'a Wrapper<'_>,)>`
= note: expected trait `for<'a, 'b> FnOnce(&'a Wrapper<'b>)`
found trait `for<'a> FnOnce(&'a Wrapper<'_>)`
note: this closure does not fulfill the lifetime requirements
--> $DIR/issue-71955.rs:48:24
|
Expand Down
16 changes: 8 additions & 8 deletions tests/ui/lifetimes/issue-105675.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error[E0308]: mismatched types
LL | thing(f);
| ^^^^^^^^ one type is more general than the other
|
= note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32, u32)>`
found trait `for<'a> FnOnce<(&u32, &'a u32, u32)>`
= note: expected trait `for<'a, 'b> FnOnce(&'a u32, &'b u32, u32)`
found trait `for<'a> FnOnce(&u32, &'a u32, u32)`
note: this closure does not fulfill the lifetime requirements
--> $DIR/issue-105675.rs:4:13
|
Expand All @@ -27,8 +27,8 @@ error[E0308]: mismatched types
LL | thing(f);
| ^^^^^^^^ one type is more general than the other
|
= note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32, u32)>`
found trait `for<'a> FnOnce<(&u32, &'a u32, u32)>`
= note: expected trait `for<'a, 'b> FnOnce(&'a u32, &'b u32, u32)`
found trait `for<'a> FnOnce(&u32, &'a u32, u32)`
note: this closure does not fulfill the lifetime requirements
--> $DIR/issue-105675.rs:4:13
|
Expand All @@ -46,8 +46,8 @@ error[E0308]: mismatched types
LL | thing(f);
| ^^^^^^^^ one type is more general than the other
|
= note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32, u32)>`
found trait `FnOnce<(&u32, &u32, u32)>`
= note: expected trait `for<'a, 'b> FnOnce(&'a u32, &'b u32, u32)`
found trait `FnOnce(&u32, &u32, u32)`
note: this closure does not fulfill the lifetime requirements
--> $DIR/issue-105675.rs:8:13
|
Expand All @@ -69,8 +69,8 @@ error[E0308]: mismatched types
LL | thing(f);
| ^^^^^^^^ one type is more general than the other
|
= note: expected trait `for<'a, 'b> FnOnce<(&'a u32, &'b u32, u32)>`
found trait `FnOnce<(&u32, &u32, u32)>`
= note: expected trait `for<'a, 'b> FnOnce(&'a u32, &'b u32, u32)`
found trait `FnOnce(&u32, &u32, u32)`
note: this closure does not fulfill the lifetime requirements
--> $DIR/issue-105675.rs:8:13
|
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/lifetimes/issue-79187-2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ error[E0308]: mismatched types
LL | take_foo(|a| a);
| ^^^^^^^^^^^^^^^ one type is more general than the other
|
= note: expected trait `for<'a> Fn<(&'a i32,)>`
found trait `Fn<(&i32,)>`
= note: expected trait `for<'a> Fn(&'a i32)`
found trait `Fn(&i32)`
note: this closure does not fulfill the lifetime requirements
--> $DIR/issue-79187-2.rs:8:14
|
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/lifetimes/issue-79187.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ error[E0308]: mismatched types
LL | thing(f);
| ^^^^^^^^ one type is more general than the other
|
= note: expected trait `for<'a> FnOnce<(&'a u32,)>`
found trait `FnOnce<(&u32,)>`
= note: expected trait `for<'a> FnOnce(&'a u32)`
found trait `FnOnce(&u32)`
note: this closure does not fulfill the lifetime requirements
--> $DIR/issue-79187.rs:4:13
|
Expand Down
Loading

0 comments on commit d538d16

Please sign in to comment.