Skip to content

Error spans in macros 1.1 #36218

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

Closed
dtolnay opened this issue Sep 2, 2016 · 8 comments · Fixed by #36308
Closed

Error spans in macros 1.1 #36218

dtolnay opened this issue Sep 2, 2016 · 8 comments · Fixed by #36308

Comments

@dtolnay
Copy link
Member

dtolnay commented Sep 2, 2016

I understand that we can't know the spans correctly after passing through rustc_macro_derive but instead of pointing to the "Serialize" in these examples I think it would usually be less misleading to point to the "struct A" or maybe to the entire struct body. I don't want to field these support tickets in Serde's issue tracker:

trait T {
    fn f();
}

#[derive(Serialize, Deserialize)]
struct A {
    exciting: &T,
}

#[derive(Serialize, Deserialize)]
struct B<'a> {
    exciting: &'a T,
}
error[E0106]: missing lifetime specifier
  --> src/main.rs:10:10
   |
10 | #[derive(Serialize, Deserialize)]
   |          ^ expected lifetime parameter

error[E0038]: the trait `T` cannot be made into an object
  --> src/main.rs:15:15
   |
15 | #[derive(Serialize, Deserialize)]
   |          ^^^^^^^^^^ the trait `T` cannot be made into an object

Side note: those errors are inconsistent with each other, one points to just the letter "S", the other to "Serialize,".

@alexcrichton
Copy link
Member

Unfortunately I think this is a fundamental downside to the current approach of macros 1.1. When we parse an arbitrary string into a token stream, we need to give them some span. Right now we just give them the span of the original #[derive] mode, as it's one of the only spans we have.

Maybe we could try to show the expanded code though? I'm not 100% sure if that'd work though.

@dtolnay
Copy link
Member Author

dtolnay commented Sep 2, 2016

I assumed you also had the span of the ast::Item that was passed in. If that is not easily available then I'll close this.

@alexcrichton
Copy link
Member

Oh so for that we convert the ast::Item to a string directly, losing all span information, and then the tokens you pass back don't necessarily have any correlation to the previous item that was stringified.

Now it's not to say we can't do anything here, it's just that we can't do anything trivial here unfortunately afaik. Maybe @nrc has an opinion though?

@dtolnay
Copy link
Member Author

dtolnay commented Sep 2, 2016

My suggestion was here instead of using the span of the derive mode, use item.span from the original input item.

@dtolnay
Copy link
Member Author

dtolnay commented Sep 2, 2016

I'll try it out and report back if the errors make any more sense.

@dtolnay
Copy link
Member Author

dtolnay commented Sep 2, 2016

@alexcrichton @nrc using item.span seems less misleading:

error[E0106]: missing lifetime specifier
  --> src/main.rs:11:1
   |
11 | struct A {
   | ^ expected lifetime parameter

error[E0038]: the trait `T` cannot be made into an object
  --> src/main.rs:16:1
   |
16 | struct B<'a> {
   | ^ the trait `T` cannot be made into an object

@nrc
Copy link
Member

nrc commented Sep 5, 2016

You could also use the span (probably synthesised) for the whole item + attribute, which might be even better. I think we could improve by noticing when we have a bad span and not showing misleading error messages. I think we already do this to some extent with syntax extensions, and we could be more aggressive about it.

@dtolnay
Copy link
Member Author

dtolnay commented Sep 6, 2016

You could also use the span (probably synthesised) for the whole item + attribute, which might be even better.

I'll try it tonight.

bors added a commit that referenced this issue Sep 11, 2016
Point macros 1.1 errors to the input item

Moved from alexcrichton#6 to continue discussion. Fixes #36218.

Before:

```rust
error[E0106]: missing lifetime specifier
  --> src/main.rs:10:10
   |
10 | #[derive(Serialize, Deserialize)]
   |          ^ expected lifetime parameter

error[E0038]: the trait `T` cannot be made into an object
  --> src/main.rs:15:15
   |
15 | #[derive(Serialize, Deserialize)]
   |          ^^^^^^^^^^ the trait `T` cannot be made into an object
```

After:

```rust
error[E0106]: missing lifetime specifier
  --> src/main.rs:11:1
   |
11 | struct A {
   | ^ expected lifetime parameter

error[E0038]: the trait `T` cannot be made into an object
  --> src/main.rs:16:1
   |
16 | struct B<'a> {
   | ^ the trait `T` cannot be made into an object
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants