Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ICE when arg types can't be found in impl/trait methods while comparing #35877

Merged
merged 2 commits into from
Aug 27, 2016
Merged

Fix ICE when arg types can't be found in impl/trait methods while comparing #35877

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
07e6e81
Extract error reporting into its own fn
KiChjang Aug 22, 2016
3d9cf30
More robust testing
KiChjang Aug 22, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
148 changes: 85 additions & 63 deletions src/librustc_typeck/check/compare_method.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,11 +299,10 @@ pub fn compare_impl_method<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>,
impl_m_span,
impl_m_body_id,
&impl_sig);
let impl_args = impl_sig.inputs.clone();
let impl_fty = tcx.mk_fn_ptr(tcx.mk_bare_fn(ty::BareFnTy {
unsafety: impl_m.fty.unsafety,
abi: impl_m.fty.abi,
sig: ty::Binder(impl_sig)
sig: ty::Binder(impl_sig.clone())
}));
debug!("compare_impl_method: impl_fty={:?}", impl_fty);

Expand All @@ -318,11 +317,10 @@ pub fn compare_impl_method<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>,
impl_m_span,
impl_m_body_id,
&trait_sig);
let trait_args = trait_sig.inputs.clone();
let trait_fty = tcx.mk_fn_ptr(tcx.mk_bare_fn(ty::BareFnTy {
unsafety: trait_m.fty.unsafety,
abi: trait_m.fty.abi,
sig: ty::Binder(trait_sig)
sig: ty::Binder(trait_sig.clone())
}));

debug!("compare_impl_method: trait_fty={:?}", trait_fty);
Expand All @@ -332,65 +330,9 @@ pub fn compare_impl_method<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>,
impl_fty,
trait_fty);

let impl_m_iter = match tcx.map.expect_impl_item(impl_m_node_id).node {
ImplItemKind::Method(ref impl_m_sig, _) => impl_m_sig.decl.inputs.iter(),
_ => bug!("{:?} is not a method", impl_m)
};

let (impl_err_span, trait_err_span) = match terr {
TypeError::Mutability => {
if let Some(trait_m_node_id) = tcx.map.as_local_node_id(trait_m.def_id) {
let trait_m_iter = match tcx.map.expect_trait_item(trait_m_node_id).node {
TraitItem_::MethodTraitItem(ref trait_m_sig, _) =>
trait_m_sig.decl.inputs.iter(),
_ => bug!("{:?} is not a MethodTraitItem", trait_m)
};

impl_m_iter.zip(trait_m_iter).find(|&(ref impl_arg, ref trait_arg)| {
match (&impl_arg.ty.node, &trait_arg.ty.node) {
(&Ty_::TyRptr(_, ref impl_mt), &Ty_::TyRptr(_, ref trait_mt)) |
(&Ty_::TyPtr(ref impl_mt), &Ty_::TyPtr(ref trait_mt)) =>
impl_mt.mutbl != trait_mt.mutbl,
_ => false
}
}).map(|(ref impl_arg, ref trait_arg)| {
match (impl_arg.to_self(), trait_arg.to_self()) {
(Some(impl_self), Some(trait_self)) =>
(impl_self.span, Some(trait_self.span)),
(None, None) => (impl_arg.ty.span, Some(trait_arg.ty.span)),
_ => bug!("impl and trait fns have different first args, \
impl: {:?}, trait: {:?}", impl_arg, trait_arg)
}
}).unwrap_or((origin.span(), tcx.map.span_if_local(trait_m.def_id)))
} else {
(origin.span(), tcx.map.span_if_local(trait_m.def_id))
}
}
TypeError::Sorts(ExpectedFound { expected, found }) => {
if let Some(trait_m_node_id) = tcx.map.as_local_node_id(trait_m.def_id) {
let trait_m_iter = match tcx.map.expect_trait_item(trait_m_node_id).node {
TraitItem_::MethodTraitItem(ref trait_m_sig, _) =>
trait_m_sig.decl.inputs.iter(),
_ => bug!("{:?} is not a MethodTraitItem", trait_m)
};
let impl_iter = impl_args.iter();
let trait_iter = trait_args.iter();
let arg_idx = impl_iter.zip(trait_iter)
.position(|(impl_arg_ty, trait_arg_ty)| {
*impl_arg_ty == found && *trait_arg_ty == expected
}).unwrap();
impl_m_iter.zip(trait_m_iter)
.nth(arg_idx)
.map(|(impl_arg, trait_arg)|
(impl_arg.ty.span, Some(trait_arg.ty.span)))
.unwrap_or(
(origin.span(), tcx.map.span_if_local(trait_m.def_id)))
} else {
(origin.span(), tcx.map.span_if_local(trait_m.def_id))
}
}
_ => (origin.span(), tcx.map.span_if_local(trait_m.def_id))
};
let (impl_err_span, trait_err_span) =
extract_spans_for_error_reporting(&infcx, &terr, origin, impl_m,
impl_sig, trait_m, trait_sig);

let origin = TypeOrigin::MethodCompatCheck(impl_err_span);

Expand Down Expand Up @@ -478,6 +420,86 @@ pub fn compare_impl_method<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>,

return true;
}

