Rust: Allow all literals in attributes (Tracking Issue for RFC #1559)

[nikomatsakis]

Tracking issue for rust-lang/rfcs#1559 -- allow all literals in attributes.

[eddyb]

@cgswords Should this be assigned to you?

[retep998]

Just checking, will this apply to align and packed (when they are finally implemented) so that I can do #[repr(align(4))] or #[repr(packed(4))]?

[eddyb]

@retep998 That is the intention AFAIK, yes.

[SergioBenitez]

I've started implementing this and have made headway. Should have a working implementation soon.

[Keats]

Any news about stabilisation of that feature?

[Nemo157]

I would also like to know if there are any thoughts on stabilisation, this would have been great to use with Macros 1.1, but I guess it's too late for that.

(also, for searchability this is the attr_literals feature, would be nice if the feature name was mentioned in the tracking issue once stuff is implemented)

[Nemo157]

I think I've found a bug (or at least a big limitation) in the current implementation, negative integer and float literals are not supported:

→ rustc - <<END
#[lit = -1]
struct Foo;
END
error: unexpected token: `-`
 --> <anon>:1:7
  |
1 | #[lit = -1]
  |       ^

error: aborting due to previous error

[eddyb]

There are no negative literals (only negation expressions).

[Nemo157]

😞 damn, I sort of expected something like that. It is a limitation for procedural (derive) macros though, any negative numbers will still need to be passed through as a string and parsed manually. I guess the next step would be to allow constant expressions in attributes, but that sounds like a much bigger step than this one.

[eddyb]

The plan ha been full token trees, for a while, really, so you'd just parse it as an expression.
Not implemented, though, AFAIK.

[Kixunil]

Will this allow type names in attributes?

[withoutboats]

I've been looking at implementing this properly. My belief is that we intend the grammar for attributes to be one of:

[ PATH ]
[ PATH ( EXPR , ... ) ]
[ PATH = EXPR ]

My (ill-informed) read of libsyntax is that we parse all attributes into the Attribute struct. After parsing their contents correctly, everything after the initial path gets unparsed into a TokenStream.

While validating that the attributes are grammatical (as part of constructing an Attribute) we use these types:

enum MetaItemKind {
    Word,
    List(Vec<NestedMetaItem>),
    NameValue(Lit),
}

enum NestedMetaItemKind {
    MetaItem(MetaItem),
    Literal(Lit)
}

It seems to me the correct thing to do is eliminated NestedMetaItem(Kind) and rewrite MetaItemKind as:

pub enum MetaItemKind {
     Word,
     List(Vec<Expr>),
     NameValue(Expr),
}

Then change the code in libsyntax/attr.rs to correctly implement the conversion to/from tokens for MetaItemKind.

Corrections appreciated.

[eddyb]

I believe the intent was to have token streams inside at least #[foo(...)], if not #[foo = ...].

cc @jseyfried @nrc

[withoutboats]

Yes, expanding it to an arbitrary token stream seems reasonable.

[bstrie]

I believe the intent was to have token streams inside at least #[foo(...)], if not #[foo = ...].

Yet another reason to deprecate the latter form. ;)

[cramertj]

Ping @withoutboats-- what's the status of this? Are you still working on something?

[withoutboats]

No I found a satisfactory near term solution for failure and it popped off my yak stack.

[nox]

What is blocking this? Can we get this stabilised? There are some parts of Stylo where I need to do stuff like #[css(aliases = "foo,bar")] instead of the better #[css(aliases("foo", "bar"))].

[nikomatsakis]

@nox

What is blocking this? Can we get this stabilised?

We'd have to implement it first, I reckon.

[nox]

@nikomatsakis I'm not sure this is not implemented already, given #[css(aliases("foo", "bar"))] already parse right now.

[nikomatsakis]

Ah, perhaps so. I don't see anyone commenting that they have implemented it though.

@nox, perhaps write up a mini stabilization report, describing the behavior to be stabilized, and pointing to tests in the repo that demonstrate said behavior?

[SergioBenitez]

I implemented the original RFC about a year and a half back. It's been in use by Rocket and its users for over a year, since Rocket's release, with no issue. These syntax changes are ready to be stabilized, in my opinion, and stabilizing these changes doesn't preclude stabilizing further loosening of the syntax in the future.

[nikomatsakis]

@SergioBenitez ah, great! then can you produce the kind of stabilization report I was talking about here?

describing the behavior to be stabilized, and pointing to tests in the repo that demonstrate said behavior?

It doesn't have to be long: a few sentences and some links to a few tests would suffice, I think.

[petrochenkov]

From what I've seen the implementation of paths and arbitrary token trees in attributes is full of technical debt so bugs are likely.
Stabilizing the part about arbitrary literals in positions already allowed for string literals seems harmless though.

[Keats]

I was using that feature in https://github.com/Keats/validator and it worked fine for literals:

#[validate(length(equal = 10))]
#[validate(range(min = 1.1, max = 10.8))]
#[validate(schema(function = "validate_category", skip_on_field_errors = false)]

All of those are using strings now instead to work on stable. Could literals be stabilized first and expressions later?

[Keats]

Anything I could do to help stabilise the current literals?

[SergioBenitez]

Stabilization Report

Overview

The attr_literals feature allows all unsuffixed literals to be used in attributes in all positions where previously only strings were accepted. This includes, but is not limited to, the following:

• top-level positions:

  #[attr(true)]
  #[attr(100)]
  #[enabled(true)]
  #[attr("hello")]
  

• nested positions:

  #[attr("string", ident(100))]
  

• values of key/value pairs:

  #[attr(enabled = true)]
  #[kind(value, number = 4)]
  

The following tests in rustc are demonstrative of this feature's behavior:

• run-pass-fulldeps/macro-crate-multi-decator-literals.rs
• pretty/attr-literals.rs
• compile-fail/suffixed-literal-meta.rs
• compile-fail/gated-attr-literals.rs

Status

The attr_literals feature was implemented in https://github.com/rust-lang/rust/pull/35850 and merged on August 30, 2016, or 2 years ago. The feature is being used in several crates including Rocket, structopt, and validator. Furthermore, a subset of this feature is already being used in the stabilized repr attribute (#[repr(align(8))]).

To date, no issues or bugs have been opened about the feature despite its fairly widespread use.

Proposal

I propose that we stabilize this feature in its entirety, allowing unsuffixed literals to be used in attributes. This change is backwards compatible. This change is also forwards compatible with the proposed change to make attribute inputs be an arbitrary TokenStream.

To this end, I submit https://github.com/rust-lang/rust/pull/53044 and https://github.com/rust-lang-nursery/reference/pull/388, stabilizing the feature.

[cramertj]

@rfcbot fcp merge

See @SergioBenitez's excellent summary above.

[rfcbot]

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

• [x] @aturon
• [x] @cramertj
• [x] @eddyb
• [x] @joshtriplett
• [x] @nikomatsakis
• [x] @nrc
• [ ] @pnkfelix
• [x] @scottmcm
• [ ] @withoutboats

No concerns currently listed.

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.

[rfcbot]

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

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

[nikomatsakis]

Have there been any recent changes here that might be responsible for https://github.com/rust-lang/rust/issues/53298?

---

lib.rs

macro_rules! helper {
    ($expr:expr) => {
        #[cfg(feature = $expr)]
        fn foo() {}
    }
}

helper!{concat!("thing")}

This should be an error, as concat!("thing") is not allowed in this position. But instead, the cfg attribute in this example is silently ignored, and fn foo gets compiled.

I tested also with #[doc], hence why the title claims that this applies to "attributes with 'name = $expr'" in general.

This is a regression from stable 1.17 to 1.18.

[SergioBenitez]

The feature has been stabilized! Looks like this issue can finally be closed.

[pravic]

Hmm, it's not clear: are all those literals just a syntax sugar and they are converted to strings internally (as it used to be without this RFC) or attribute "consumers" will expect integer literals too?

[eddyb]

@pravic The consumers can see the real literal tokens, and in the foo(...) form, any tokens that might be in there (but this is tracked elsewhere).