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

Allow path as value in name-value attribute #76734

Closed
wants to merge 2 commits into from

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Sep 15, 2020

This commit allows proc macros to take inert attributes that look like #[path = ident] or #[path = p::p::path]. Previously the rhs was only permitted to be an unsuffixed numeric literal or the identifier special cases true and false. However, parenthesized #[path(key = ident)] and #[path(key = p::p::path)] were already allowed; see https://docs.rs/cxx/0.4.4/cxx/attr.bridge.html for one attribute currently using that style.

My use case for this is related to C++ namespace support in https://github.com/dtolnay/cxx.

- #[cxx::bridge(namespace = co::stuff)]  // previously only one namespace can be specified
+ #[cxx::bridge]
  mod ffi {
+     #[namespace = co::server]  // now can allow different namespace for different stuff
      extern "C++" {
          ...
+     }

+     #[namespace = co::client]
+     extern "C++" {
          ...
      }
  }

Chesterton's fence: why was the rhs syntax restricted originally? I believe this was to leave open the possibility of #[m1 = "...", m2 = "..."] syntax which would be equivalent to #[m1 = "..."] #[m2 = "..."] but more compact (this might still happen). Allowing an "arbitrary tokenstream" on the rhs would rule that out. In this PR I've still kept the rhs syntax quite restricted, adding only mod-style paths and remaining compatible with compact syntax being introduced later.

@dtolnay dtolnay added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Sep 15, 2020
@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(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 Sep 15, 2020
@jyn514 jyn514 added A-proc-macros Area: Procedural macros A-attributes Area: Attributes (`#[…]`, `#![…]`) labels Sep 15, 2020
@petrochenkov
Copy link
Contributor

petrochenkov commented Sep 15, 2020

cc https://internals.rust-lang.org/t/macro-expansion-points-in-attributes/11455 with which this extension overlaps.
(I was planning to support arbitrary expressions in this position, basically (and make them expandable), but didn't get to implementing #55414 (comment) myself and couldn't find other volunteers either.)

@dtolnay
Copy link
Member Author

dtolnay commented Sep 15, 2020

Expressions seems fine and is still compatible with potentially allowing attributes to be written combined like #[m1 = expr1, m2 = expr2].

Do you think we shouldn't take this PR without the bigger change in #55414 (comment)? I think they're largely orthogonal because values which are just mod-style paths would not participate in any kind of expansion described there.

@pickfire

This comment has been minimized.

@petrochenkov
Copy link
Contributor

Do you think we shouldn't take this PR without the bigger change in #55414 (comment)?

Yes.
I'll try to prioritize #55414 (comment) and implement it myself sooner.

In the meantime #[path(key = p::p::path)] can be written through a string literal as #[path(key = "p::p::path")], so supporting paths here is not very urgent.

@petrochenkov
Copy link
Contributor

I'll try to prioritize #55414 (comment) and implement it myself sooner.

I started working on this in #77271.

@dtolnay dtolnay reopened this Nov 3, 2020
@dtolnay
Copy link
Member Author

dtolnay commented Nov 3, 2020

Reopening with rebase on #77271.

@petrochenkov
Copy link
Contributor

This is superseded by #78837 now, but it would be nice to land the tests here, so marking this as blocked on #78837.

@petrochenkov petrochenkov added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 7, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 10, 2020
Accept arbitrary expressions in key-value attributes at parse time

Continuation of rust-lang#77271.

We now support arbitrary expressions in values of key-value attributes at parse time.
```
#[my_attr = EXPR]
```
Previously only unsuffixed literals and interpolated expressions (`$expr`) were accepted.

There are two immediate motivational cases for this:
- External doc strings (`#[doc = include_str!("my_doc.md")]`, eliminating the need in rust-lang#44732) and expanding macros in this position in general. Currently such macro expansions are supported in this position in interpolated `$expr`s (the `#[doc = $doc]` idiom).
- Paths (`#[namespace = foo::bar] extern "C++" { ... }`) like proposed in rust-lang#76734.

If the attribute in question survives expansion, then the value is still restricted to unsuffixed literals by a semantic check.
This restriction doesn't prevent the use cases listed above, so this PR keeps it in place for now.

Closes rust-lang#52607.
Previous attempt - rust-lang#67121.
Some more detailed write up on internals - https://internals.rust-lang.org/t/macro-expansion-points-in-attributes/11455.
Tracking issue - rust-lang#78835.
@bors
Copy link
Contributor

bors commented Dec 10, 2020

☔ The latest upstream changes (presumably #78837) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@petrochenkov
Copy link
Contributor

#78837 has landed, unblocking.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Dec 10, 2020
@crlf0710 crlf0710 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 15, 2021
@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 8, 2021
@petrochenkov
Copy link
Contributor

Closing due to inactivity.

@dtolnay dtolnay deleted the path-eq-path branch March 30, 2021 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-proc-macros Area: Procedural macros S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

8 participants