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

syntax: Unsupport foo! bar { ... } macros in the parser #62258

Merged
merged 2 commits into from
Jul 4, 2019

Conversation

petrochenkov
Copy link
Contributor

Their support in expansion was removed in #61606.

Also un-reserve macro_rules as a macro name, there's no ambiguity between macro_rules definitions and macro calls (it also wasn't reserved correctly).

cc rust-lang/wg-grammar#51

@rust-highfive
Copy link
Collaborator

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 30, 2019
Copy link
Contributor

@Centril Centril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me with or without nits.

src/libsyntax/parse/parser.rs Outdated Show resolved Hide resolved
src/libsyntax/parse/parser.rs Outdated Show resolved Hide resolved
@Centril
Copy link
Contributor

Centril commented Jun 30, 2019

r? @Centril

@rust-highfive rust-highfive assigned Centril and unassigned eddyb Jun 30, 2019
@eddyb
Copy link
Member

eddyb commented Jul 1, 2019

cc @rust-lang/lang This PR lets one define macros named macro_rules which would be used for an invocation like macro_rules!(...), while macro_rules! foo {...} would still refer to the builtin macro.

While from an implementation point of view this seems reasonable (treating macro_rules! foo {...} as a special syntax and not a macro invocation), I do want to make sure this is not a compatibility hazard (in case we want to, say, allow something like macro foo = macro_rules! {...}; in the future).

That said, I think we can land this PR as-is, and revert it later if we really don't want to do this.

@eddyb eddyb added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jul 1, 2019
@petrochenkov
Copy link
Contributor Author

Two notes:

  • macro_rules was already a special syntax, not a macro invocation, this PR doesn't change that
  • the "reservation" could be worked around by use foo as macro_rules previously
    macro_rules! foo { () => (0) }
    
    use foo as macro_rules;
    
    fn main() {
        println!("{}", macro_rules!());
    }

@eddyb
Copy link
Member

eddyb commented Jul 1, 2019

@petrochenkov Oh, and that use wouldn't affect macro_rules! ident {...} syntax anyway?
I can see how that makes this PR even more of a slam-dunk.

Like I said, I think we should just land it and if anyone has any concerns, they can be raised async.

@petrochenkov
Copy link
Contributor Author

petrochenkov commented Jul 1, 2019

Oh, and that use wouldn't affect macro_rules! ident {...} syntax anyway?

Yes, macro_rules in macro_rules! foo { ... } doesn't go through name resolution, since it's a context-dependent keyword like union and processed during parsing.
https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=aa35ddb773e577d4d2b9842d3f0059d1

@petrochenkov
Copy link
Contributor Author

@bors r=Centril

@bors
Copy link
Contributor

bors commented Jul 1, 2019

📌 Commit d0dc41a has been approved by Centril

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 1, 2019
@Centril
Copy link
Contributor

Centril commented Jul 1, 2019

Removing nomination per @eddyb's note re. async and since the whole team was pinged.

(It's clear to me that this is a slam dunk)

Centril added a commit to Centril/rust that referenced this pull request Jul 1, 2019
syntax: Unsupport `foo! bar { ... }` macros in the parser

Their support in expansion was removed in rust-lang#61606.

Also un-reserve `macro_rules` as a macro name, there's no ambiguity between `macro_rules` definitions and macro calls (it also wasn't reserved correctly).

cc rust-lang/wg-grammar#51
Centril added a commit to Centril/rust that referenced this pull request Jul 3, 2019
syntax: Unsupport `foo! bar { ... }` macros in the parser

Their support in expansion was removed in rust-lang#61606.

Also un-reserve `macro_rules` as a macro name, there's no ambiguity between `macro_rules` definitions and macro calls (it also wasn't reserved correctly).

cc rust-lang/wg-grammar#51
Centril added a commit to Centril/rust that referenced this pull request Jul 3, 2019
syntax: Unsupport `foo! bar { ... }` macros in the parser

Their support in expansion was removed in rust-lang#61606.

Also un-reserve `macro_rules` as a macro name, there's no ambiguity between `macro_rules` definitions and macro calls (it also wasn't reserved correctly).

cc rust-lang/wg-grammar#51
Centril added a commit to Centril/rust that referenced this pull request Jul 3, 2019
syntax: Unsupport `foo! bar { ... }` macros in the parser

Their support in expansion was removed in rust-lang#61606.

Also un-reserve `macro_rules` as a macro name, there's no ambiguity between `macro_rules` definitions and macro calls (it also wasn't reserved correctly).

cc rust-lang/wg-grammar#51
bors added a commit that referenced this pull request Jul 3, 2019
Rollup of 17 pull requests

Successful merges:

 - #62039 (Remove needless lifetimes (rustc))
 - #62153 (Update the `rust-installer` submodule)
 - #62173 (rename InterpretCx -> InterpCx)
 - #62240 (wfcheck: resolve the type-vars in `AdtField` types)
 - #62249 (Use mem::take instead of mem::replace with default)
 - #62252 (Update mem::replace example to not be identical to mem::take)
 - #62258 (syntax: Unsupport `foo! bar { ... }` macros in the parser)
 - #62268 (Clean up inherent_impls)
 - #62287 (Use link attributes on extern "C" blocks with llvm-libuwind)
 - #62295 (miri realloc: do not require giving old size+align)
 - #62297 (refactor check_for_substitution)
 - #62316 (When possible without changing semantics, implement Iterator::last in terms of DoubleEndedIterator::next_back for types in liballoc and libcore.)
 - #62317 (Migrate `compile-pass` annotations to `build-pass`)
 - #62337 (Fix bucket in CPU usage script)
 - #62344 (simplify Option::get_or_insert)
 - #62346 (enable a few more tests in Miri and update the comment for others)
 - #62351 (remove bogus example from drop_in_place)

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Jul 3, 2019
syntax: Unsupport `foo! bar { ... }` macros in the parser

Their support in expansion was removed in rust-lang#61606.

Also un-reserve `macro_rules` as a macro name, there's no ambiguity between `macro_rules` definitions and macro calls (it also wasn't reserved correctly).

cc rust-lang/wg-grammar#51
bors added a commit that referenced this pull request Jul 3, 2019
Rollup of 16 pull requests

Successful merges:

 - #62039 (Remove needless lifetimes (rustc))
 - #62173 (rename InterpretCx -> InterpCx)
 - #62240 (wfcheck: resolve the type-vars in `AdtField` types)
 - #62249 (Use mem::take instead of mem::replace with default)
 - #62252 (Update mem::replace example to not be identical to mem::take)
 - #62258 (syntax: Unsupport `foo! bar { ... }` macros in the parser)
 - #62268 (Clean up inherent_impls)
 - #62287 (Use link attributes on extern "C" blocks with llvm-libuwind)
 - #62295 (miri realloc: do not require giving old size+align)
 - #62297 (refactor check_for_substitution)
 - #62316 (When possible without changing semantics, implement Iterator::last in terms of DoubleEndedIterator::next_back for types in liballoc and libcore.)
 - #62317 (Migrate `compile-pass` annotations to `build-pass`)
 - #62337 (Fix bucket in CPU usage script)
 - #62344 (simplify Option::get_or_insert)
 - #62346 (enable a few more tests in Miri and update the comment for others)
 - #62351 (remove bogus example from drop_in_place)

Failed merges:

r? @ghost
@bors bors merged commit d0dc41a into rust-lang:master Jul 4, 2019
@petrochenkov petrochenkov deleted the idclean branch July 8, 2019 13:33
topecongiro added a commit to topecongiro/rustfmt that referenced this pull request Jul 28, 2019
This is now unsupported and treated as a hard syntax error.
cc rust-lang/rust#62258.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants