Add a `literal` fragment specifier to `macro_rules!`. #1576

Open
wants to merge 1 commit into from

9 participants

@LeoTestard

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

@pnkfelix pnkfelix commented on an outdated diff Apr 8, 2016
text/0000-macros-literal-matcher.md
+```
+
+# Motivation
+
+There are a lot of macros out there that take literal constants as arguments (often string constants). For now, most use the `expr` fragment specifier, which is fine since literal constants are a subset of expressions. But it has the following issues:
+* It restricts the syntax of those macros. A limited set of FOLLOW tokens is allowed after an `expr` specifier. For example `$e:expr : $t:ty` is not allowed whereas `$l:literal : $t:ty` should be. There is no reason to arbitrarily restrict the syntax of those macros where they will only be actually used with literal constants.
+* It does not allow for proper error reporting where the macro actually *needs* the parameter to be a literal constant. With this RFC, bad usage of such macros will give a proper syntax error message whereas with `epxr` it would probably give a syntax or typing error inside the generated code, which is hard to understand.
+* It's not consistent. There is no reason to allow expressions, types, etc. but not literals.
+
+# Design
+
+Add a `literal` (or `lit`, or `constant`) matcher in macro patterns that calls the `parse_lit` method from `libsyntax::parse::Parser`. The FOLLOW set of this matcher should be the same as `ident` since it matches a single token.
+
+# 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.
@pnkfelix
The Rust Programming Language member
pnkfelix added a note Apr 8, 2016

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".)

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

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

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 label Apr 17, 2016
@LeoTestard

@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

(Edited to mention the possibility of using tt.)

@aturon aturon added the I-nominated label Jul 28, 2016
@nrc
nrc commented Aug 4, 2016

cc @jseyfried in case you have an opinion on this

@nrc
nrc commented Aug 4, 2016

also cc @cswords

@nrc
nrc commented Aug 4, 2016

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

@jseyfried
jseyfried commented Aug 4, 2016 edited

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

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
@joshtriplett

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

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.

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