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

Leading :: semantics in macros depend on calling crate, not defining crate. #52848

Closed
alercah opened this issue Jul 30, 2018 · 6 comments
Closed

Comments

@alercah
Copy link
Contributor

alercah commented Jul 30, 2018

Easiest reproduction is to make a 2018 crate (note: rust_2018_preview is not sufficient) that calls the quote! macro:

#![feature(proc_macro_quote, proc_macro_non_items)]
use proc_macro::{quote, TokenStream}

#[proc_macro]
fn macro(input: TokenStream) -> TokenStream {
    quote! { $input }
}

This gives an error: can't find crate for `TokenStream` . Upon closer inspection, the offender is this line, and specifically, ::TokenStream::new().

The issue is that, even though hygiene dictates that this name should be looked up in the defining create, the semantics of leading :: are not accounted for. In the defining crate, in Rust 2015, it means "crate root". In the using crate, Rust 2018, it means "crate registrar" (i.e., the thing that holds crate names).

@petrochenkov, does this impact the stabilization of Span? I don't understand how hygeine works when a path has multiple pieces (identifiers or colons) from different caller/definer contexts. (Aside: I would prefer some elucidation for eventual documentation purposes.)

See also #44660. I don't think it impacts $crate at all, but I'm not sure.

@petrochenkov
Copy link
Contributor

petrochenkov commented Jul 30, 2018

This looks very similar to #50376.

does this impact the stabilization of Span?

Well, yes and no.
On one hand, we are stabilizing only Span::call_site in macros 1.2 that doesn't provide anything fundamentally new compared with long stable proc macro derives.
On another hand, the problem already affects proc macro derives as well.
This rather blocks stabilization of --edition or specific features that need cross-edition compatibility, like the "module reform".

This is all annoying. Edition compatibility is not even part of the language, in theory we could provide it on best-effort basis, but in practice it forces our hand in decisions like #50376.
So many unnecessary problems followed from one single decision to allow large scale breakage / semantic changes across editions at all.

@alercah
Copy link
Contributor Author

alercah commented Jul 30, 2018

Yes, this does look a lot like #50376. As it is, it is definitely blocking the stabilization of proc_macro_quote, but that's an implementation nit that can be tweaked there (and, in fact, I'd appreciate if you could do so).

I unfortunately don't fully understand the current hygiene rules, and in particular how they interact with each other and with new features, in large part because they aren't well-documented. Do you use any live chat medium that I could use to talk things over and pick your brain on? I could put a bunch of thoughts into an issue like this but I imagine much has been tread over already.

(In the meanwhile, I think we should fix proc_macro::quote so that it's usable from 2018.)

@petrochenkov
Copy link
Contributor

Do you use any live chat medium

Not really.
Compiler team is testing out various chats now for possible migration, e.g. https://rust-lang.zulipchat.com, so we can try that.

@djrenren
Copy link
Contributor

@petrochenkov Any update on this?

@petrochenkov
Copy link
Contributor

No updates, but #50376 (comment) and #50376 (comment) describe reasonable rules that can be implemented to address this issue.

bors added a commit that referenced this issue Nov 25, 2018
[beta] resolve: Implement edition hygiene for imports and absolute paths

The changes in behavior of imports and absolute paths are the most significant breaking changes of 2018 edition.
However, these changes are not covered by edition hygiene, so macros defined by 2015 edition crates expanded in 2018 edition crates are still interpreted in the 2018 edition way when they contain imports or absolute paths.
This means the promise of seamless integration of crates built with different editions, including use of macros, doesn't hold fully.
This PR fixes that and implements edition hygiene for imports and absolute paths, so they behave according to the edition in which they were written, even in macros.

### Detailed rules (mostly taken from #50376)
#### Preface
We keep edition data per-span in the compiler. This means each span knows its edition.
Each identifier has a span, so each identifier knows its edition.

#### Absolute paths

Explicitly written absolute paths `::ident::...` are desugared into something like `{{root}}::ident::...` in the compiler, where `{{root}}` is also treated as an identifier.
`{{root}}` inherits its span/hygienic-context from the token `::`.

If the span is from 2015 edition, then `{{root}}` is interpreted as the current crate root (`crate::...` with same hygienic context).
If the span is from 2018 edition, then `{{root}}` is interpreted as "crate universe" (`extern::...`).

#### Imports

To resolve an import `use ident::...` we need to resolve `ident` first.
The idea in this PR is that `ident` fully knows how to resolve itself.

If `ident`'s span is from 2015 edition, then the identifier is resolved in the current crate root (effectively `crate::ident::...` where `crate` has the same hygienic context as `ident`).
If `ident`'s span is from 2018 edition, then the identifier is resolved in the current scope, without prepending anything (well, at least with uniform paths).

There's one corner case in which there's no identifier - prefix-less glob import `use *;`.
In this case the span is inherited from the token `*`.
`use *;` && `is_2015(span(*))` -> `use crate::*;` && `span(crate) == span(*)`.
`use *;` && `is_2018(span(*))` -> error.

---
Why beta:
	- Compatibility of 2015 edition crates with 2018 edition crates, including macros, is one of the main promises of the edition initiative.
	- This is technically a breaking change for 2018 edition crates or crates depending on 2018 edition crates.
	- ~This PR is based on #55884 which hasn't landed on nightly yet :)~ No longer true (#56042).

Previous attempt #50999
Closes #50376
Closes #52848
Closes #53007

r? @ghost
@petrochenkov
Copy link
Contributor

Closed by #56053

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

No branches or pull requests

3 participants