Skip to content

Commit

Permalink
Detect mismatch of impl Trait and Generics
Browse files Browse the repository at this point in the history
Add a function to detect the distinction between the following code

```
trait Foo {
    fn bar(&self, &impl Debug);
}

impl Foo for () {
    fn bar<U: Debug>(&self, _: U) { }
}
```

What is still left to do is add a new error code and explanation, and
clean up extraneous function arguments.
  • Loading branch information
chrisvittal committed Nov 5, 2017
1 parent ec5623c commit 225955a
Showing 1 changed file with 50 additions and 0 deletions.
50 changes: 50 additions & 0 deletions src/librustc_typeck/check/compare_method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ pub fn compare_impl_method<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
return;
}

if let Err(ErrorReported) = compare_synthetic_generics(tcx,
impl_m,
impl_m_span,
trait_m,
trait_item_span) {
return;
}

if let Err(ErrorReported) = compare_predicate_entailment(tcx,
impl_m,
impl_m_span,
Expand Down Expand Up @@ -321,6 +329,8 @@ fn compare_predicate_entailment<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
found: impl_fty,
})),
&terr);
// FIXME(chrisvittal) do not emit this error if we already have an error on the
// return value of the trait
diag.emit();
return Err(ErrorReported);
}
Expand Down Expand Up @@ -707,6 +717,46 @@ fn compare_number_of_method_arguments<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
Ok(())
}

fn compare_synthetic_generics<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,

This comment has been minimized.

Copy link
@nikomatsakis

nikomatsakis Nov 6, 2017

This doesn't seem quite right. For example, if I have

trait Foo {
    fn foo<T: Debug>(x: &T, y: &impl Debug);
}

impl Foo {
    fn foo<T: Debug>(x: &impl Debug, y: &T) { }
}

I think that this code will pass, no? Ah, I suppose not, we'll probably get a somewhat obscure error, just as if you had done <A,B> (A, B) vs <B,A> (A, B). Well, I'd like a ui test for this scenario anyhow. =)

This comment has been minimized.

Copy link
@chrisvittal

chrisvittal Nov 7, 2017

Author Owner

At current, you get

error[E0053]: method `foo` has an incompatible type for trait
  --> tmp.rs:14:33
   |
10 |     fn foo<U: Debug>(&self, _x: &impl Debug, _y: &U);
   |                                 ----------- type in trait
...
14 |     fn foo<T: Debug>(&self, _x: &T, _y: &impl Debug) { }
   |                                 ^^ expected type parameter, found a different type parameter
   |
   = note: expected type `fn(&(), &, &T)`
              found type `fn(&(), &T, &)`

error: aborting due to previous error

I'd like to use the new error, but at current it's still illegal, and reasonably helpful even.

impl_m: &ty::AssociatedItem,
_impl_m_span: Span, // FIXME necessary?
trait_m: &ty::AssociatedItem,
_trait_item_span: Option<Span>) // FIXME necessary?
-> Result<(), ErrorReported> {
// FIXME(chrisvittal) Clean up this function, list of FIXME items:
// 1. New error code
// 2. Better messages for the span lables
// 3. Explination as to what is going on
// 4. Correct the function signature for what we actually use
// If we get here, we already have the same number of generics, so the zip will
// be okay.
let mut error_found = false;
let impl_m_generics = tcx.generics_of(impl_m.def_id);
let trait_m_generics = tcx.generics_of(trait_m.def_id);
for (impl_ty, trait_ty) in impl_m_generics.types.iter().zip(trait_m_generics.types.iter()) {
if impl_ty.synthetic != trait_ty.synthetic {
let impl_node_id = tcx.hir.as_local_node_id(impl_ty.def_id).unwrap();
let impl_span = tcx.hir.span(impl_node_id);
let trait_node_id = tcx.hir.as_local_node_id(trait_ty.def_id).unwrap();
let trait_span = tcx.hir.span(trait_node_id);
let mut err = struct_span_err!(tcx.sess,
impl_span,
E0053,
"method `{}` has incompatible signature for trait",
trait_m.name);
err.span_label(trait_span, "Annotation in trait");

This comment has been minimized.

Copy link
@nikomatsakis

nikomatsakis Nov 6, 2017

Nit: lower-case

err.span_label(impl_span, "Annotation in impl");

This comment has been minimized.

Copy link
@nikomatsakis

nikomatsakis Nov 6, 2017

Nit: lower-case

err.emit();
error_found = true;
}
}
if error_found {
Err(ErrorReported)
} else {
Ok(())
}
}

pub fn compare_const_impl<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
impl_c: &ty::AssociatedItem,
impl_c_span: Span,
Expand Down

0 comments on commit 225955a

Please sign in to comment.