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

Expose span locations on stable #166

Merged
merged 1 commit into from Jan 31, 2019

Conversation

4 participants
@dtolnay
Copy link
Collaborator

dtolnay commented Jan 28, 2019

In the way suggested by #165 (comment).

Show resolved Hide resolved src/fallback.rs Outdated
@mitsuhiko

This comment was marked as resolved.

Copy link

mitsuhiko commented Jan 28, 2019

Currently fails like this with the feature enabled:

error[E0433]: failed to resolve: use of undeclared type or module `RefCell`
   --> /Users/mitsuhiko/.cargo/git/checkouts/proc-macro2-bc9077ffc3c20345/f24566c/src/fallback.rs:228:40
    |
228 |     static CODEMAP: RefCell<Codemap> = RefCell::new(Codemap {
    |                                        ^^^^^^^ use of undeclared type or module `RefCell`

error[E0412]: cannot find type `RefCell` in this scope
   --> /Users/mitsuhiko/.cargo/git/checkouts/proc-macro2-bc9077ffc3c20345/f24566c/src/fallback.rs:228:21
    |
228 |     static CODEMAP: RefCell<Codemap> = RefCell::new(Codemap {
    |                     ^^^^^^^ not found in this scope
help: possible candidate is found in another module, you can import it into scope
    |
5   | use std::cell::RefCell;
    |

error[E0412]: cannot find type `Codemap` in this scope
   --> /Users/mitsuhiko/.cargo/git/checkouts/proc-macro2-bc9077ffc3c20345/f24566c/src/fallback.rs:228:29
    |
228 |     static CODEMAP: RefCell<Codemap> = RefCell::new(Codemap {
    |                             ^^^^^^^ not found in this scope

error[E0422]: cannot find struct, variant or union type `Codemap` in this scope
   --> /Users/mitsuhiko/.cargo/git/checkouts/proc-macro2-bc9077ffc3c20345/f24566c/src/fallback.rs:228:53
    |
228 |     static CODEMAP: RefCell<Codemap> = RefCell::new(Codemap {
    |                                                     ^^^^^^^ not found in this scope

error[E0422]: cannot find struct, variant or union type `FileInfo` in this scope
   --> /Users/mitsuhiko/.cargo/git/checkouts/proc-macro2-bc9077ffc3c20345/f24566c/src/fallback.rs:231:21
    |
231 |         files: vec![FileInfo {
    |                     ^^^^^^^^ not found in this scope

error[E0609]: no field `lo` on type `&fallback::Span`
   --> /Users/mitsuhiko/.cargo/git/checkouts/proc-macro2-bc9077ffc3c20345/f24566c/src/fallback.rs:379:40
    |
379 |             fi.offset_line_column(self.lo as usize)
    |                                        ^^

error[E0609]: no field `hi` on type `&fallback::Span`
   --> /Users/mitsuhiko/.cargo/git/checkouts/proc-macro2-bc9077ffc3c20345/f24566c/src/fallback.rs:388:40
    |
388 |             fi.offset_line_column(self.hi as usize)
    |                                        ^^

error[E0560]: struct `fallback::Span` has no field named `lo`
   --> /Users/mitsuhiko/.cargo/git/checkouts/proc-macro2-bc9077ffc3c20345/f24566c/src/fallback.rs:233:26
    |
233 |             span: Span { lo: 0, hi: 0 },
    |                          ^^ `fallback::Span` does not have this field

error[E0560]: struct `fallback::Span` has no field named `hi`
   --> /Users/mitsuhiko/.cargo/git/checkouts/proc-macro2-bc9077ffc3c20345/f24566c/src/fallback.rs:233:33
    |
233 |             span: Span { lo: 0, hi: 0 },
    |                                 ^^ `fallback::Span` does not have this field

error: aborting due to 9 previous errors

@dtolnay dtolnay force-pushed the dtolnay:location branch from f24566c to 383649a Jan 28, 2019

@mitsuhiko

This comment has been minimized.

Copy link

mitsuhiko commented Jan 28, 2019

Can confirm this works!

@dtolnay dtolnay added the wip label Jan 28, 2019

@dtolnay

This comment has been minimized.

Copy link
Collaborator

dtolnay commented Jan 28, 2019

Tagging wip because this needs tests. Does not work when I tried it:

use proc_macro2::TokenTree;

fn main() {
    let sp = syn::parse_str::<TokenTree>("testing").unwrap().span();
    println!("start line={} column={}", sp.start().line, sp.start().column);
    println!("end line={} column={}", sp.end().line, sp.end().column);
}
start line=1 column=0
end line=1 column=0

I am not going to be able to get to this today. @mitsuhiko could you investigate?

@mitsuhiko

This comment has been minimized.

Copy link

mitsuhiko commented Jan 28, 2019

Yes, will investigate. Information seems absent indeed.

@alexcrichton

This comment has been minimized.

Copy link
Owner

alexcrichton commented Jan 28, 2019

If we're going to offer this in a published stable version then I think we can probably go ahead and do this without a feature. The proc-macro2 crate can't have many major version bumps due to the degree of reliance on it through syn in the proc-macro ecosystem, so being stable is a big commitment. If we're willing to commit to this as a stable feature then I think it's worth providing by default.

I think that may also simplify the cfg story here (which I am always confused by every time I look at this!). That way outside of a proc-macro context you always have accurate information, but in a proc-macro context you'll only have accurate information if a nightly feature toggle is enabled.

@dtolnay

This comment has been minimized.

Copy link
Collaborator

dtolnay commented Jan 28, 2019

I kept a feature because last time I checked there was a big performance hit for Span going from 0 bytes to 8 bytes. There are a lot of spans in the syntax tree...

@alexcrichton

This comment has been minimized.

Copy link
Owner

alexcrichton commented Jan 28, 2019

Ah ok that makes sense to me, and in that case seems fine to document as such as to why it's a feature

@mystor

This comment has been minimized.

Copy link
Contributor

mystor commented Jan 28, 2019

Hmm, I wonder if a span interning system could compress span sizes down even more, and if that would be helpful here.

@dtolnay dtolnay force-pushed the dtolnay:location branch from 383649a to 6078302 Jan 28, 2019

@dtolnay

This comment has been minimized.

Copy link
Collaborator

dtolnay commented Jan 28, 2019

I added a note in build.rs about the performance hit.

@dtolnay dtolnay force-pushed the dtolnay:location branch from 6078302 to f7b7f73 Jan 28, 2019

@dtolnay dtolnay removed the wip label Jan 28, 2019

@dtolnay dtolnay force-pushed the dtolnay:location branch from f7b7f73 to 3b1f7d2 Jan 28, 2019

@dtolnay

This comment has been minimized.

Copy link
Collaborator

dtolnay commented Jan 28, 2019

I think this works now.

I filed #167 to follow up on mitigating the performance impact.

@mitsuhiko

This comment has been minimized.

Copy link

mitsuhiko commented Jan 31, 2019

Is this blocked on #167 or is it mergeable as such?

@dtolnay

This comment has been minimized.

Copy link
Collaborator

dtolnay commented Jan 31, 2019

I don't think this needs to block on the performance work, because the increase in size of Span only happens if you opt in to the new feature. Thanks for the reminder!

@dtolnay dtolnay merged commit 2b01a3f into alexcrichton:master Jan 31, 2019

@dtolnay dtolnay deleted the dtolnay:location branch Jan 31, 2019

@dtolnay

This comment has been minimized.

Copy link
Collaborator

dtolnay commented Jan 31, 2019

Published in 0.4.27.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment