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

Tracking issue for adding a `lifetime` specifier to `macro_rules!` #34303

Closed
nikomatsakis opened this Issue Jun 16, 2016 · 25 comments

Comments

Projects
None yet
@nikomatsakis
Contributor

nikomatsakis commented Jun 16, 2016

@sgrif

This comment has been minimized.

Contributor

sgrif commented Jun 16, 2016

I'll open a new PR soon, I want to do one more thing on top of #33135

@Phlosioneer

This comment has been minimized.

Contributor

Phlosioneer commented Jun 21, 2016

Godspeed, @sgrif! This being in nightly would make my week.

@comex

This comment has been minimized.

Contributor

comex commented Jan 3, 2017

@sgrif Any update?

@brson

This comment has been minimized.

Contributor

brson commented Mar 1, 2017

This doesn't seem particularly hard. Help wanted.

@durka

This comment has been minimized.

Contributor

durka commented Mar 1, 2017

I wish we knew what the "one more thing" was besides just reopening the closed PR.

@chriskrycho chriskrycho referenced this issue Mar 29, 2017

Closed

Document all features #9

18 of 48 tasks complete
@mikeyhew

This comment has been minimized.

Contributor

mikeyhew commented May 4, 2017

I'd like to help implement this. Where do I start?

@petrochenkov

This comment has been minimized.

Contributor

petrochenkov commented May 4, 2017

@mikeyhew
This is a good issue to start working on the compiler frontend.
You may want to start with taking @sgrif's patch and rebasing it on the current master, and then making sure the tests pass.
Then you'll have to add a feature gate for lifetime matcher and some docs (unstable book section) (see e0cd766, and 1d46805, and #41012 as a whole for examples of the same things being done with vis matcher.)

@durka

I wish we knew what the "one more thing" was besides just reopening the closed PR.

This should be something truly marvelous, that this GitHub comment was too narrow to contain.

@ricochet1k ricochet1k referenced this issue Dec 20, 2017

Merged

Allow lifetimes in macros #46895

1 of 1 task complete

bors added a commit that referenced this issue Jan 1, 2018

Auto merge of #46895 - ricochet1k:macro-lifetimes, r=jseyfried
Allow lifetimes in macros

This is a resurrection of PR #41927 which was a resurrection of #33135, which is intended to fix #34303.

In short, this allows macros_rules! to use :lifetime as a matcher to match 'lifetimes.

Still to do:
- [x]  Feature gate

@bors bors closed this in #46895 Jan 1, 2018

@sgrif

This comment has been minimized.

Contributor

sgrif commented Jan 1, 2018

This issue should remain open until the feature is stable, right?

@petrochenkov petrochenkov reopened this Jan 1, 2018

@cramertj

This comment has been minimized.

Member

cramertj commented Mar 9, 2018

@rfcbot fcp merge

AFAIK this feature has gone relatively without-incident since its implementation, and it adds key functionality to macros. Tests can be found here, here, here, and here.

@petrochenkov

This comment has been minimized.

Contributor

petrochenkov commented Mar 10, 2018

Some alternatives are discussed in https://internals.rust-lang.org/t/pre-rfc-splitting-lifetime-into-two-tokens/6716 - either splitting lifetimes into two tokens, then they can be matched with ' $i: ident or just making them identifiers so the can be matched with $i: ident / $'i: ident.

I don't think it prevents stabilization of the lifetime matcher, if those changes are implemented it will still continue to work and we'll be able to deprecate it if necessary.

@cramertj

This comment has been minimized.

Member

cramertj commented Mar 10, 2018

@rfcbot fcp merge

trying again

@nikomatsakis nikomatsakis added the T-lang label Mar 12, 2018

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Mar 12, 2018

@rfcbot

This comment has been minimized.

rfcbot commented Mar 12, 2018

Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged teams:

Concerns:

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Mar 15, 2018

@rfcbot fcp concern

@petrochenkov recently brought it to my attention that '_ matches the lifetime fragment (whereas _ -- rightly, to my mind -- does not match ident). I believe this is incorrect, or at least worth debating. =) '_ is not a lifetime name, it is rather a token used to control elision rules.

On the other hand, I can see an argument that not supporting '_ means we can't "forward" lifetimes in some cases where we would like to. (I suppose a similar argument would apply to _ and ident.)

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Mar 15, 2018

@rfcbot concern underscore-lifetime

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Mar 15, 2018

(Bother, I can never remember the dang syntax.)

@durka

This comment has been minimized.

Contributor

durka commented Mar 15, 2018

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Mar 16, 2018

@durka right, that's precisely what I was thinking when I wrote the "on the other hand" part. I think I'm going to resolve my concern -- otherwise I guess we need a bunch more matchers.

@rfcbot resolve underscore-lifetime

@durka

This comment has been minimized.

Contributor

durka commented Mar 19, 2018

@nikomatsakis please say "resolved" for the bot

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Mar 20, 2018

@rfcbot resolved underscore-lifetime

Sigh.

@rfcbot

This comment has been minimized.

rfcbot commented Mar 20, 2018

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot

This comment has been minimized.

rfcbot commented Mar 30, 2018

The final comment period is now complete.

@durka

This comment has been minimized.

Contributor

durka commented Mar 30, 2018

🚀 🚀 🚀 🚀 🚀

@dtolnay

This comment has been minimized.

Member

dtolnay commented Jun 10, 2018

We don't have tests for a macro_rules falling through to the next rule after a $:lifetime matcher.

And it doesn't work. 😢 I filed #51477 to follow up.

@Centril

This comment has been minimized.

Contributor

Centril commented Sep 15, 2018

I think we can close this as it was stabilized; bugs and other things can be tracked as we usually do.

@Centril Centril closed this Sep 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment