Consider providing meaningful Spans in stable implementation

[dtolnay]

It looks like cpp_syn uses low and high indices into the input string like so:

pub struct Span {
    pub lo: usize,
    pub hi: usize,
}

@mystor if we are able to provide the same thing here, would that eliminate the need for cpp_syn?

[mystor]

@dtolnay If we provide a way to access these indices then I imagine I could use this instead of cpp_syn. Currently I am using them to provide better error messages when parsing entire crates and to handle c++-only operators better (by telling if, for example, i++ is i + + or i ++ - this is probably handled by the new format for operators in TokenStreams).

I'm not sure if we would want to expose that implementation detail which we could only expose from the stable half from proc_macro2 though. I imagine that it might be easier to instead try to prototype some APIs which we could try to get into proc_macro proper for dealing with rustc's Codemap (and have a fake Codemap inside of proc_macro2::stable). Not sure.

What do you think @alexcrichton?

[alexcrichton]

Yeah I'm slightly wary of the wrt the intention that this crate just goes away in the future once proc_macro has a stable API that's larger. The "default" mode of this crate will then just be "reexport proc_macro" and there'd be no way for us to add this functionality on top (e.g. have an additive cargo feature).

I don't really mind hacking around it today though, we could have a Cargo feature that's basically flagged "this is unlikely to work in all situations, use at your own peril"

[mystor]

I don't really mind hacking around it today though, we could have a Cargo feature that's basically flagged "this is unlikely to work in all situations, use at your own peril"

I definitely don't want to export any APIs from proc-macro2 which we don't think are going to at the very least have equivalents in proc-macro before the new API is stable. I think we could potentially come up with an API for consuming spans (say Span::source_file_line_column(&self) -> Option<(String, usize, usize)> which produces the file information, Span::raw_source_text(&self) -> Option<String> to get the raw text for a span, and proc_macro2::parse_file<P: AsRef<Path>>(p: P) -> io::Result<TokenStream> to parse a file (which would have stuff like filename and line information metadata so that tokens from it get nice spans)?), which will likely not appear exactly as we implement in proc-macro proper, but something like it will probably be added.

If we come up with nice to use APIs, we could potentially contribute them back to proc-macro once rust-lang/rust#40939 has been merged.

[alexcrichton]

That seems plausible to me, yeah, but aren't there questions about where these contexts are stored and such? E.g. tls, handles, pointers, etc

[mystor]

I think that the current api for proc-macro has basically made that decision for us. FromStr and the functions for proc_macro_derive don't provide any way to pass in a context from the compiler, meaning that the compiler must place its context into TLS, and that at least some means of parsing into TokenStreams will read from TLS.

That leaves 4 major options:

1. Provide methods for doing these parsing operations on a Context object. When running the compiler, there is a method to pull this object out of TLS. FromStr pulls its context from TLS implicitly for backwards compatibility reasons, and is unusable outside of a proc-macro crate.

2. Provide these methods as static methods or methods on static objects. When these methods are run, pull the Context internally out of TLS and use it. Provide some mechanism for initializing and de-initializing the Context in TLS, potentially with a RAII guard object.

3. Like 2, but lazily initialize a Context object in TLS when it is requested instead and lives until the thread dies, or

4. Like 3, but you explicitly initialize the context (with some sort of proc_macro::init() method - this could be useful if we have initialization-time configuration options which we need to let the caller pass), and it lives until the thread dies.

The first has the problem that now all code inside of proc-macro needs to be able to deal with receiving Tokens, Spans, etc. from a different context, for example of you read in a crate with context A, but append that TokenStream to a TokenStream read in in context B. I think that this would be unnecessary overhead and a bit overkill.

The second has the same problem. So long as we let the caller control the Context object, and don't have some sort of lifetime parameter tying TokenStream objects to that Context, we run into the issue that you can mix tokens and spans between contexts, which is undesirable. It's also a very ugly not-rust-like API.

I think that the only practical implementation option is either the third or the fourth, where the context is initialized when requested, and persists until the thread dies. That way we can make methods infallible, the API is relatively easy to use, and make it statically impossible to mix TokenStreams parsed by different Contexts.

If a crate wants to clean up its Context objects, it'll have to use a new thread to do so, but that is probably worth it to avoid doing some implementation gymnastics to allow for threads to have multiple Contexts.

[alexcrichton]

Hm sorry I think I'm super lost now at this point. Want to just send a PR with what you're thinking? I don't really have many opinions here per-se, this crate is basically just a stopgap until proc_macro is itself stable.

[mystor]

I think the best way to do this would be for me to make a PR to rust-lang/rust after the new API lands with my suggested API, get feedback on it there, and then perhaps mirror it into proc_macro2 if feedback looks positive. I'd have to either implement a codemap inside of proc_macro2, or copy over the one I wrote for fauxenstream, and then make the parser location aware in order to implement this, and it seems a bit silly to do that work for an API which won't appear in proc-macro.

[alexcrichton]

Makes sense to me!

[dtolnay]

Implemented in #36.