Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit c905e9d

Browse files
Dylan-DPCDylan-DPC
authored
Rollup merge of rust-lang#84014 - estebank:cool-bears-hot-tip, r=varkor
Improve trait/impl method discrepancy errors * Use more accurate spans * Clean up some code by removing previous hack * Provide structured suggestions Structured suggestions are particularly useful for cases where arbitrary self types are used, like in custom `Future`s, because the way to write `self: Pin<&mut Self>` is not necessarily self-evident when first encountered.
2 parents b6780b3 + bb502c4 commit c905e9d

File tree

17 files changed

+258
-117
lines changed

17 files changed

+258
-117
lines changed

‎compiler/rustc_middle/src/ty/error.rs‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ pub enum TypeError<'tcx> {
3636
UnsafetyMismatch(ExpectedFound<hir::Unsafety>),
3737
AbiMismatch(ExpectedFound<abi::Abi>),
3838
Mutability,
39+
ArgumentMutability(usize),
3940
TupleSize(ExpectedFound<usize>),
4041
FixedArraySize(ExpectedFound<u64>),
4142
ArgCount,
@@ -46,6 +47,7 @@ pub enum TypeError<'tcx> {
4647
RegionsPlaceholderMismatch,
4748

4849
Sorts(ExpectedFound<Ty<'tcx>>),
50+
ArgumentSorts(ExpectedFound<Ty<'tcx>>, usize),
4951
IntMismatch(ExpectedFound<ty::IntVarValue>),
5052
FloatMismatch(ExpectedFound<ty::FloatTy>),
5153
Traits(ExpectedFound<DefId>),
@@ -110,7 +112,7 @@ impl<'tcx> fmt::Display for TypeError<'tcx> {
110112
AbiMismatch(values) => {
111113
write!(f, "expected {} fn, found {} fn", values.expected, values.found)
112114
}
113-
Mutability => write!(f, "types differ in mutability"),
115+
ArgumentMutability(_) | Mutability => write!(f, "types differ in mutability"),
114116
TupleSize(values) => write!(
115117
f,
116118
"expected a tuple with {} element{}, \
@@ -142,7 +144,7 @@ impl<'tcx> fmt::Display for TypeError<'tcx> {
142144
br_string(br)
143145
),
144146
RegionsPlaceholderMismatch => write!(f, "one type is more general than the other"),
145-
Sorts(values) => ty::tls::with(|tcx| {
147+
ArgumentSorts(values, _) | Sorts(values) => ty::tls::with(|tcx| {
146148
report_maybe_different(
147149
f,
148150
&values.expected.sort_string(tcx),
@@ -199,10 +201,11 @@ impl<'tcx> TypeError<'tcx> {
199201
use self::TypeError::*;
200202
match self {
201203
CyclicTy(_) | CyclicConst(_) | UnsafetyMismatch(_) | Mismatch | AbiMismatch(_)
202-
| FixedArraySize(_) | Sorts(_) | IntMismatch(_) | FloatMismatch(_)
203-
| VariadicMismatch(_) | TargetFeatureCast(_) => false,
204+
| FixedArraySize(_) | ArgumentSorts(..) | Sorts(_) | IntMismatch(_)
205+
| FloatMismatch(_) | VariadicMismatch(_) | TargetFeatureCast(_) => false,
204206

205207
Mutability
208+
| ArgumentMutability(_)
206209
| TupleSize(_)
207210
| ArgCount
208211
| RegionsDoesNotOutlive(..)
@@ -339,7 +342,7 @@ impl<'tcx> TyCtxt<'tcx> {
339342
use self::TypeError::*;
340343
debug!("note_and_explain_type_err err={:?} cause={:?}", err, cause);
341344
match err {
342-
Sorts(values) => {
345+
ArgumentSorts(values, _) | Sorts(values) => {
343346
match (values.expected.kind(), values.found.kind()) {
344347
(ty::Closure(..), ty::Closure(..)) => {
345348
db.note("no two closures, even if identical, have the same type");

‎compiler/rustc_middle/src/ty/relate.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,12 @@ impl<'tcx> Relate<'tcx> for ty::FnSig<'tcx> {
179179
} else {
180180
relation.relate_with_variance(ty::Contravariant, a, b)
181181
}
182+
})
183+
.enumerate()
184+
.map(|(i, r)| match r {
185+
Err(TypeError::Sorts(exp_found)) => Err(TypeError::ArgumentSorts(exp_found, i)),
186+
Err(TypeError::Mutability) => Err(TypeError::ArgumentMutability(i)),
187+
r => r,
182188
});
183189
Ok(ty::FnSig {
184190
inputs_and_output: tcx.mk_type_list(inputs_and_output)?,

‎compiler/rustc_middle/src/ty/structural_impls.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,7 @@ impl<'a, 'tcx> Lift<'tcx> for ty::error::TypeError<'a> {
587587
UnsafetyMismatch(x) => UnsafetyMismatch(x),
588588
AbiMismatch(x) => AbiMismatch(x),
589589
Mutability => Mutability,
590+
ArgumentMutability(i) => ArgumentMutability(i),
590591
TupleSize(x) => TupleSize(x),
591592
FixedArraySize(x) => FixedArraySize(x),
592593
ArgCount => ArgCount,
@@ -607,6 +608,7 @@ impl<'a, 'tcx> Lift<'tcx> for ty::error::TypeError<'a> {
607608
CyclicTy(t) => return tcx.lift(t).map(|t| CyclicTy(t)),
608609
CyclicConst(ct) => return tcx.lift(ct).map(|ct| CyclicConst(ct)),
609610
ProjectionMismatched(x) => ProjectionMismatched(x),
611+
ArgumentSorts(x, i) => return tcx.lift(x).map(|x| ArgumentSorts(x, i)),
610612
Sorts(x) => return tcx.lift(x).map(Sorts),
611613
ExistentialMismatch(x) => return tcx.lift(x).map(ExistentialMismatch),
612614
ConstMismatch(x) => return tcx.lift(x).map(ConstMismatch),

‎compiler/rustc_typeck/src/check/compare_method.rs‎

Lines changed: 87 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -278,9 +278,8 @@ fn compare_predicate_entailment<'tcx>(
278278
if let Err(terr) = sub_result {
279279
debug!("sub_types failed: impl ty {:?}, trait ty {:?}", impl_fty, trait_fty);
280280

281-
let (impl_err_span, trait_err_span) = extract_spans_for_error_reporting(
282-
&infcx, param_env, &terr, &cause, impl_m, impl_sig, trait_m, trait_sig,
283-
);
281+
let (impl_err_span, trait_err_span) =
282+
extract_spans_for_error_reporting(&infcx, &terr, &cause, impl_m, trait_m);
284283

285284
cause.make_mut().span = impl_err_span;
286285

@@ -291,18 +290,79 @@ fn compare_predicate_entailment<'tcx>(
291290
"method `{}` has an incompatible type for trait",
292291
trait_m.ident
293292
);
294-
if let TypeError::Mutability = terr {
295-
if let Some(trait_err_span) = trait_err_span {
296-
if let Ok(trait_err_str) = tcx.sess.source_map().span_to_snippet(trait_err_span)
293+
match &terr {
294+
TypeError::ArgumentMutability(0) | TypeError::ArgumentSorts(_, 0)
295+
if trait_m.fn_has_self_parameter =>
296+
{
297+
let ty = trait_sig.inputs()[0];
298+
let sugg = match ExplicitSelf::determine(ty, |_| ty == impl_trait_ref.self_ty())
297299
{
300+
ExplicitSelf::ByValue => "self".to_owned(),
301+
ExplicitSelf::ByReference(_, hir::Mutability::Not) => "&self".to_owned(),
302+
ExplicitSelf::ByReference(_, hir::Mutability::Mut) => {
303+
"&mut self".to_owned()
304+
}
305+
_ => format!("self: {}", ty),
306+
};
307+
308+
// When the `impl` receiver is an arbitrary self type, like `self: Box<Self>`, the
309+
// span points only at the type `Box<Self`>, but we want to cover the whole
310+
// argument pattern and type.
311+
let impl_m_hir_id =
312+
tcx.hir().local_def_id_to_hir_id(impl_m.def_id.expect_local());
313+
let span = match tcx.hir().expect_impl_item(impl_m_hir_id).kind {
314+
ImplItemKind::Fn(ref sig, body) => tcx
315+
.hir()
316+
.body_param_names(body)
317+
.zip(sig.decl.inputs.iter())
318+
.map(|(param, ty)| param.span.to(ty.span))
319+
.next()
320+
.unwrap_or(impl_err_span),
321+
_ => bug!("{:?} is not a method", impl_m),
322+
};
323+
324+
diag.span_suggestion(
325+
span,
326+
"change the self-receiver type to match the trait",
327+
sugg,
328+
Applicability::MachineApplicable,
329+
);
330+
}
331+
TypeError::ArgumentMutability(i) | TypeError::ArgumentSorts(_, i) => {
332+
if trait_sig.inputs().len() == *i {
333+
// Suggestion to change output type. We do not suggest in `async` functions
334+
// to avoid complex logic or incorrect output.
335+
let impl_m_hir_id =
336+
tcx.hir().local_def_id_to_hir_id(impl_m.def_id.expect_local());
337+
match tcx.hir().expect_impl_item(impl_m_hir_id).kind {
338+
ImplItemKind::Fn(ref sig, _)
339+
if sig.header.asyncness == hir::IsAsync::NotAsync =>
340+
{
341+
let msg = "change the output type to match the trait";
342+
let ap = Applicability::MachineApplicable;
343+
match sig.decl.output {
344+
hir::FnRetTy::DefaultReturn(sp) => {
345+
let sugg = format!("-> {} ", trait_sig.output());
346+
diag.span_suggestion_verbose(sp, msg, sugg, ap);
347+
}
348+
hir::FnRetTy::Return(hir_ty) => {
349+
let sugg = trait_sig.output().to_string();
350+
diag.span_suggestion(hir_ty.span, msg, sugg, ap);
351+
}
352+
};
353+
}
354+
_ => {}
355+
};
356+
} else if let Some(trait_ty) = trait_sig.inputs().get(*i) {
298357
diag.span_suggestion(
299358
impl_err_span,
300-
"consider changing the mutability to match the trait",
301-
trait_err_str,
359+
"change the parameter type to match the trait",
360+
trait_ty.to_string(),
302361
Applicability::MachineApplicable,
303362
);
304363
}
305364
}
365+
_ => {}
306366
}
307367

308368
infcx.note_type_err(
@@ -385,86 +445,35 @@ fn check_region_bounds_on_impl_item<'tcx>(
385445

386446
fn extract_spans_for_error_reporting<'a, 'tcx>(
387447
infcx: &infer::InferCtxt<'a, 'tcx>,
388-
param_env: ty::ParamEnv<'tcx>,
389448
terr: &TypeError<'_>,
390449
cause: &ObligationCause<'tcx>,
391450
impl_m: &ty::AssocItem,
392-
impl_sig: ty::FnSig<'tcx>,
393451
trait_m: &ty::AssocItem,
394-
trait_sig: ty::FnSig<'tcx>,
395452
) -> (Span, Option<Span>) {
396453
let tcx = infcx.tcx;
397454
let impl_m_hir_id = tcx.hir().local_def_id_to_hir_id(impl_m.def_id.expect_local());
398-
let (impl_m_output, impl_m_iter) = match tcx.hir().expect_impl_item(impl_m_hir_id).kind {
399-
ImplItemKind::Fn(ref impl_m_sig, _) => {
400-
(&impl_m_sig.decl.output, impl_m_sig.decl.inputs.iter())
455+
let mut impl_args = match tcx.hir().expect_impl_item(impl_m_hir_id).kind {
456+
ImplItemKind::Fn(ref sig, _) => {
457+
sig.decl.inputs.iter().map(|t| t.span).chain(iter::once(sig.decl.output.span()))
401458
}
402459
_ => bug!("{:?} is not a method", impl_m),
403460
};
404-
405-
match *terr {
406-
TypeError::Mutability => {
407-
if let Some(def_id) = trait_m.def_id.as_local() {
408-
let trait_m_hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
409-
let trait_m_iter = match tcx.hir().expect_trait_item(trait_m_hir_id).kind {
410-
TraitItemKind::Fn(ref trait_m_sig, _) => trait_m_sig.decl.inputs.iter(),
411-
_ => bug!("{:?} is not a TraitItemKind::Fn", trait_m),
412-
};
413-
414-
iter::zip(impl_m_iter, trait_m_iter)
415-
.find(|&(ref impl_arg, ref trait_arg)| {
416-
match (&impl_arg.kind, &trait_arg.kind) {
417-
(
418-
&hir::TyKind::Rptr(_, ref impl_mt),
419-
&hir::TyKind::Rptr(_, ref trait_mt),
420-
)
421-
| (&hir::TyKind::Ptr(ref impl_mt), &hir::TyKind::Ptr(ref trait_mt)) => {
422-
impl_mt.mutbl != trait_mt.mutbl
423-
}
424-
_ => false,
425-
}
426-
})
427-
.map(|(ref impl_arg, ref trait_arg)| (impl_arg.span, Some(trait_arg.span)))
428-
.unwrap_or_else(|| (cause.span(tcx), tcx.hir().span_if_local(trait_m.def_id)))
429-
} else {
430-
(cause.span(tcx), tcx.hir().span_if_local(trait_m.def_id))
461+
let trait_args = trait_m.def_id.as_local().map(|def_id| {
462+
let trait_m_hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
463+
match tcx.hir().expect_trait_item(trait_m_hir_id).kind {
464+
TraitItemKind::Fn(ref sig, _) => {
465+
sig.decl.inputs.iter().map(|t| t.span).chain(iter::once(sig.decl.output.span()))
431466
}
467+
_ => bug!("{:?} is not a TraitItemKind::Fn", trait_m),
432468
}
433-
TypeError::Sorts(ExpectedFound { .. }) => {
434-
if let Some(def_id) = trait_m.def_id.as_local() {
435-
let trait_m_hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
436-
let (trait_m_output, trait_m_iter) =
437-
match tcx.hir().expect_trait_item(trait_m_hir_id).kind {
438-
TraitItemKind::Fn(ref trait_m_sig, _) => {
439-
(&trait_m_sig.decl.output, trait_m_sig.decl.inputs.iter())
440-
}
441-
_ => bug!("{:?} is not a TraitItemKind::Fn", trait_m),
442-
};
469+
});
443470

444-
let impl_iter = impl_sig.inputs().iter();
445-
let trait_iter = trait_sig.inputs().iter();
446-
iter::zip(iter::zip(impl_iter, trait_iter), iter::zip(impl_m_iter, trait_m_iter))
447-
.find_map(|((&impl_arg_ty, &trait_arg_ty), (impl_arg, trait_arg))| match infcx
448-
.at(&cause, param_env)
449-
.sub(trait_arg_ty, impl_arg_ty)
450-
{
451-
Ok(_) => None,
452-
Err(_) => Some((impl_arg.span, Some(trait_arg.span))),
453-
})
454-
.unwrap_or_else(|| {
455-
if infcx
456-
.at(&cause, param_env)
457-
.sup(trait_sig.output(), impl_sig.output())
458-
.is_err()
459-
{
460-
(impl_m_output.span(), Some(trait_m_output.span()))
461-
} else {
462-
(cause.span(tcx), tcx.hir().span_if_local(trait_m.def_id))
463-
}
464-
})
465-
} else {
466-
(cause.span(tcx), tcx.hir().span_if_local(trait_m.def_id))
467-
}
471+
match *terr {
472+
TypeError::ArgumentMutability(i) => {
473+
(impl_args.nth(i).unwrap(), trait_args.and_then(|mut args| args.nth(i)))
474+
}
475+
TypeError::ArgumentSorts(ExpectedFound { .. }, i) => {
476+
(impl_args.nth(i).unwrap(), trait_args.and_then(|mut args| args.nth(i)))
468477
}
469478
_ => (cause.span(tcx), tcx.hir().span_if_local(trait_m.def_id)),
470479
}
@@ -514,8 +523,7 @@ fn compare_self_type<'tcx>(
514523
tcx.sess,
515524
impl_m_span,
516525
E0185,
517-
"method `{}` has a `{}` declaration in the impl, but \
518-
not in the trait",
526+
"method `{}` has a `{}` declaration in the impl, but not in the trait",
519527
trait_m.ident,
520528
self_descr
521529
);
@@ -535,8 +543,7 @@ fn compare_self_type<'tcx>(
535543
tcx.sess,
536544
impl_m_span,
537545
E0186,
538-
"method `{}` has a `{}` declaration in the trait, but \
539-
not in the impl",
546+
"method `{}` has a `{}` declaration in the trait, but not in the impl",
540547
trait_m.ident,
541548
self_descr
542549
);
@@ -993,8 +1000,7 @@ crate fn compare_const_impl<'tcx>(
9931000
tcx.sess,
9941001
cause.span,
9951002
E0326,
996-
"implemented const `{}` has an incompatible type for \
997-
trait",
1003+
"implemented const `{}` has an incompatible type for trait",
9981004
trait_c.ident
9991005
);
10001006

‎src/test/ui/associated-types/defaults-specialization.stderr‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@ LL | fn make() -> Self::Ty {
1515
| -------- type in trait
1616
...
1717
LL | fn make() -> u8 { 0 }
18-
| ^^ expected associated type, found `u8`
18+
| ^^
19+
| |
20+
| expected associated type, found `u8`
21+
| help: change the output type to match the trait: `<A<T> as Tr>::Ty`
1922
|
2023
= note: expected fn pointer `fn() -> <A<T> as Tr>::Ty`
2124
found fn pointer `fn() -> u8`
@@ -30,7 +33,10 @@ LL | default type Ty = bool;
3033
| ----------------------- expected this associated type
3134
LL |
3235
LL | fn make() -> bool { true }
33-
| ^^^^ expected associated type, found `bool`
36+
| ^^^^
37+
| |
38+
| expected associated type, found `bool`
39+
| help: change the output type to match the trait: `<B<T> as Tr>::Ty`
3440
|
3541
= note: expected fn pointer `fn() -> <B<T> as Tr>::Ty`
3642
found fn pointer `fn() -> bool`

‎src/test/ui/compare-method/bad-self-type.rs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
use std::future::Future;
2+
use std::task::{Context, Poll};
3+
4+
fn main() {}
5+
6+
struct MyFuture {}
7+
8+
impl Future for MyFuture {
9+
type Output = ();
10+
fn poll(self, _: &mut Context<'_>) -> Poll<()> {
11+
//~^ ERROR method `poll` has an incompatible type for trait
12+
todo!()
13+
}
14+
}
15+
16+
trait T {
17+
fn foo(self);
18+
fn bar(self) -> Option<()>;
19+
}
20+
21+
impl T for MyFuture {
22+
fn foo(self: Box<Self>) {}
23+
//~^ ERROR method `foo` has an incompatible type for trait
24+
fn bar(self) {}
25+
//~^ ERROR method `bar` has an incompatible type for trait
26+
}

0 commit comments

Comments
 (0)