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

Add a literal fragment specifier to macro_rules!. #1576

Merged
merged 1 commit into from
Aug 12, 2016

Conversation

LeoTestard
Copy link
Contributor

@LeoTestard LeoTestard commented Apr 8, 2016

Add a literal fragment specifier for macro_rules! patterns that matches literal constants.

Rendered

@LeoTestard LeoTestard force-pushed the master branch 4 times, most recently from 5fa69f4 to 5c67bc0 Compare April 8, 2016 12:20

# Drawbacks

This is a bit inconsistent since it does not include all literal constants (in this context, I mean ‶literals that do not require any computation″): indeed it does not include compound literals, for example struct literals `Foo { x: some_literal, y: some_literal }` or arrays `[some_literal ; N]`, where `some_literal` can itself be a compound literal. See in alternatives why this is disallowed.
Copy link
Member

Choose a reason for hiding this comment

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

The Design section should be expanded to specify what is the set of literal constants that you are adding here.

(I.e. the fact that you are adding the clarifying aside "in this context, I mean ..." in the drawbacks section is a bad sign -- the design section should have enough information for someone to deduce what you mean by "literal".)

@LeoTestard LeoTestard force-pushed the master branch 2 times, most recently from 8d9a531 to 7e55f54 Compare April 8, 2016 13:00
@bluss
Copy link
Member

bluss commented Apr 11, 2016

I've been missing this, for example in a macro that uses consecutive integer literals like shuffle![v, 1 2 3 4]. You can work around it using tt.

@DanielKeep
Copy link

Aye; there's an important alternative not mentioned in the RFC: use tt instead.

The problem I can see with this proposal is that it doesn't really simplify existing macros, nor open up new ones. The lack of backtracking means that even if this matcher is more specific than tt, it won't permit macros that wouldn't have been expressible before.

This will probably give better error messages on matching failures, but nothing else I can think of. No, wait, it does have one advantage over tt: you won't need the reparse trick, since you should be able to assume a literal is an expression. ... still not sure that's enough to pull its weight, though.

@nrc nrc added the T-lang Relevant to the language team, which will review and decide on the RFC. label Apr 17, 2016
@LeoTestard
Copy link
Contributor Author

@DanielKeep The weight should be that big. And backtracking is coming with rust-lang/rust#33840. The better error messages are a big gain. :/

@LeoTestard
Copy link
Contributor Author

(Edited to mention the possibility of using tt.)

@nrc
Copy link
Member

nrc commented Aug 4, 2016

cc @jseyfried in case you have an opinion on this

@nrc
Copy link
Member

nrc commented Aug 4, 2016

also cc @cswords

@nrc
Copy link
Member

nrc commented Aug 4, 2016

I'm in favour of this, though slightly (and vaguely) worried about the implementation details

@jseyfried
Copy link

jseyfried commented Aug 4, 2016

I'm also in favour of this.

I don't think implementation will be a problem, but I'm not too familiar with the details of how fragments are implemented.

n.b. the reparse trick is no longer needed, so that's no longer an advantage over tt.

@nikomatsakis
Copy link
Contributor

Hear ye, hear ye! This RFC is now entering final comment period. We discussed this at the most recent @rust-lang/lang meeting and decided that we are currently inclined to accept (of course no final decision has been reached).

To summarize the discussion so far:

  • one can currently use tt for this purpose, but the error messages are less good

@nikomatsakis nikomatsakis added the final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. label Aug 8, 2016
@joshtriplett
Copy link
Member

I like this RFC.

Would it make sense to also have more specific versions for specific kinds of literals? Several times, I've wanted to have a macro accept a string literal specifically.

@LeoTestard
Copy link
Contributor Author

I don't think implementation will be a problem, but I'm not too familiar with the details of how fragments are implemented.

It's actually very simple, it just calls the parse_whatever() functions from libsyntax::parse::parser.

  • one can currently use tt for this purpose, but the error messages are less good

And the future-proofing is less good too. Using a more specific matcher restricts the FIRST sets, which is good.

Would it make sense to also have more specific versions for specific kinds of literals? Several times, I've wanted to have a macro accept a string literal specifically.

I guess it would make sense, though the gain in terms of error messages is small. I'm not sure if it's possible though, since I think the libsyntax parser has a single parsing function for all literal types. It can surely be refactored and split into several functions, but I'm not sure it's worth it.

@nikomatsakis
Copy link
Contributor

Would it make sense to also have more specific versions for specific kinds of literals? Several times, I've wanted to have a macro accept a string literal specifically.

Yeah, I wondered the same. Could be useful, but I guess there is value in "any literal" as well, and it's something we could easily add later.

@nrc nrc removed the I-nominated label Aug 11, 2016
@joshtriplett
Copy link
Member

@nrc Absolutely! To clarify, I'd love to see the proposed RFC accepted, and I don't want to bikeshed it. I'd also love to see specifiers for specific types of literals, which could either go in this RFC or a future RFC.

@nikomatsakis
Copy link
Contributor

Huzzah! The @rust-lang/lang team has decided to accept this RFC.

@nikomatsakis
Copy link
Contributor

Tracking issue: rust-lang/rust#35625

If you'd like to keep following the development of this feature, please subscribe to that issue, thanks! :)

@nikomatsakis nikomatsakis merged commit 09aa1d6 into rust-lang:master Aug 12, 2016
@da-x
Copy link
Member

da-x commented May 13, 2018

Dear all, the implementation is now in master :)

@Centril Centril added A-macros Macro related proposals and issues A-syntax Syntax related proposals & ideas labels Nov 23, 2018
@pravic

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Macro related proposals and issues A-syntax Syntax related proposals & ideas final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.