Skip to content

Commit

Permalink
Auto merge of #106399 - estebank:type-err-span-label, r=nagisa
Browse files Browse the repository at this point in the history
Modify primary span label for E0308

Looking at the reactions to https://hachyderm.io/`@ekuber/109622160673605438,` a lot of people seem to have trouble understanding the current output, where the primary span label on type errors talks about the specific types that diverged, but these can be deeply nested type parameters. Because of that we could see "expected i32, found u32" in the label while the note said "expected Vec<i32>, found Vec<u32>". This understandably confuses people. I believe that once people learn to read these errors it starts to make more sense, but this PR changes the output to be more in line with what people might expect, without sacrificing terseness.

Fix #68220.
  • Loading branch information
bors committed Jan 31, 2023
2 parents a64ef7d + 449dfc6 commit f361413
Show file tree
Hide file tree
Showing 428 changed files with 1,110 additions and 1,053 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,7 @@ fn check_opaque_meets_bounds<'tcx>(
match ocx.eq(&misc_cause, param_env, opaque_ty, hidden_ty) {
Ok(()) => {}
Err(ty_err) => {
let ty_err = ty_err.to_string(tcx);
tcx.sess.delay_span_bug(
span,
&format!("could not unify `{hidden_ty}` with revealed type:\n{ty_err}"),
Expand Down
36 changes: 29 additions & 7 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let mode = no_match_data.mode;
let tcx = self.tcx;
let rcvr_ty = self.resolve_vars_if_possible(rcvr_ty);
let ty_str = with_forced_trimmed_paths!(self.ty_to_string(rcvr_ty));
let (ty_str, ty_file) = tcx.short_ty_string(rcvr_ty);
let short_ty_str = with_forced_trimmed_paths!(rcvr_ty.to_string());
let is_method = mode == Mode::MethodCall;
let unsatisfied_predicates = &no_match_data.unsatisfied_predicates;
let similar_candidate = no_match_data.similar_candidate;
Expand All @@ -276,11 +277,24 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
};

if self.suggest_wrapping_range_with_parens(tcx, rcvr_ty, source, span, item_name, &ty_str)
|| self.suggest_constraining_numerical_ty(
tcx, rcvr_ty, source, span, item_kind, item_name, &ty_str,
)
{
// We could pass the file for long types into these two, but it isn't strictly necessary
// given how targetted they are.
if self.suggest_wrapping_range_with_parens(
tcx,
rcvr_ty,
source,
span,
item_name,
&short_ty_str,
) || self.suggest_constraining_numerical_ty(
tcx,
rcvr_ty,
source,
span,
item_kind,
item_name,
&short_ty_str,
) {
return None;
}
span = item_name.span;
Expand Down Expand Up @@ -319,6 +333,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
rcvr_ty.prefix_string(self.tcx),
ty_str_reported,
);
let ty_str = if short_ty_str.len() < ty_str.len() && ty_str.len() > 10 {
short_ty_str
} else {
ty_str
};
if let Some(file) = ty_file {
err.note(&format!("the full type name has been written to '{}'", file.display(),));
}
if rcvr_ty.references_error() {
err.downgrade_to_delayed_bug();
}
Expand Down Expand Up @@ -826,7 +848,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let primary_message = primary_message.unwrap_or_else(|| {
format!(
"the {item_kind} `{item_name}` exists for {actual_prefix} `{ty_str}`, \
but its trait bounds were not satisfied"
but its trait bounds were not satisfied"
)
});
err.set_primary_message(&primary_message);
Expand Down
34 changes: 31 additions & 3 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::lang_items::LangItem;
use rustc_hir::Node;
use rustc_middle::dep_graph::DepContext;
use rustc_middle::ty::print::with_forced_trimmed_paths;
use rustc_middle::ty::relate::{self, RelateResult, TypeRelation};
use rustc_middle::ty::{
self, error::TypeError, List, Region, Ty, TyCtxt, TypeFoldable, TypeSuperVisitable,
Expand Down Expand Up @@ -1612,16 +1613,31 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
{
format!("expected this to be `{}`", expected)
} else {
terr.to_string()
terr.to_string(self.tcx).to_string()
};
label_or_note(sp, &terr);
label_or_note(span, &msg);
} else {
label_or_note(span, &terr.to_string());
label_or_note(span, &terr.to_string(self.tcx));
label_or_note(sp, &msg);
}
} else {
label_or_note(span, &terr.to_string());
if let Some(values) = values
&& let Some((e, f)) = values.ty()
&& let TypeError::ArgumentSorts(..) | TypeError::Sorts(_) = terr
{
let e = self.tcx.erase_regions(e);
let f = self.tcx.erase_regions(f);
let expected = with_forced_trimmed_paths!(e.sort_string(self.tcx));
let found = with_forced_trimmed_paths!(f.sort_string(self.tcx));
if expected == found {
label_or_note(span, &terr.to_string(self.tcx));
} else {
label_or_note(span, &format!("expected {expected}, found {found}"));
}
} else {
label_or_note(span, &terr.to_string(self.tcx));
}
}

if let Some((expected, found, exp_p, found_p)) = expected_found {
Expand Down Expand Up @@ -1849,6 +1865,18 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
self.check_and_note_conflicting_crates(diag, terr);

self.note_and_explain_type_err(diag, terr, cause, span, cause.body_id.to_def_id());
if let Some(exp_found) = exp_found
&& let exp_found = TypeError::Sorts(exp_found)
&& exp_found != terr
{
self.note_and_explain_type_err(
diag,
exp_found,
cause,
span,
cause.body_id.to_def_id(),
);
}

if let Some(ValuePairs::PolyTraitRefs(exp_found)) = values
&& let ty::Closure(def_id, _) = exp_found.expected.skip_binder().self_ty().kind()
Expand Down
19 changes: 13 additions & 6 deletions compiler/rustc_infer/src/infer/error_reporting/note_and_explain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,25 +137,25 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
diag.help(
"given a type parameter `T` and a method `foo`:
```
trait Trait<T> { fn foo(&tcx) -> T; }
trait Trait<T> { fn foo(&self) -> T; }
```
the only ways to implement method `foo` are:
- constrain `T` with an explicit type:
```
impl Trait<String> for X {
fn foo(&tcx) -> String { String::new() }
fn foo(&self) -> String { String::new() }
}
```
- add a trait bound to `T` and call a method on that trait that returns `Self`:
```
impl<T: std::default::Default> Trait<T> for X {
fn foo(&tcx) -> T { <T as std::default::Default>::default() }
fn foo(&self) -> T { <T as std::default::Default>::default() }
}
```
- change `foo` to return an argument of type `T`:
```
impl<T> Trait<T> for X {
fn foo(&tcx, x: T) -> T { x }
fn foo(&self, x: T) -> T { x }
}
```",
);
Expand Down Expand Up @@ -218,6 +218,13 @@ impl<T> Trait<T> for X {
);
}
}
(ty::FnPtr(_), ty::FnDef(def, _))
if let hir::def::DefKind::Fn = tcx.def_kind(def) => {
diag.note(
"when the arguments and return types match, functions can be coerced \
to function pointers",
);
}
_ => {}
}
debug!(
Expand Down Expand Up @@ -389,14 +396,14 @@ impl<T> Trait<T> for X {
```
trait Trait {
type T;
fn foo(&tcx) -> Self::T;
fn foo(&self) -> Self::T;
}
```
the only way of implementing method `foo` is to constrain `T` with an explicit associated type:
```
impl Trait for X {
type T = String;
fn foo(&tcx) -> Self::T { String::new() }
fn foo(&self) -> Self::T { String::new() }
}
```",
);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/infer/error_reporting/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
(msg, sug)
}
};
diag.span_suggestion(span, msg, sug, Applicability::MaybeIncorrect);
diag.span_suggestion_verbose(span, msg, sug, Applicability::MaybeIncorrect);
}
(ty::FnDef(did1, substs1), ty::FnDef(did2, substs2)) => {
let expected_sig =
Expand Down
Loading

0 comments on commit f361413

Please sign in to comment.