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

E0407 needs to be updated to new format #35697

Closed
sophiajt opened this issue Aug 15, 2016 · 8 comments
Closed

E0407 needs to be updated to new format #35697

sophiajt opened this issue Aug 15, 2016 · 8 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints

Comments

@sophiajt
Copy link
Contributor

From: src/test/compile-fail/E0407.rs

E0407 needs a span_label, updating it from:

error[E0407]: method `b` is not a member of trait `Foo`
  --> src/test/compile-fail/E0407.rs:19:5
   |
19 |     fn b() {} //~ ERROR E0407
   |     ^^^^^^^^^

To:

error[E0407]: method `b` is not a member of trait `Foo`
  --> src/test/compile-fail/E0407.rs:19:5
   |
19 |     fn b() {} //~ ERROR E0407
   |     ^^^^^^^^^ not a member of `Foo`

Bonus: tighten the span to focus on b:

error[E0407]: method `b` is not a member of trait `Foo`
  --> src/test/compile-fail/E0407.rs:19:5
   |
19 |     fn b() {} //~ ERROR E0407
   |        ^ not a member of `Foo`
@tvladyslav
Copy link
Contributor

@jonathandturner give me a hint how to implement bonus part, please.
I see, that my span is transferred into error handler from here:
rust/src/librustc_resolve/lib.rs:1952

ImplItemKind::Method(ref sig, _) => {
    // If this is a trait impl, ensure the method
    // exists in trait
    this.check_trait_item(impl_item.ident.name,
        impl_item.span,
        |n, s| ResolutionError::MethodNotMemberOfTrait(n, s));

ImplItem does not contain useful info. impl_item.span corresponds to whole function. The only stupid idea, that I tried - hardcode, like this:

        ResolutionError::MethodNotMemberOfTrait(method, trait_) => {
            use std::ops::{Add, Sub};
            use syntax_pos::Pos;
            use syntax::codemap::BytePos;
            let func_span = Span {
                lo: span.lo.add(BytePos::from_usize(3)),
                hi: span.hi.sub(BytePos::from_usize(5)),
                expn_id: span.expn_id
            };
            let mut err = struct_span_err!(resolver.session,
                                           func_span,
                                           E0407,
                                           "method `{}` is not a member of trait `{}`",
                                           method,
                                           trait_);
            err.span_label(func_span, &format!("not a member of `{}`", trait_));
            err

Hardcoded shift for lower bound is more or less acceptable, because usually function starts from "fn ", upper bound is stupid 100%.
Another idea is to use span_to_string from CodeMap, parse string, get function name index and length and then create a Span. I don't know if it worth to implement.

@tvladyslav
Copy link
Contributor

Right now without bonus. But I work on it!

@sophiajt
Copy link
Contributor Author

I took a look at the bonus last night. I had a few ideas, but for what time I spent with it, it seemed like it wasn't going to be one of the easier bonuses :)

In general, it's much better to thread the Span information you need through or to look it up with a span_if_local. We're trying to avoid actually changing the spans (though there are a couple of rare exceptions).

Pinging @KiChjang who is also working on some of the bonuses and might have some ideas.

@KiChjang
Copy link
Member

KiChjang commented Aug 17, 2016

My ideas are all bad in this case, because there is no span information stored by the parser :(. Both span_if_local and span_to_string would still not work in this case since a NodeId wouldn't map to an Ident while the latter expects a span parameter. We might have to break the parser API again by adding a span field to the Ident struct...

@sophiajt
Copy link
Contributor Author

@KiChjang - possibly, though I'd like to underline other names as well, like in the case of an unused function we could underline the name instead of the whole function.

@eddyb - any thought on adding spans? I know @nikomatsakis mentioned you were thinking about possibly adding some spans in the future.

eddyb added a commit to eddyb/rust that referenced this issue Aug 18, 2016
@tvladyslav
Copy link
Contributor

@KiChjang So shell I wait for changes in parser API?

@KiChjang
Copy link
Member

You can add span information for the Ident struct yourself. Look in parser.rs.

@bstrie bstrie added the A-diagnostics Area: Messages for errors, warnings, and lints label Aug 24, 2016
@sophiajt
Copy link
Contributor Author

Closing this issue, but feel free to open one about the bonus.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints
Projects
None yet
Development

No branches or pull requests

4 participants