fn extract_spans_for_error_reporting<'a, 'gcx, 'tcx>(infcx: &infer::InferCtxt<'a, 'gcx, 'tcx>,
terr: &TypeError,
origin: TypeOrigin,
impl_m: &ty::Method,
impl_sig: ty::FnSig<'tcx>,
trait_m: &ty::Method,
trait_sig: ty::FnSig<'tcx>)
-> (Span, Option<Span>) {
let tcx = infcx.tcx;
let impl_m_node_id = tcx.map.as_local_node_id(impl_m.def_id).unwrap();
let (impl_m_output, impl_m_iter) = match tcx.map.expect_impl_item(impl_m_node_id).node {
ImplItemKind::Method(ref impl_m_sig, _) =>
(&impl_m_sig.decl.output, impl_m_sig.decl.inputs.iter()),
_ => bug!("{:?} is not a method", impl_m)
};

match *terr {
TypeError::Mutability => {
if let Some(trait_m_node_id) = tcx.map.as_local_node_id(trait_m.def_id) {
let trait_m_iter = match tcx.map.expect_trait_item(trait_m_node_id).node {
TraitItem_::MethodTraitItem(ref trait_m_sig, _) =>
trait_m_sig.decl.inputs.iter(),
_ => bug!("{:?} is not a MethodTraitItem", trait_m)
};

impl_m_iter.zip(trait_m_iter).find(|&(ref impl_arg, ref trait_arg)| {
match (&impl_arg.ty.node, &trait_arg.ty.node) {
(&Ty_::TyRptr(_, ref impl_mt), &Ty_::TyRptr(_, ref trait_mt)) |
(&Ty_::TyPtr(ref impl_mt), &Ty_::TyPtr(ref trait_mt)) =>
impl_mt.mutbl != trait_mt.mutbl,
_ => false
}
}).map(|(ref impl_arg, ref trait_arg)| {
match (impl_arg.to_self(), trait_arg.to_self()) {
(Some(impl_self), Some(trait_self)) =>
(impl_self.span, Some(trait_self.span)),
(None, None) => (impl_arg.ty.span, Some(trait_arg.ty.span)),
_ => bug!("impl and trait fns have different first args, \
impl: {:?}, trait: {:?}", impl_arg, trait_arg)
}
}).unwrap_or((origin.span(), tcx.map.span_if_local(trait_m.def_id)))
} else {
(origin.span(), tcx.map.span_if_local(trait_m.def_id))
}
}
TypeError::Sorts(ExpectedFound { .. }) => {
if let Some(trait_m_node_id) = tcx.map.as_local_node_id(trait_m.def_id) {
let (trait_m_output, trait_m_iter) =
match tcx.map.expect_trait_item(trait_m_node_id).node {
TraitItem_::MethodTraitItem(ref trait_m_sig, _) =>
(&trait_m_sig.decl.output, trait_m_sig.decl.inputs.iter()),
_ => bug!("{:?} is not a MethodTraitItem", trait_m)
};

let impl_iter = impl_sig.inputs.iter();
let trait_iter = trait_sig.inputs.iter();
impl_iter.zip(trait_iter).zip(impl_m_iter).zip(trait_m_iter)
.filter_map(|(((impl_arg_ty, trait_arg_ty), impl_arg), trait_arg)| {
match infcx.sub_types(true, origin, trait_arg_ty, impl_arg_ty) {
Ok(_) => None,
Err(_) => Some((impl_arg.ty.span, Some(trait_arg.ty.span)))
}
})
.next()
.unwrap_or_else(|| {
if infcx.sub_types(false, origin, impl_sig.output,
trait_sig.output).is_err() {
(impl_m_output.span(), Some(trait_m_output.span()))
Copy link
Member Author

@KiChjang KiChjang Aug 26, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arielb1 I don't see a way to improve this? If we're not grabbing the span of the type, are we supposed to grab the span of the entire function declaration?

} else {
(origin.span(), tcx.map.span_if_local(trait_m.def_id))
}
})
} else {
(origin.span(), tcx.map.span_if_local(trait_m.def_id))
}
}
_ => (origin.span(), tcx.map.span_if_local(trait_m.def_id))
}
}
}

pub fn compare_const_impl<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>,
Expand Down
37 changes: 37 additions & 0 deletions src/test/compile-fail/issue-35869.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(conservative_impl_trait)]

trait Foo {
fn foo(fn(u8) -> ()); //~ NOTE type in trait
fn bar(Option<u8>); //~ NOTE type in trait
fn baz((u8, u16)); //~ NOTE type in trait
fn qux() -> u8; //~ NOTE type in trait
}

struct Bar;

impl Foo for Bar {
fn foo(_: fn(u16) -> ()) {}
//~^ ERROR method `foo` has an incompatible type for trait
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: maybe this should be a ui test? It's hard to tell just what is being highlighted here. :)

//~| NOTE expected u8
fn bar(_: Option<u16>) {}
//~^ ERROR method `bar` has an incompatible type for trait
//~| NOTE expected u8
fn baz(_: (u16, u16)) {}
//~^ ERROR method `baz` has an incompatible type for trait
//~| NOTE expected u8
fn qux() -> u16 { 5u16 }
//~^ ERROR method `qux` has an incompatible type for trait
//~| NOTE expected u8
}

fn main() {}