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

Having a variable in scope affects macro invocation parsing #9737

Closed
lilyball opened this issue Oct 6, 2013 · 12 comments
Closed

Having a variable in scope affects macro invocation parsing #9737

lilyball opened this issue Oct 6, 2013 · 12 comments
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-syntaxext Area: Syntax extensions P-medium Medium priority

Comments

@lilyball
Copy link
Contributor

lilyball commented Oct 6, 2013

This is really weird. If I have a macro that expects a particular alphabetic token, and I define a variable with that same token, the macro suddenly stops parsing.

Macro in question:

macro_rules! f!((v: $x:expr) => ( println!("{:?}", $x) ))

I can invoke it like

f!(v: 3);

and it will work just fine. But the following blows up:

let v = 5;
f!(v: 3);
//~^ error: No rules expected the token: v
@emberian
Copy link
Member

cc @paulstansifer @jbclements

@paulstansifer
Copy link
Contributor

This is a consequence of hygiene: the macro is looking for the identifier v (in some sense, the "original" v, even though there's no value corresponding to the "original" v), but let defines a new and different identifier.

This is similar to how keyword arguments behave in Racket:

-> (cond [else 1] [#t 2])
; readline-input:5:6: cond: bad syntax (`else' clause must be last)
-> (let [(else #f)] (cond [else 1] [#t 2]))
2

...but not as sophisticated, as Racket allows you to define a my-else to play the same role as else did.

Of course, "but Racket does it that way" isn't an automatic justification for everything macro-related. Maybe the Rust macro parser would make more sense if we were treating v as a "custom token" instead of an identifier. But I could also see this behavior being confusing if v were in a more "expression-like" position. But maybe in Rust macros, there are fewer "expression-like" positions? @jbclements, any thoughts?

@paulstansifer
Copy link
Contributor

Implementing the behavior that the OP expects would be fairly simple: it'd just mean having the macro parser compare identifiers unhygienically instead of hygienically.

@jbclements
Copy link
Contributor

Yes, that seems right to me. I think we should perform that comparison unhygienically. I have a nagging feeling that this could cause unwanted behavior in sufficiently "interesting" situations--I'm thinking specifically of macros that expand into macro definitions.

Well, this may be a non-issue; currently, even if we fix macros-that-define-macros, there's no way to use a pattern variable in another pattern--it would just be a new binding. To wit:

macro_rules!(($i:ident)=>(macro_rules!(($i : 3)=>(3))))

Anyhow, WRT the bug report: I agree. We should compare these by name, not as identifiers. Is that a one-liner?

... sigh...

@jbclements
Copy link
Contributor

Added a test case for this in my code, haven't made pull request yet.

@emberian
Copy link
Member

emberian commented Apr 7, 2014

Visiting for triage. @jbclements did that ever get submitted/land?

@jbclements
Copy link
Contributor

No, I don't believe so. This still looks like a simple change.

@alexcrichton alexcrichton added the A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) label Nov 17, 2014
@kmcallister kmcallister added I-nominated E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels Mar 3, 2015
@pnkfelix
Copy link
Member

pnkfelix commented Mar 5, 2015

if this gets fixed by 1.0, great. But it doesn't block the release

also, if there is a fix that can be shown to not break any existing code, then this would be classified as "just as bug" and thus such a fix could be landed post 1.0.

P-high, not 1.0.

@pnkfelix pnkfelix added P-medium Medium priority and removed E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels Mar 5, 2015
@pnkfelix pnkfelix added this to the 1.0 milestone Mar 5, 2015
@pnkfelix pnkfelix removed this from the 1.0 milestone Mar 5, 2015
@TimNN
Copy link
Contributor

TimNN commented Aug 14, 2015

I believe this is fixed as the following runs without errors on playpen stable:

macro_rules! f((v: $x:expr) => ( println!("{:?}", $x) ));

fn main() {
    let v = 5;
    f!(v: 3);
}

@pnkfelix
Copy link
Member

pnkfelix commented Jun 10, 2016

(Edit: I am also inclined to close this ticket)

The most obvious variation I could think of that might still exhibit the problem, namely one where the macro is defined within the same scope that binds v, does not exhibit the problem.

That is, compiles and runs:

#![allow(unused_variables)]

macro_rules! f((v: $x:expr) => ( println!("{:?}", $x) ));

fn foo() {
    macro_rules! g((v: $x:expr) => ( println!("{:?}", $x) ));
    f!(v: 3);
    g!(v: 3);
}

fn bar() {
    let v = 4;
    macro_rules! g((v: $x:expr) => ( println!("{:?} {:?}", $x, v) ));  
    let v = 5;
    f!(v: 3);
    g!(v: 3);
}

fn main() {
    foo();
    bar();
}

@jseyfried
Copy link
Contributor

jseyfried commented Jun 10, 2016

@pnkfelix

I am also inclined to close this ticket

Agreed.

@Mark-Simulacrum
Copy link
Member

Closing as per previous comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-syntaxext Area: Syntax extensions P-medium Medium priority
Projects
None yet
Development

No branches or pull requests

10 participants