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

Tracking issue for `proc_macro::Span` inspection APIs #54725

Open
alexcrichton opened this issue Oct 1, 2018 · 12 comments

Comments

Projects
None yet
7 participants
@alexcrichton
Copy link
Member

commented Oct 1, 2018

This issue is intended to track a number of unstable APIs which are used to inspect the contents of a Span for information like the file name, byte position, manufacturing new spans, combining them, etc.

This issue tracks the proc_macro_span unstable feature.

I'm not currently sure what the blockers are for stabilization, but I know there are some! If others would like to fill this in that'd be great!

@softprops

This comment has been minimized.

Copy link

commented Oct 10, 2018

I wanted to shed some pain this is causing with tarpaulin.

Tarpaulin has worked amazingly well for me and my company as a replacement for kcov which historically been a pain to get accurate, reliable and correct results. At the moment tarpaulin stands as the most promising goto option for codecoverage in rust and just feels more like the only first class tooling option.

Having one of those when choosing to invest in a technology is important for many company's adoption story for checking off code quantity checkmarks. When they see that rust doesn't have a reasonable code quality story that works on stable rust, that's can result in a "pass" rather than "I'll take it". There are currently some work arounds for making this work-ish on stable but it feels very much like the story serde was in a year or so ago when you wanted to show all your friends how amazing serde was but then was embarrassed to show what it took to make work on stable because of a macro stabilization blocker.

@JakeTherston

This comment has been minimized.

Copy link

commented Oct 28, 2018

With procedural macros having reached a point where they're very useful on stable, I expect many users will find themselves needing access to this information. Would it be reasonable to only stabilize parts of the Span API that are not too risky? Perhaps exposing a function that optionally returns the path of the file where a macro is invoked if such a file exists?

kevinmehall added a commit to kevinmehall/rust-peg that referenced this issue Nov 3, 2018

Drop dependency on #![feature(proc_macro_span)]
This unstable feature (rust-lang/rust#54725) is the last thing that we
require in Nightly. Removing it will cause a significant regression in
error messages, but that can be improved after switching to parsing the
grammar as tokens rather than as a string literal.

@maghoff maghoff referenced this issue Dec 7, 2018

Open

Relative paths #11

@matklad

This comment has been minimized.

Copy link
Member

commented Feb 1, 2019

I have a concern about exposing LineColum information. It looks like it could play badly with incremental compilation, especially in the IDE context.

My understanding is that, if one adds a blank line to the start of the file, the line_column information of the input spans to proc macro changes. That means that IDE would have to re-expand procedural macros even after insignificant white space changes.

I would feel more confident if proc-macros were strictly a pure function from the input token stream to output token stream. This can be achieved, for example, by making line-column infocation relative to the start of macro invocation (as opposed to relative to the start of the file).

I don't feel that exposing absolute position is necessary the end of the world: IDE can track if a macro actually makes use of the info, or it can supply macro with some fake line/columns. But it's hard to tell if such hacks would work well in practice, and it's hard to experiment with them for the lack of IDE which handled proc-macros incrementally....

@davidlattimore

This comment has been minimized.

Copy link

commented Feb 2, 2019

If the parser allocated IDs to every AST node, then, and this is the hard part, when an edit was made to the source, the parser tried to keep those IDs the same in all the non-edited code and only allocate new IDs for new nodes, that would allow spans to be kept completely separate from the AST. Those IDs could be passed through macro expansion without causing unnecessary invalidations. If something needed a span later on, it could then go back and ask the parser for the span for that particular AST node ID. I feel like having an incremental parser is important, not because parsing is the bottleneck, but because it underpins everything else.

@matklad

This comment has been minimized.

Copy link
Member

commented Feb 2, 2019

@davidlattimore this is fascinating, but slightly off-topic for the issue. I've created a thread on internals: https://internals.rust-lang.org/t/macros-vs-incremental-parsing/9323

@est31

This comment has been minimized.

Copy link
Contributor

commented May 30, 2019

The column!() macro as well as std::panic::Location::column are returning 1-based columns while the span available from the proc-macro crate is 0-based according to its docs. Is this inconsistency intended?

@est31

This comment has been minimized.

Copy link
Contributor

commented May 30, 2019

This thread has more discussion about 1-based columns: #46762 (comment)

@est31

This comment has been minimized.

Copy link
Contributor

commented May 30, 2019

Another open question is how this API relates to #47389 which is about minimizing span information throughout the compiler. Should stabilization be blocked until a design for #47389 is found? Is it too late already as we have set_span functionality? @michaelwoerister what do you think?

@michaelwoerister

This comment has been minimized.

Copy link
Contributor

commented May 31, 2019

#47389 is mostly concerned about data types that are used later in the compilation pipeline, such as type information and MIR. Exposing things at the proc-macro level should not be too bad.

@est31

This comment has been minimized.

Copy link
Contributor

commented Jun 1, 2019

But rust-analyzer might one day expand the scope of incremental compilation to the parsing stage, right?

@matklad

This comment has been minimized.

Copy link
Member

commented Jun 1, 2019

That is already true for rust-analyzer, macro expansion stage runs incrementally. We do’t yet support line/column macro, but I think what we would do is that we’ll just expand them to a dummy value like 42. This probably won’t break macro (It could be this line indeed), but will allow us to avoid re-expanding macro after every keystroke

@est31

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2019

What we could do is store for each macro invocation whether it has invoked any of the inspection APIs or not and then re-run the macro only if any of the span information it has requested has had any changes. Basically only adding the infos the requested spans to the hash (or only adding span info to the hash if there's been any request of span info). However, as this has to update state, it would make span inspection slower than it could be, especially if something inspects spans routinely. So not sure about the performance impacts of this.

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.