Attribute macros invoked at crate root have issues #41430
Comments
|
cc @jseyfried |
|
@abonander What about this? #![feature(proc_macro)]
extern crate foo_macros;
use foo_macros::foo_attr;
mod bar {
#![foo_attr]
}If compiles, then the issue is that the attribute is at the crate root, not that it is an inner attribute. |
|
@jseyfried Updated issue as this only affects the crate root |
Adds temporary regression test; this ideally should work as-is (rust-lang#41430) Closes rust-lang#41211
abonander
changed the title
|
So @jseyfried and I figured out on IRC that this is due to the compiler trying to expand the attribute before the My best idea is along the lines of this: if an attribute macro fails to resolve at crate root only (since inner attributes on modules should resolve in the parent's scope), import-process root, get the resolution for the attribute, reinitialize the resolver and copy the resolution for the attribute macro, then expand the root module. Some concerns from @jseyfried:
cc @nrc |
Don't panic if an attribute macro fails to resolve at crate root Adds temporary regression test; this ideally should work as-is (rust-lang#41430) Closes rust-lang#41211 r? @jseyfried
Don't panic if an attribute macro fails to resolve at crate root Adds temporary regression test; this ideally should work as-is (rust-lang#41430) Closes rust-lang#41211 r? @jseyfried
Don't panic if an attribute macro fails to resolve at crate root Adds temporary regression test; this ideally should work as-is (rust-lang#41430) Closes rust-lang#41211 r? @jseyfried
|
@nrc Some input here when you get the chance, I'd like to tackle this soonish. |
|
sorry it took a while to see this - I was away on parental leave and then had to nuke my notifications.
I wonder if this axiom is correct? An alternative seems to be that we could resolve attributes at the scope in which they are written rather than the scope in which they apply. Would that have knock-on effects? |
Interesting, I like this idea. I'm not aware of any knock-on effects, it should be straightforward to implement, and it would address this use case. |
|
I was under the impression that inner attributes were syntactic sugar for outer attributes, and it makes sense for outer attributes to resolve in the same scope as the item they're applied to. Crates are just special because there's no outer scope to resolve. |
Whether a feature is desugared or not is an implementation detail and we should do what makes the most sense from the user's POV, not the compilers. Desugaring can take place after name resolution if we like, so we don't need to change the implementation either. |
|
So the plan of action is to just reorder attribute expansion and import processing for a given item? Or just at the crate root for now? |
|
@abonander I wouldn't think of this as reordering attribute expansion or import processing, just changing the scope in which e.g. |
|
If we make this change, I think we should make the change everywhere (not just at the crate root) for consistency. |
|
#44660 improves the situation. The following compiles okay on nightly: I believe the absoluteness of the path can be omitted in the future, once the RFC 2126 implementation proceeds further. However #45458 still makes using the crate-level macros difficult. The issue concerns compiler plugins, but can also be reproduced through attribute macros. |
|
Moving from #48066 as per @Manishearth wishes. Although I would prefer if the title of this issue was updated - the resolution itself is somewhat "solved" in nightly. Even if the current While normal proc-macro attribute work, crate level proc-macro attributes seem to have several issues. The clearest of these is the order of the attributes and Some of these issues are due to lack of library support (syn), but some are caused by rustc itself. Identity macro ✔️First of all: The following proc-macro works just fine with nightly 29c8276: #[proc_macro_attribute]
fn attribute(_: TokenStream, input: TokenStream) -> TokenStream {
input
}#![feature(extern_absolute_paths)]
#![::my_crate::attribute]
fn main() {}Round trip through FromIterator ❌As long as the macro doesn't touch the input, everything works just fine. However this isn't really that useful for a macro. Once we try doing anything with the input token stream (short of a #[proc_macro_attribute]
fn attribute(_: TokenStream, input: TokenStream) -> TokenStream {
TokenStream::from_iter( input.into_iter() )
}#![feature(extern_absolute_paths)]
#![::my_crate::attribute]
fn main() {}This results in the following error message The reason seems to be that the pub mod {
...
}This is most likely caused by the Stripping the
|
Manishearth
changed the title
|
A crate's token-stream should look exactly like the source; we shouldn't be emitting |
|
I think the solution here would be to introduce |
|
Or would it be preferable to pass a syntatically correct representation of the crate as some |
I believe syn already handles "crate" properly as https://docs.rs/syn/0.12/syn/struct.File.html
From a user perspective, I found the So while a token stream without |
|
I think for sanity's sake we should pass the crate's tokens just as they appear in the source, which would mean handling a crate not like a module but like its own thing. |
|
In #50101 @alexcrichton stated that he doesn't expect support for proc-macros on the crate root to be stabilized in the foreseeable future, that we should address the possible use-cases individually instead of allowing an attribute to completely rewrite the crate. |
|
The use case that prompted me to examine this in the first place was prototyping parts of the test framework pre-RFC. That RFC suggested a whole-crate proc-macro as one option for custom test framework. It even goes as far as stating the following for one of the options:
Currently the general understanding among people seems to be that inner attribute proc macros not working on modules and crates is a bug instead of an unsupported feature. If I remember right they do work on modules after all; Allowing people to wrap the whole crate in a Though I do appreciate @alexcrichton's concerns over this issue. Just to keep the discussion here I'll copy his response from #50101 here:
|
|
FWIW for the test frameworks thing we can make it work without explicitly
having this feature, what test frameworks need is a way to apply a
whole-crate proc macro _as an argument to rustc_. This is less tricky.
…On Fri, Apr 27, 2018, 2:01 AM Mikko Rantanen ***@***.***> wrote:
The use case that prompted me to examine this in the first place was
prototyping parts of the test framework pre-RFC. That RFC suggested a
whole-crate proc-macro as one option for custom test framework. It even
goes as far as stating the following for one of the options:
This assumes that #![foo] ("inner attribute") macros work on modules and
on crates.
Currently the general understanding among people seems to be that inner
attribute proc macros not working on modules and crates is a bug instead of
an unsupported feature. If I remember right they do work on modules after
all; Allowing people to wrap the whole crate in a mod statement to
achieve the same result.
Though I do appreciate @alexcrichton <https://github.com/alexcrichton>'s
concerns over this issue. Just to keep the discussion here I'll copy his
response from #50101 <#50101> here:
Er sorry yeah, let me clarify. Right now for "Macros 1.2" were very
unlikely to stabilize attribute invocations on modules. It's a weird
question of what does #[foo] mod foo; receive? Does it receive mod foo ;?
The contents of the module foo?
Something like entire crate expansion also has huge repercussions on
hygiene as well as whether it's feasible/quick. Entire crate expansion runs
a risk of being extremely slow (we have to serialize back and forth with
the procedural macro) and highly non-incremental (it's a complete black box
to the compiler). I'd imagine that we're very far away from stabilizing
this functionality, if at all.
In that sense I'd personally be wary of landing this functionality in the
compiler, even on nightly. I think it'd be best to take other avenues of
attack for use cases that would otherwise require entire-crate expansion.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#41430 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABivSJMruLX34G06FmxGus3Hgf78cU7oks5tst5WgaJpZM4NDpSo>
.
|
|
@Manishearth Doesn't that result in the same hygiene/incremental issues? Referring here to @alexcrichton's comment:
I know one of the options for the test framework RFC is to just generate a main function. The "Only add stuff"-kind of proc macro does somewhat circumvent the hygiene/incrmental compilation issues, but that's just one option mentioned in the RFC - all the other options face the same issues I'd imagine. Essentially what I'm after here is: If those issues are solved/resolved/accepeted in relation to the the test RFC, doesn't that apply here as well? Although what I suspect is that given the issue raised here, the test RFC will end up going with the "just generate main()" option. |
|
Slow and non-incremental matters less for testing. "just add a main function" is none of the options, all of the options already have to run some kind of whole crate proc macro to rewrite things to be The hygiene issue is still a thing, but if folks just use the "generate main" API we put on crates.io that's still not a problem. And most of the frameworks which wish to do something more complex than that probably won't need to worry as much about hygiene. |
|
Well, that's not entirely true -- the "just generate main" API would have less of a serialization overhead. But that's about it. |
|
Another reason that I don't think that this is a great idea is #43081 right now. Any change to the AST would lose span information for the entire crate. I think that's basically a showstopper until we fix that. |
jonas-schievink
added
A-attributes
Could someone point me to the code that does this? (I could not find it myself.) We want to update the Prusti verifier to work with the latest version of the Rust compiler. However, one of the blocking issues we have is that I cannot find any more a way to obtain a mutable reference into an AST. We used to rewrite AST to type-check specifications. Treating each specification as a separate procedural macro invocation is very problematic because we need to maintain a global state. |
|
Since #43081 (comment) mentions that #43081 is likely to be fixed soon for all stable Rust syntax we can reiterate on this issue. I'd love to hear what else is blocking this feature and if there are any other show stoppers, bugs or design considerations before we can get there. |
When invoking some attribute procedural macro with inner form, i.e.
#![foo_attr]at the crate root, a resolution error occurs:This should, ideally, resolve and execute properly.
The text was updated successfully, but these errors were encountered: