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

Backward spans are displayed strangely #42104

Closed
dtolnay opened this Issue May 19, 2017 · 7 comments

Comments

Projects
None yet
4 participants
@dtolnay
Copy link
Member

dtolnay commented May 19, 2017

macro_rules! m {
    ($a:tt $b:tt) => {
        $b $a;
    }
}

fn main() {
    m!(S struct);
}

This is a regression in 1.18.0-beta but unclear whether it affects anything in practice. We encountered this in #rust-libs discussing #40939.

$ rustc +stable --version
rustc 1.17.0 (56124baa9 2017-04-24)
$ cargo +stable build
   Compiling testing v0.1.0
warning: struct is never used: `S`
 --> src/main.rs:8:10
  |
8 |     m!(S struct);
  |     -----^------- in this macro invocation
  |
  = note: #[warn(dead_code)] on by default

    Finished dev [unoptimized + debuginfo] target(s) in 0.34 secs
$ rustc +beta --version
rustc 1.18.0-beta.2 (f4e8e81c3 2017-05-11)
$ cargo +beta build
   Compiling testing v0.1.0
warning: struct is never used: `S`
 --> src/main.rs:8:10
  |
3 | |         $b $a;
  | |______________^
...
8 |       m!(S struct);
  |  _____-----^-------
  | |     |
  | |     in this macro invocation
  |
  = note: #[warn(dead_code)] on by default

    Finished dev [unoptimized + debuginfo] target(s) in 0.45 secs
@dtolnay

This comment has been minimized.

Copy link
Member Author

dtolnay commented May 19, 2017

cc @estebank who touched this code recently.

@estebank

This comment has been minimized.

Copy link
Contributor

estebank commented May 19, 2017

Oh, beautiful... I'm seeing many problems with this output, beyond the obvious. I feel the proper output should either be:

warning: struct is never used: `S`
 --> src/main.rs:8:10
  |
3 |         $b $a;
  |         ^^^^^^ unused struct
...
8 |     m!(S struct);
  |     ------------- in this macro invocation
  |
  = note: #[warn(dead_code)] on by default

or

warning: struct is never used: `S`
 --> src/main.rs:8:10
  |
8 |     m!(S struct);
  |     ---^^^^^^^^-- in this macro invocation
  |        |
  |        unused struct
  = note: #[warn(dead_code)] on by default

I can make (or mentor) a small patch to either swap end and start if they are reversed or only point at the last end for that case, but I'd prefer to properly fix the spans generated in macro invocations in a better way regardless.

@dtolnay

This comment has been minimized.

Copy link
Member Author

dtolnay commented Jun 17, 2017

This made it into 1.18.0.

@brson

This comment has been minimized.

Copy link
Contributor

brson commented Jul 13, 2017

Help wanted. @estebank can mentor.

bors added a commit that referenced this issue Jul 22, 2017

Auto merge of #43352 - estebank:macro-span-replacement, r=petrochenkov
Use the macro structure spans instead of the invocation

Fix #42104, CC #2887.

@bors bors closed this in #43352 Jul 22, 2017

@estebank

This comment has been minimized.

Copy link
Contributor

estebank commented Jul 22, 2017

@brson should this be backported to 1.19? I don't feel there's a need to rush it.

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member

Mark-Simulacrum commented Jul 22, 2017

I'm going to suggest that we don't backport this; it's not high-impact enough.

@estebank

This comment has been minimized.

Copy link
Contributor

estebank commented Jul 22, 2017

That's my thinking as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.