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.

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

Include type of missing trait methods in error #36371

Closed
wants to merge 3 commits into from
Closed
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
21 changes: 21 additions & 0 deletions src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,27 @@ pub enum Node<'ast> {
NodeTyParam(&'ast TyParam)
}

impl<'ast> Node<'ast> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add spans to everything?

pub fn span(&self) -> Option<Span> {
match *self {
NodeItem(item) => Some(item.span),
NodeForeignItem(item) => Some(item.span),
NodeTraitItem(item) => Some(item.span),
NodeImplItem(item) => Some(item.span),
NodeVariant(_) => None,
NodeExpr(item) => Some(item.span),
NodeStmt(_) => None,
NodeTy(ty) => Some(ty.span),
NodeLocal(pat) => Some(pat.span),
NodePat(pat) => Some(pat.span),
NodeBlock(block) => Some(block.span),
NodeStructCtor(_) => None,
NodeLifetime(lifetime) => Some(lifetime.span),
NodeTyParam(ty) => Some(ty.span),
}
}
}

/// Represents an entry and its parent NodeID.
/// The odd layout is to bring down the total size.
#[derive(Copy, Debug)]
Expand Down
84 changes: 84 additions & 0 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,40 @@ impl<'tcx> ImplOrTraitItem<'tcx> {
_ => None,
}
}

pub fn signature<'a>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> String {
match *self {
MethodTraitItem(ref item) => {
match tcx.map.get_if_local(item.def_id) {
Some(node) => {
match node.span() {
Some(span) => match tcx.sess.codemap().span_to_oneline_snippet(span) {
Ok(snippet) => snippet,
Err(_) => item.signature(tcx),
},
None => item.signature(tcx),
}
}
None => item.signature(tcx),
}
}
TypeTraitItem(ref item) => item.signature(tcx),
ConstTraitItem(ref item) => {
match tcx.map.get_if_local(item.def_id) {
Some(node) => {
match node.span() {
Some(span) => match tcx.sess.codemap().span_to_oneline_snippet(span) {
Ok(snippet) => snippet,
Err(_) => item.signature(tcx),
},
None => item.signature(tcx),
}
}
None => item.signature(tcx),
}
}
}
}
}

#[derive(Clone, Debug, PartialEq, Eq, Copy, RustcEncodable, RustcDecodable)]
Expand Down Expand Up @@ -327,6 +361,36 @@ impl<'tcx> Method<'tcx> {
ImplContainer(id) => id,
}
}

pub fn signature<'a>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> String {
let name = self.name.to_string();
let unsafety = match self.fty.unsafety {
hir::Unsafety::Unsafe => "unsafe ",
hir::Unsafety::Normal => "",
};
let has_gen_types = !self.generics.types.is_empty();
let type_args = if has_gen_types {
format!("<{}>", self.generics.types.clone().into_iter()
.map(|t| t.name.as_str().to_string())
.collect::<Vec<String>>()
.join(", "))
} else {
String::new()
};
let args = self.fty.sig.inputs().0.iter()
.map(|t| format!("{:?}", t)).collect::<Vec<_>>().join(", ");
//let return_type = format!("{}", tcx.item_name(self.fty.sig.output().0.def_id).as_str());
let return_type = format!("{:?}", self.fty.sig.output().0);
let return_signature = if &return_type == "()" {
"".to_string()
} else {
format!(" -> {}", return_type)
};

// unsafe fn name<'a, T>(args) -> ReturnType
format!("{}fn {}{}({}){};", unsafety, name, type_args, args, return_signature)
//format!("{}fn {}", unsafety, self.fty.sig.0)//name, type_args, args, return_signature)
}
}

impl<'tcx> PartialEq for Method<'tcx> {
Expand Down Expand Up @@ -354,6 +418,19 @@ pub struct AssociatedConst<'tcx> {
pub has_value: bool
}

impl<'tcx> AssociatedConst<'tcx> {
pub fn signature<'a>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> String {
// const FOO: Type = DEFAULT;
let value = if self.has_value {
" = <DEFAULT>"
} else {
""
};
//format!("const {}: {}{};", self.name.to_string(), tcx.item_name(self.ty.def_id).as_str(), value)
format!("const {}: {:?}{};", self.name.to_string(), self.ty, value)
}
}

#[derive(Clone, Copy, Debug)]
pub struct AssociatedType<'tcx> {
pub name: Name,
Expand All @@ -364,6 +441,13 @@ pub struct AssociatedType<'tcx> {
pub container: ImplOrTraitItemContainer,
}

impl<'tcx> AssociatedType<'tcx> {
pub fn signature<'a>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> String {
//// type Type;
format!("type {};", self.name.to_string())
}
}

#[derive(Clone, PartialEq, RustcDecodable, RustcEncodable, Copy)]
pub enum Variance {
Covariant, // T<A> <: T<B> iff A <: B -- e.g., function return type
Expand Down
31 changes: 23 additions & 8 deletions src/librustc_errors/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,22 +109,26 @@ impl EmitterWriter {
fn add_annotation_to_file(file_vec: &mut Vec<FileWithAnnotatedLines>,
file: Rc<FileMap>,
line_index: usize,
ann: Annotation) {
annotation: Option<Annotation>) {

let ann = match annotation {
Some(ref ann) => vec![ann.clone()],
None => vec![]
};
for slot in file_vec.iter_mut() {
// Look through each of our files for the one we're adding to
if slot.file.name == file.name {
// See if we already have a line for it
for line_slot in &mut slot.lines {
if line_slot.line_index == line_index {
line_slot.annotations.push(ann);
if line_slot.line_index == line_index && annotation.is_some() {
line_slot.annotations.push(annotation.unwrap());
return;
}
}
// We don't have a line yet, create one
slot.lines.push(Line {
line_index: line_index,
annotations: vec![ann],
annotations: ann,
});
slot.lines.sort();
return;
Expand All @@ -135,7 +139,7 @@ impl EmitterWriter {
file: file,
lines: vec![Line {
line_index: line_index,
annotations: vec![ann],
annotations: ann,
}],
});
}
Expand All @@ -151,6 +155,8 @@ impl EmitterWriter {
let mut hi = cm.lookup_char_pos(span_label.span.hi);
let mut is_minimized = false;

let start = lo.line;
let end = hi.line + 1;
// If the span is multi-line, simplify down to the span of one character
if lo.line != hi.line {
hi.line = lo.line;
Expand All @@ -168,15 +174,24 @@ impl EmitterWriter {
}

add_annotation_to_file(&mut output,
lo.file,
lo.file.clone(),
lo.line,
Annotation {
Some(Annotation {
start_col: lo.col.0,
end_col: hi.col.0,
is_primary: span_label.is_primary,
is_minimized: is_minimized,
label: span_label.label.clone(),
});
}));
if start != end {
// Add the rest of the lines, without any annotation
for line in start+1..end {
add_annotation_to_file(&mut output,
lo.file.clone(),
line,
None);
}
}
}
}
output
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_errors/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub mod registry;
pub mod styled_buffer;
mod lock;

use syntax_pos::{BytePos, Loc, FileLinesResult, FileName, MultiSpan, Span, NO_EXPANSION };
use syntax_pos::{BytePos, Loc, FileLinesResult, FileName, MultiSpan, Span, NO_EXPANSION};
use syntax_pos::{MacroBacktrace};

#[derive(Clone)]
Expand Down
1 change: 1 addition & 0 deletions src/librustc_typeck/check/compare_method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ pub fn compare_impl_method<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>,
err.span_label(span, & format!("`{}` used in trait",
trait_m.explicit_self));
}

err.emit();
return;
}
Expand Down
34 changes: 27 additions & 7 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1105,24 +1105,44 @@ fn check_impl_items_against_trait<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>,

if !is_implemented {
if !is_provided {
missing_items.push(trait_item.name());
missing_items.push(trait_item);
} else if associated_type_overridden {
invalidated_items.push(trait_item.name());
}
}
}

if !missing_items.is_empty() {
struct_span_err!(tcx.sess, impl_span, E0046,
let codemap = tcx.sess.codemap();
let start = codemap.lookup_line(impl_span.lo);
let end = codemap.lookup_line(impl_span.hi);
let sp = if let (Ok(start), Ok(end)) = (start, end) {
if start.line != end.line {
impl_span.start_point()
} else {
impl_span
}
} else {
impl_span
};
let mut err = struct_span_err!(tcx.sess, sp, E0046,
"not all trait items implemented, missing: `{}`",
missing_items.iter()
.map(|name| name.to_string())
.collect::<Vec<_>>().join("`, `"))
.span_label(impl_span, &format!("missing `{}` in implementation",
.map(|trait_item| trait_item.name().to_string())
.collect::<Vec<_>>().join("`, `"));
err.span_label(sp, &format!("missing `{}` in implementation",
missing_items.iter()
.map(|name| name.to_string())
.map(|name| name.name().to_string())
.collect::<Vec<_>>().join("`, `"))
).emit();
);
for trait_item in missing_items {
if let Some(span) = tcx.map.span_if_local(trait_item.def_id()) {
err.span_label(span, &"missing definition in implementation");
} else {
err.note(&format!("infered definition: `{}`", trait_item.signature(tcx)));
}
}
err.emit();
}

if !invalidated_items.is_empty() {
Expand Down
59 changes: 58 additions & 1 deletion src/libsyntax/codemap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ impl CodeMap {
}

// If the relevant filemap is empty, we don't return a line number.
fn lookup_line(&self, pos: BytePos) -> Result<FileMapAndLine, Rc<FileMap>> {
pub fn lookup_line(&self, pos: BytePos) -> Result<FileMapAndLine, Rc<FileMap>> {
let idx = self.lookup_filemap_idx(pos);

let files = self.files.borrow();
Expand Down Expand Up @@ -670,6 +670,63 @@ impl CodeMap {
}
}

/// Reformat `sp`'s snippet to oneline if it is available
///
/// Given a snippet like:
///
/// ```text
/// fn foo< 'lifetime, T >(
/// &self,
/// bar : &Type< 'lifetime, T>)
/// -> std::result::Result<(),
/// Error>;
/// ```
///
/// it'll return:
///
/// ```text
/// fn foo<'lifetime, T>(&self, bar: &Type<'lifetime, T>) -> std::result::Result<(), Error>;
/// ```
pub fn span_to_oneline_snippet(&self, sp: Span) -> Result<String, SpanSnippetError> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nikomatsakis could there be a method already in the codebase that does this? If there isn't, does this seem reasonable to be added for these use cases?

let no_space_after = ["<", "("];
let no_space_before = [">", ")", ",", ":", ";"];

let snippet = self.span_to_snippet(sp);
match snippet {
Ok(snippet) => {
let mut it = snippet.split_whitespace();
let mut next = it.next();
let mut result = String::new();

loop { // Remove spaces after `<` and `(` and before `>`, `)` `:` and `,`
match next {
Some(c) => {
let peek = it.next();
match peek {
Some(n) => {
result.push_str(c);

if !(no_space_after.into_iter().any(|x| c.ends_with(x)) ||
no_space_before.into_iter().any(|x| n.starts_with(x))) {
result.push_str(" ");
}
next = peek;
}
None => { // last item, don't skip
result.push_str(c);
next = peek;
}
}
}
None => break, // end of iter
}
}
Ok(result)
}
Err(e) => Err(e),
}
}

pub fn get_filemap(&self, filename: &str) -> Option<Rc<FileMap>> {
for fm in self.files.borrow().iter() {
if filename == fm.name {
Expand Down
6 changes: 6 additions & 0 deletions src/libsyntax_pos/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ impl Span {
Span { lo: BytePos(lo), hi: self.hi, expn_id: self.expn_id}
}

/// Returns a new span representing just the start-point of this span
pub fn start_point(self) -> Span {
let lo = cmp::min(self.lo.0, self.hi.0 - 1);
Span { lo: BytePos(lo), hi: BytePos(lo), expn_id: self.expn_id}
}

/// Returns `self` if `self` is not the dummy span, and `other` otherwise.
pub fn substitute_dummy(self, other: Span) -> Span {
if self.source_equal(&DUMMY_SP) { other } else { self }
Expand Down
1 change: 1 addition & 0 deletions src/test/compile-fail/E0046.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ struct Bar;
impl Foo for Bar {}
//~^ ERROR E0046
//~| NOTE missing `foo` in implementation
//~| NOTE fn foo();

fn main() {
}
Loading