Skip to content

Additional span info for E0053 #35765

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.

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

Merged
merged 3 commits into from
Aug 18, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,13 @@ impl<'ast> Map<'ast> {
}
}

pub fn expect_impl_item(&self, id: NodeId) -> &'ast ImplItem {
match self.find(id) {
Some(NodeImplItem(item)) => item,
_ => bug!("expected impl item, found {}", self.node_to_string(id))
}
}

pub fn expect_trait_item(&self, id: NodeId) -> &'ast TraitItem {
match self.find(id) {
Some(NodeTraitItem(item)) => item,
Expand Down
6 changes: 5 additions & 1 deletion src/librustc/infer/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
pub fn note_type_err(&self,
diag: &mut DiagnosticBuilder<'tcx>,
origin: TypeOrigin,
secondary_span: Option<(Span, String)>,
values: Option<ValuePairs<'tcx>>,
terr: &TypeError<'tcx>)
{
Expand Down Expand Up @@ -553,6 +554,9 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
}

diag.span_label(span, &terr);
if let Some((sp, msg)) = secondary_span {
diag.span_label(sp, &msg);
}

self.note_error_origin(diag, &origin);
self.check_and_note_conflicting_crates(diag, terr, span);
Expand All @@ -569,7 +573,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
self.tcx.sess, trace.origin.span(), E0308,
"{}", trace.origin.as_failure_str()
);
self.note_type_err(&mut diag, trace.origin, Some(trace.values), terr);
self.note_type_err(&mut diag, trace.origin, None, Some(trace.values), terr);
diag
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
self.tcx.sess, origin.span(), E0271,
"type mismatch resolving `{}`", predicate
);
self.note_type_err(&mut diag, origin, values, err);
self.note_type_err(&mut diag, origin, None, values, err);
self.note_obligation_cause(&mut diag, obligation);
diag.emit();
});
Expand Down
106 changes: 85 additions & 21 deletions src/librustc_typeck/check/compare_method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@ use middle::free_region::FreeRegionMap;
use rustc::infer::{self, InferOk, TypeOrigin};
use rustc::ty;
use rustc::traits::{self, Reveal};
use rustc::ty::error::ExpectedFound;
use rustc::ty::error::{ExpectedFound, TypeError};
use rustc::ty::subst::{Subst, Substs};
use rustc::hir::map::Node;
use rustc::hir::{ImplItemKind, TraitItem_};
use rustc::hir::{ImplItemKind, TraitItem_, Ty_};

use syntax::ast;
use syntax_pos::Span;
Expand Down Expand Up @@ -300,6 +299,7 @@ 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,
Expand All @@ -318,6 +318,7 @@ 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,
Expand All @@ -331,16 +332,82 @@ 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();
Copy link
Contributor

Choose a reason for hiding this comment

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

This unwrap is probably the cause of the ICE / regression #35869.

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)))
Copy link
Member Author

Choose a reason for hiding this comment

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

I ended up not using ccx.ast_ty_to_ty_cache since it can apparently store types that are exactly the same as either of expected or found on a completely different AST node. Iterating over the arguments is a safer bet since we have all the contextual information we need.

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

let origin = TypeOrigin::MethodCompatCheck(impl_err_span);

let mut diag = struct_span_err!(
tcx.sess, origin.span(), E0053,
"method `{}` has an incompatible type for trait", trait_m.name
);

infcx.note_type_err(
&mut diag, origin,
&mut diag,
origin,
trait_err_span.map(|sp| (sp, format!("original trait requirement"))),
Some(infer::ValuePairs::Types(ExpectedFound {
expected: trait_fty,
found: impl_fty
})), &terr
expected: trait_fty,
found: impl_fty
})),
&terr
);
diag.emit();
return
Expand Down Expand Up @@ -487,12 +554,9 @@ pub fn compare_const_impl<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>,
trait_ty);

