-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
self
variable in macros is broken (regression)
#15682
Comments
This is macro hygiene, and is by design: you can't magically capture a variable that the macro can't see when it is defined (very much similar to a normal closure, which can only capture variables that are in scope when the closure is defined). You either need to define the macro inside the method (so that the macro_rules! self_call(
($self_:ident, $name:ident) => ($self_.$name())
)
// ...
fn x(&self) {
self_call!(self, y)
} The reason it worked before is our macro hygiene implementation was not complete (@jbclements has been making good progress on closing the remaining major holes, as the compilation failure of the code in this bug indicates). (Closing as not-a-bug.) |
One follow-up; it's now possible to have macros that expand into methods, so you could also just write macro_rules! self_calling_method(
($name:ident) => (fn x(&self){self.$name()})
)
//...
self_calling_method(y) ... if that's what you wanted. |
@huonw, shame on me, it's so obvious :( I guess it is @jbclements, no, unfortunately, this is not what I needed, but it is nice that it is possible. However, as far as I remember, this can be emulated by expanding into multiple
|
@netvl yes, definitely. |
(Unless the method is a trait method, in which case you need to define all such methods in the single |
@huonw @jbclements Do you know if This would allow us to have some pretty big ergonomic wins through macros if we reconsider this.
vs the potential
Note: we can't even use |
I bet there's an interesting way in which removing this hygiene could cause a nasty future bug. I can't quite put my finger on it, though. |
We do allow nested items, e.g. impl Foo {
fn bar(&self) {
struct Inner;
impl Inner {
fn baz(&self) {}
}
}
} Removing macro_rules! make_method {
($e: expr) => { fn method(&self) { $e } }
} and calls it like fn bar(&self) {
struct Inner;
impl Inner {
make_method!(self.qux()) // meant to be the outer `self`
}
} However, that seems like like a tiny edge case that will rarely crop up. (That said, as a simpler example, I guess being able to write |
@jbclements It's likely! My understanding of the system is shallow :) @huonw The nested case is an issue I hadn't considered, I guess a better suggestion would be to introduce macro_rules! call_foo {
($e:expr) => { $self.foo($e) }
} Where: Any macro that uses Would this suffice for hygiene since |
Is it safe to say this issue has been left aside because the solution @Ryman suggested is very hard to implement? I'd like to make a case for the initial 'just allow self' implementation. The implementation is likely one or two small conditionals in the linter, and it will enable a whole class of macros that are now blocked pending this bug. Subsequently we can immediately file a bug that there is a case where nested implementations cause a problem. This more rare bug can then have a more focused discussion on how to fix it. Even if then it's decided that we should make it $self, it won't be a big problem for macro library authors to add the '$' to their macros when this more complex feature is implemented. |
The current solution is definitely painful. self should not act as an identifier here. |
Recover better on missing parameter in param list We should do the same for argument lists, but that is more tricky to fix.
This code does not compile (playpen), though it should:
This is the error:
This worked previously (it certainly did on June, 8th - this is the last time I compiled my code which used such macro).
The text was updated successfully, but these errors were encountered: