-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
ctxts on macro invocations won't work in some places #8063
Comments
triage bump |
I still think both of these solutions would work fine. Is this coming up in practice? |
(triage) I don't exactly understand the problem here (i.e. will it cause a failure of hygiene?); @jbclements, do you happen to have a code example demonstrating it? |
I seem to have stumbled upon this same issue, this gist shows it. I get an "unresolved name a" on the linked line, which doesn't happen if I change it to something like |
cc me |
@uorbe001 no... that doesn't sound like this issue. I'm working on reproducing your issue.... |
@jbclements any way I can help? I can try to make a simplified code sample if that helps |
@uorbe001 wurf... I can't reproduce your issue without all of your code. Looks like there's a dependency on 'rustspec'. Apologies if I'm missing something obvious. |
@jbclements I'll try to reproduce it without it in a few days, I'm a bit busy atm ;) |
@jbclements I managed to reproduce it without my library, the code is kind of messy, but it fails. I've been trying to remove stuff that might not be important for this code, but I'm kind of stuck, if you can see something that is superfluous and can make it easier to read by removing it, let me know and I'll update the gist: https://gist.github.com/uorbe001/1118f22c378d2e382d29 I think the problem might be related to the fact that I'm calling Does this sound like part of this issue, or should I add the gist on a different one? |
Ah! You’re cheating :). Put differently: this isn’t a simple pattern macro, you’re messing around with tokens directly. Can you help me out a bit more and tell me what this macro (mymacro!) is supposed to do? I can partially see it, but maybe you can spell it out explicitly for me? |
The macro is supposed to allow me to define contexts for different test cases to reuse setup/dropdown methods and provide more context for each test case. It's essentially a port of rspec and similar frameworks from other languages, you can see how I intend to use it here: https://github.com/uorbe001/rustspec/blob/master/tests/test.rs#L20 |
Okay, that makes sense. First thing: this is almost certainly not related to this issue. Second thing: something like this should work, yes; hygiene should be working with you, here, not against you. It should be possible to rearrange the pieces at will, and "just work". So I do think that there's a hygiene issue here, probably related (as you point out) to the re-parsing of the trees. Can you open a separate issue for this? |
@jbclements done ;-) |
This bug has long been open, and there hasn't been a lot of recent discussion. Macros aren't @jbclements , a reproduction would be nice if you still think this is a relevant issue. Thanks! |
@steveklabnik , is there a macros czar? What's the state of macros in Rust these days? |
Whoah, first of all, sorry for the half-sentence in the previous mention. What I was going to say is relevant to your question:
Basically, we reserved a rust-lang/rfcs#440 is the tracking issue for macros++. That said, if people are interested in fixing macro issues, we're accepting improvements. I'm fine with re-opening this issue, it was just a year since the last bit of discussion and no direct reproduction, so I figured it was closeable. |
Okay, sounds good. Thanks! John |
The current scheme of attaching contexts to macro invocations won't work correctly when that macro invocation is inside of another macro definition, because that definition will be represented only as token trees, and the macro invocation won't accumulate context correctly.
There are a couple of possible fixes for this. One would be to back out the existing scheme and just use the context associated with the macro invocation identifier--that is, the 'foo' in an invocation foo!(3,4,5). A better way of doing it--if it's not too expensive--would be to add a context to every tt_delim, and then transfer it correctly to a tt_delim that parses into a macro invocation.
The text was updated successfully, but these errors were encountered: