Skip to content

Commit

Permalink
Auto merge of #132147 - estebank:long-types-2, r=davidtwco
Browse files Browse the repository at this point in the history
Tweak E0277 output when a candidate is available

*Follow up to #132086.*

Go from

```
error[E0277]: the trait bound `Then<Ignored<chumsky::combinator::Filter<chumsky::primitive::Any<&str, chumsky::extra::Full<EmptyErr, (), ()>>, {closure@src/main.rs:9:17: 9:27}>, char>, chumsky::combinator::Map<impl CSTParser<'a, O>, O, {closure@src/main.rs:11:24: 11:27}>, (), (), chumsky::extra::Full<EmptyErr, (), ()>>: CSTParser<'a>` is not satisfied
 --> src/main.rs:7:50
  |
7 | fn leaf<'a, O>(parser: impl CSTParser<'a, O>) -> impl CSTParser<'a, ()> {
  |                                                  ^^^^^^^^^^^^^^^^^^^^^^ the trait `chumsky::private::ParserSealed<'_, &str, (), chumsky::extra::Full<EmptyErr, (), ()>>` is not implemented for `Then<Ignored<Filter<Any<&str, ...>, ...>, ...>, ..., ..., ..., ...>`, which is required by `Then<Ignored<chumsky::combinator::Filter<chumsky::primitive::Any<&str, chumsky::extra::Full<EmptyErr, (), ()>>, {closure@src/main.rs:9:17: 9:27}>, char>, chumsky::combinator::Map<impl CSTParser<'a, O>, O, {closure@src/main.rs:11:24: 11:27}>, (), (), chumsky::extra::Full<EmptyErr, (), ()>>: CSTParser<'a>`
  |
  = help: the trait `chumsky::private::ParserSealed<'_, &'a str, ((), ()), chumsky::extra::Full<EmptyErr, (), ()>>` is implemented for `Then<Ignored<chumsky::combinator::Filter<chumsky::primitive::Any<&str, chumsky::extra::Full<EmptyErr, (), ()>>, {closure@src/main.rs:9:17: 9:27}>, char>, chumsky::combinator::Map<impl CSTParser<'a, O>, O, {closure@src/main.rs:11:24: 11:27}>, (), (), chumsky::extra::Full<EmptyErr, (), ()>>`
  = help: for that trait implementation, expected `((), ())`, found `()`
  = note: required for `Then<Ignored<Filter<Any<&str, ...>, ...>, ...>, ..., ..., ..., ...>` to implement `Parser<'_, &str, ()>`
note: required for `Then<Ignored<Filter<Any<&str, ...>, ...>, ...>, ..., ..., ..., ...>` to implement `CSTParser<'a>`
 --> src/main.rs:5:16
  |
5 | impl<'a, O, T> CSTParser<'a, O> for T where T: Parser<'a, &'a str, O> {}
  |                ^^^^^^^^^^^^^^^^     ^          ---------------------- unsatisfied trait bound introduced here
  = note: the full name for the type has been written to '/home/gh-estebank/longlong/target/debug/deps/longlong-0008f9a4f2023b08.long-type-13239977239800463552.txt'
  = note: consider using `--verbose` to print the full type name to the console
  = note: the full name for the type has been written to '/home/gh-estebank/longlong/target/debug/deps/longlong-0008f9a4f2023b08.long-type-13239977239800463552.txt'
  = note: consider using `--verbose` to print the full type name to the console
```

to

```
error[E0277]: the trait bound `Then<Ignored<chumsky::combinator::Filter<chumsky::primitive::Any<&str, chumsky::extra::Full<EmptyErr, (), ()>>, {closure@src/main.rs:9:17: 9:27}>, char>, chumsky::combinator::Map<impl CSTParser<'a, O>, O, {closure@src/main.rs:11:24: 11:27}>, (), (), chumsky::extra::Full<EmptyErr, (), ()>>: CSTParser<'a>` is not satisfied
  --> src/main.rs:7:50
   |
7  | fn leaf<'a, O>(parser: impl CSTParser<'a, O>) -> impl CSTParser<'a, ()> {
   |                                                  ^^^^^^^^^^^^^^^^^^^^^^ unsatisfied trait bound
...
11 |     ws.then(parser.map(|_| ()))
   |     --------------------------- return type was inferred to be `Then<Ignored<..., ...>, ..., ..., ..., ...>` here
   |
   = help: the trait `ParserSealed<'_, &_, (), Full<_, _, _>>` is not implemented for `Then<Ignored<..., ...>, ..., ..., ..., ...>`
           but trait `ParserSealed<'_, &'a _, ((), ()), Full<_, _, _>>` is implemented for it
   = help: for that trait implementation, expected `((), ())`, found `()`
   = note: required for `Then<Ignored<..., ...>, ..., ..., ..., ...>` to implement `Parser<'_, &str, ()>`
note: required for `Then<Ignored<..., ...>, ..., ..., ..., ...>` to implement `CSTParser<'a>`
  --> src/main.rs:5:16
   |
5  | impl<'a, O, T> CSTParser<'a, O> for T where T: Parser<'a, &'a str, O> {}
   |                ^^^^^^^^^^^^^^^^     ^          ---------------------- unsatisfied trait bound introduced here
   = note: the full name for the type has been written to '/home/gh-estebank/longlong/target/debug/deps/longlong-df9d52be87eada65.long-type-1337037744507305372.txt'
   = note: consider using `--verbose` to print the full type name to the console
```

* Remove redundant wording
* Introduce trait diff highlighting logic and use it
* Fix incorrect "long type written to path" logic (can be split off)
* Point at tail expression in more cases in E0277
* Avoid long primary span labels in E0277 by moving them to a `help`

Fix #132013.

There are individual commits that can be their own PR. If the review load is too big, happy to split them off.
  • Loading branch information
bors committed Nov 2, 2024
2 parents 00ed73c + 7e0d3ed commit b3f75cc
Show file tree
Hide file tree
Showing 79 changed files with 487 additions and 227 deletions.
21 changes: 18 additions & 3 deletions compiler/rustc_middle/src/ty/error.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::borrow::Cow;
use std::fs::File;
use std::hash::{DefaultHasher, Hash, Hasher};
use std::io::{Read, Write};
use std::path::PathBuf;

use rustc_errors::pluralize;
Expand Down Expand Up @@ -250,8 +252,8 @@ impl<'tcx> TyCtxt<'tcx> {
}

let width = self.sess.diagnostic_width();
let length_limit = width.saturating_sub(30);
if regular.len() <= width {
let length_limit = width / 2;
if regular.len() <= width * 2 / 3 {
return regular;
}
let short = self.ty_string_with_limit(ty, length_limit);
Expand All @@ -265,7 +267,20 @@ impl<'tcx> TyCtxt<'tcx> {
*path = Some(path.take().unwrap_or_else(|| {
self.output_filenames(()).temp_path_ext(&format!("long-type-{hash}.txt"), None)
}));
match std::fs::write(path.as_ref().unwrap(), &format!("{regular}\n")) {
let Ok(mut file) =
File::options().create(true).read(true).append(true).open(&path.as_ref().unwrap())
else {
return regular;
};

// Do not write the same type to the file multiple times.
let mut contents = String::new();
let _ = file.read_to_string(&mut contents);
if let Some(_) = contents.lines().find(|line| line == &regular) {
return short;
}

match write!(file, "{regular}\n") {
Ok(_) => short,
Err(_) => regular,
}
Expand Down
61 changes: 61 additions & 0 deletions compiler/rustc_trait_selection/src/error_reporting/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,67 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
values
}

pub fn cmp_traits(
&self,
def_id1: DefId,
args1: &[ty::GenericArg<'tcx>],
def_id2: DefId,
args2: &[ty::GenericArg<'tcx>],
) -> (DiagStyledString, DiagStyledString) {
let mut values = (DiagStyledString::new(), DiagStyledString::new());

if def_id1 != def_id2 {
values.0.push_highlighted(self.tcx.def_path_str(def_id1).as_str());
values.1.push_highlighted(self.tcx.def_path_str(def_id2).as_str());
} else {
values.0.push_normal(self.tcx.item_name(def_id1).as_str());
values.1.push_normal(self.tcx.item_name(def_id2).as_str());
}

if args1.len() != args2.len() {
let (pre, post) = if args1.len() > 0 { ("<", ">") } else { ("", "") };
values.0.push_normal(format!(
"{pre}{}{post}",
args1.iter().map(|a| a.to_string()).collect::<Vec<_>>().join(", ")
));
let (pre, post) = if args2.len() > 0 { ("<", ">") } else { ("", "") };
values.1.push_normal(format!(
"{pre}{}{post}",
args2.iter().map(|a| a.to_string()).collect::<Vec<_>>().join(", ")
));
return values;
}

if args1.len() > 0 {
values.0.push_normal("<");
values.1.push_normal("<");
}
for (i, (a, b)) in std::iter::zip(args1, args2).enumerate() {
let a_str = a.to_string();
let b_str = b.to_string();
if let (Some(a), Some(b)) = (a.as_type(), b.as_type()) {
let (a, b) = self.cmp(a, b);
values.0.0.extend(a.0);
values.1.0.extend(b.0);
} else if a_str != b_str {
values.0.push_highlighted(a_str);
values.1.push_highlighted(b_str);
} else {
values.0.push_normal(a_str);
values.1.push_normal(b_str);
}
if i + 1 < args1.len() {
values.0.push_normal(", ");
values.1.push_normal(", ");
}
}
if args1.len() > 0 {
values.0.push_normal(">");
values.1.push_normal(">");
}
values
}

/// Compares two given types, eliding parts that are the same between them and highlighting
/// relevant differences, and return two representation of those types for highlighted printing.
pub fn cmp(&self, t1: Ty<'tcx>, t2: Ty<'tcx>) -> (DiagStyledString, DiagStyledString) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::unord::UnordSet;
use rustc_errors::codes::*;
use rustc_errors::{
Applicability, Diag, ErrorGuaranteed, MultiSpan, StashKey, StringPart, Suggestions, pluralize,
struct_span_code_err,
Applicability, Diag, ErrorGuaranteed, Level, MultiSpan, StashKey, StringPart, Suggestions,
pluralize, struct_span_code_err,
};
use rustc_hir::def::Namespace;
use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId};
Expand Down Expand Up @@ -328,6 +328,11 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
}
} else if let Some(custom_explanation) = safe_transmute_explanation {
err.span_label(span, custom_explanation);
} else if explanation.len() > self.tcx.sess.diagnostic_width() {
// Really long types don't look good as span labels, instead move it
// to a `help`.
err.span_label(span, "unsatisfied trait bound");
err.help(explanation);
} else {
err.span_label(span, explanation);
}
Expand Down Expand Up @@ -1832,21 +1837,81 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
if impl_trait_ref.references_error() {
return false;
}
let self_ty = impl_trait_ref.self_ty().to_string();
err.highlighted_help(vec![
StringPart::normal(format!(
"the trait `{}` ",
impl_trait_ref.print_trait_sugared()
)),
StringPart::highlighted("is"),

if let [child, ..] = &err.children[..]
&& child.level == Level::Help
&& let Some(line) = child.messages.get(0)
&& let Some(line) = line.0.as_str()
&& line.starts_with("the trait")
&& line.contains("is not implemented for")
{
// HACK(estebank): we remove the pre-existing
// "the trait `X` is not implemented for" note, which only happens if there
// was a custom label. We do this because we want that note to always be the
// first, and making this logic run earlier will get tricky. For now, we
// instead keep the logic the same and modify the already constructed error
// to avoid the wording duplication.
err.children.remove(0);
}

let traits = self.cmp_traits(
obligation_trait_ref.def_id,
&obligation_trait_ref.args[1..],
impl_trait_ref.def_id,
&impl_trait_ref.args[1..],
);
let traits_content = (traits.0.content(), traits.1.content());
let types = self.cmp(obligation_trait_ref.self_ty(), impl_trait_ref.self_ty());
let types_content = (types.0.content(), types.1.content());
let mut msg = vec![StringPart::normal("the trait `")];
if traits_content.0 == traits_content.1 {
msg.push(StringPart::normal(
impl_trait_ref.print_trait_sugared().to_string(),
));
} else {
msg.extend(traits.0.0);
}
msg.extend([
StringPart::normal("` "),
StringPart::highlighted("is not"),
StringPart::normal(" implemented for `"),
if let [TypeError::Sorts(_)] = &terrs[..] {
StringPart::normal(self_ty)
} else {
StringPart::highlighted(self_ty)
},
StringPart::normal("`"),
]);
if types_content.0 == types_content.1 {
let ty =
self.tcx.short_ty_string(obligation_trait_ref.self_ty(), &mut None);
msg.push(StringPart::normal(ty));
} else {
msg.extend(types.0.0);
}
msg.push(StringPart::normal("`"));
if types_content.0 == types_content.1 {
msg.push(StringPart::normal("\nbut trait `"));
msg.extend(traits.1.0);
msg.extend([
StringPart::normal("` "),
StringPart::highlighted("is"),
StringPart::normal(" implemented for it"),
]);
} else if traits_content.0 == traits_content.1 {
msg.extend([
StringPart::normal("\nbut it "),
StringPart::highlighted("is"),
StringPart::normal(" implemented for `"),
]);
msg.extend(types.1.0);
msg.push(StringPart::normal("`"));
} else {
msg.push(StringPart::normal("\nbut trait `"));
msg.extend(traits.1.0);
msg.extend([
StringPart::normal("` "),
StringPart::highlighted("is"),
StringPart::normal(" implemented for `"),
]);
msg.extend(types.1.0);
msg.push(StringPart::normal("`"));
}
err.highlighted_help(msg);

if let [TypeError::Sorts(exp_found)] = &terrs[..] {
let exp_found = self.resolve_vars_if_possible(*exp_found);
Expand Down Expand Up @@ -2475,12 +2540,16 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
&& self.tcx.trait_impls_of(trait_def_id).is_empty()
&& !self.tcx.trait_is_auto(trait_def_id)
&& !self.tcx.trait_is_alias(trait_def_id)
&& trait_predicate.polarity() == ty::PredicatePolarity::Positive
{
err.span_help(
self.tcx.def_span(trait_def_id),
crate::fluent_generated::trait_selection_trait_has_no_impls,
);
} else if !suggested && !unsatisfied_const {
} else if !suggested
&& !unsatisfied_const
&& trait_predicate.polarity() == ty::PredicatePolarity::Positive
{
// Can't show anything else useful, try to find similar impls.
let impl_candidates = self.find_similar_impl_candidates(trait_predicate);
if !self.report_similar_impl_candidates(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3563,17 +3563,34 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
)]);
}
ObligationCauseCode::OpaqueReturnType(expr_info) => {
if let Some((expr_ty, hir_id)) = expr_info {
let expr_ty = self.tcx.short_ty_string(expr_ty, long_ty_file);
let expr = self.infcx.tcx.hir().expect_expr(hir_id);
err.span_label(
expr.span,
with_forced_trimmed_paths!(format!(
"return type was inferred to be `{expr_ty}` here",
)),
);
suggest_remove_deref(err, &expr);
}
let (expr_ty, expr) = if let Some((expr_ty, hir_id)) = expr_info {
let expr_ty = tcx.short_ty_string(expr_ty, long_ty_file);
let expr = tcx.hir().expect_expr(hir_id);
(expr_ty, expr)
} else if let Some(body_id) = tcx.hir_node_by_def_id(body_id).body_id()
&& let body = tcx.hir().body(body_id)
&& let hir::ExprKind::Block(block, _) = body.value.kind
&& let Some(expr) = block.expr
&& let Some(expr_ty) = self
.typeck_results
.as_ref()
.and_then(|typeck| typeck.node_type_opt(expr.hir_id))
&& let Some(pred) = predicate.as_clause()
&& let ty::ClauseKind::Trait(pred) = pred.kind().skip_binder()
&& self.can_eq(param_env, pred.self_ty(), expr_ty)
{
let expr_ty = tcx.short_ty_string(expr_ty, long_ty_file);
(expr_ty, expr)
} else {
return;
};
err.span_label(
expr.span,
with_forced_trimmed_paths!(format!(
"return type was inferred to be `{expr_ty}` here",
)),
);
suggest_remove_deref(err, &expr);
}
}
}
Expand Down Expand Up @@ -3680,6 +3697,9 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
err: &mut Diag<'_>,
trait_pred: ty::PolyTraitPredicate<'tcx>,
) {
if trait_pred.polarity() == ty::PredicatePolarity::Negative {
return;
}
let Some(diagnostic_name) = self.tcx.get_diagnostic_name(trait_pred.def_id()) else {
return;
};
Expand Down
3 changes: 2 additions & 1 deletion tests/ui/async-await/async-closures/not-clone-closure.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ error[E0277]: the trait bound `NotClonableUpvar: Clone` is not satisfied in `{as
--> $DIR/not-clone-closure.rs:32:15
|
LL | not_clone.clone();
| ^^^^^ within `{async closure@$DIR/not-clone-closure.rs:29:21: 29:34}`, the trait `Clone` is not implemented for `NotClonableUpvar`
| ^^^^^ unsatisfied trait bound
|
= help: within `{async closure@$DIR/not-clone-closure.rs:29:21: 29:34}`, the trait `Clone` is not implemented for `NotClonableUpvar`
note: required because it's used within this closure
--> $DIR/not-clone-closure.rs:29:21
|
Expand Down
3 changes: 3 additions & 0 deletions tests/ui/async-await/async-error-span.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ error[E0277]: `()` is not a future
|
LL | fn get_future() -> impl Future<Output = ()> {
| ^^^^^^^^^^^^^^^^^^^^^^^^ `()` is not a future
LL |
LL | panic!()
| -------- return type was inferred to be `_` here
|
= help: the trait `Future` is not implemented for `()`

Expand Down
1 change: 1 addition & 0 deletions tests/ui/async-await/coroutine-not-future.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//@ edition:2018
//@compile-flags: --diagnostic-width=300
#![feature(coroutines, coroutine_trait, stmt_expr_attributes)]

use std::future::Future;
Expand Down
Loading

0 comments on commit b3f75cc

Please sign in to comment.