// Locate the Span containing just the type of the offending impl
if let Some(impl_trait_node) = tcx.map.get_if_local(impl_c.def_id) {
if let Node::NodeImplItem(impl_trait_item) = impl_trait_node {
if let ImplItemKind::Const(ref ty, _) = impl_trait_item.node {
origin = TypeOrigin::Misc(ty.span);
}
}
match tcx.map.expect_impl_item(impl_c_node_id).node {
ImplItemKind::Const(ref ty, _) => origin = TypeOrigin::Misc(ty.span),
_ => bug!("{:?} is not a impl const", impl_c)
}

let mut diag = struct_span_err!(
Expand All @@ -502,16 +566,16 @@ pub fn compare_const_impl<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>,
);

// Add a label to the Span containing just the type of the item
if let Some(orig_trait_node) = tcx.map.get_if_local(trait_c.def_id) {
if let Node::NodeTraitItem(orig_trait_item) = orig_trait_node {
if let TraitItem_::ConstTraitItem(ref ty, _) = orig_trait_item.node {
diag.span_label(ty.span, &format!("original trait requirement"));
}
}
}
let trait_c_node_id = tcx.map.as_local_node_id(trait_c.def_id).unwrap();
let trait_c_span = match tcx.map.expect_trait_item(trait_c_node_id).node {
TraitItem_::ConstTraitItem(ref ty, _) => ty.span,
_ => bug!("{:?} is not a trait const", trait_c)
};

infcx.note_type_err(
&mut diag, origin,
&mut diag,
origin,
Some((trait_c_span, format!("original trait requirement"))),
Some(infer::ValuePairs::Types(ExpectedFound {
expected: trait_ty,
found: impl_ty
Expand Down
14 changes: 10 additions & 4 deletions src/test/compile-fail/E0053.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,21 @@
// except according to those terms.

trait Foo {
fn foo(x: u16);
fn bar(&self);
fn foo(x: u16); //~ NOTE original trait requirement
fn bar(&self); //~ NOTE original trait requirement
}

struct Bar;

impl Foo for Bar {
fn foo(x: i16) { } //~ ERROR E0053
fn bar(&mut self) { } //~ ERROR E0053
fn foo(x: i16) { }
//~^ ERROR method `foo` has an incompatible type for trait
//~| NOTE expected u16
fn bar(&mut self) { }
//~^ ERROR method `bar` has an incompatible type for trait
//~| NOTE values differ in mutability
//~| NOTE expected type `fn(&Bar)`
//~| NOTE found type `fn(&mut Bar)`
}

fn main() {
Expand Down
27 changes: 27 additions & 0 deletions src/test/ui/mismatched_types/trait-impl-fn-incompatibility.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// 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.

// rustc-env:RUST_NEW_ERROR_FORMAT

trait Foo {
fn foo(x: u16);
fn bar(&mut self, bar: &mut Bar);
}

struct Bar;

impl Foo for Bar {
fn foo(x: i16) { }
fn bar(&mut self, bar: &Bar) { }
}

fn main() {
}

23 changes: 23 additions & 0 deletions src/test/ui/mismatched_types/trait-impl-fn-incompatibility.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
error[E0053]: method `foo` has an incompatible type for trait
--> $DIR/trait-impl-fn-incompatibility.rs:21:15
|
14 | fn foo(x: u16);
| --- original trait requirement
...
21 | fn foo(x: i16) { }
| ^^^ expected u16, found i16

error[E0053]: method `bar` has an incompatible type for trait
--> $DIR/trait-impl-fn-incompatibility.rs:22:28
|
15 | fn bar(&mut self, bar: &mut Bar);
| -------- original trait requirement
...
22 | fn bar(&mut self, bar: &Bar) { }
| ^^^^ values differ in mutability
|
= note: expected type `fn(&mut Bar, &mut Bar)`
= note: found type `fn(&mut Bar, &Bar)`

error: aborting due to 2 previous errors