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

Accept arbitrary expressions in key-value attributes at parse time #78837

Merged
merged 1 commit into from
Dec 10, 2020

Conversation

petrochenkov
Copy link
Contributor

@petrochenkov petrochenkov commented Nov 7, 2020

Continuation of #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:

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 #52607.
Previous attempt - #67121.
Some more detailed write up on internals - https://internals.rust-lang.org/t/macro-expansion-points-in-attributes/11455.
Tracking issue - #78835.

@rust-highfive
Copy link
Collaborator

r? @davidtwco

(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 Nov 7, 2020
@petrochenkov petrochenkov added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Nov 7, 2020
@SoniEx2
Copy link
Contributor

SoniEx2 commented Nov 7, 2020

does this solve the translatable docs problem?

@petrochenkov
Copy link
Contributor Author

@SoniEx2
No? But it may provide tools for a higher-level system to solve it.
Similarly to #44732, this is a low-level feature, in theory in can be used to document crate with different cfgs and include different documentation depending on that cfg.

@SoniEx2
Copy link
Contributor

SoniEx2 commented Nov 7, 2020

#[doc = include_str!("my_doc.md")] looks nice but you'd have to be able to supply the path based on a build setting (uhh like #[doc = include_str!(concat!(env!("doc_lang"), "my_doc.md"))] maybe?)

@petrochenkov
Copy link
Contributor Author

#[doc = include_str!(concat!(env!("doc_lang"), "my_doc.md"))]

This should already work, I will add a test.

Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

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

I'm not exceptionally confident in my understanding of this, but it looks broadly correct to me. r=me if you don't have other changes to make or are fine with just my review.

@petrochenkov petrochenkov added I-nominated S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 8, 2020
@bors

This comment has been minimized.

@m-ou-se
Copy link
Member

m-ou-se commented Nov 17, 2020

This change will allow for finally cleaning up the doc_comment hacks in core. 😃 See #79150.

@jyn514
Copy link
Member

jyn514 commented Nov 18, 2020

External doc strings (#[doc = include_str!("my_doc.md")], eliminating the need in #44732) and expanding macros in this position in general. Currently such macro expansions are supported in this position in interpolated $exprs (the #[doc = $doc] idiom).

Before recommending this, someone should test how this interacts with unindent_comments (#78400).

@jyn514
Copy link
Member

jyn514 commented Nov 18, 2020

(but to be clear: I'm strongly in favor of this, I'm always +1 for having features in rustc instead of special-casing rustdoc.)

@jyn514 jyn514 added the A-attributes Area: #[attributes(..)] label Nov 18, 2020
@m-ou-se
Copy link
Member

m-ou-se commented Nov 18, 2020

Before recommending this, someone should test how this interacts with unindent_comments

@jyn514 Looks like that works just fine:

#![feature(extended_key_value_attributes)]

const _: &[u8] = compile_time_run::run_command!("sh", "-c", "echo '    .second(\"hello\")' > somefile");

/// You are supposed to call it like this
///
/// # Example
///
/// ```
/// let example = Example::new()
///     .first("hello")
#[cfg_attr(not(feature = "one"), doc = include_str!("../somefile"))]
///     .third("hello")
///     .build();
/// ```
pub struct Example {}

This results in:

image

@petrochenkov
Copy link
Contributor Author

To lang team: this PR has lower priority than #79078.

@Mark-Simulacrum
Copy link
Member

We discussed this during the lang team meeting and felt that if this is just adding unstable surface area we can proceed here without a T-lang FCP. If there is something we're stabilizing then we'd like that called out more explicitly though and we can take another look :)

We also had a question around the hygiene behavior of the passed tokens -- do we handle those equivalently to other macro input? If so then no concerns.

@LeSeulArtichaut LeSeulArtichaut added the F-extended_key_value_attributes `#![feature(extended_key_value_attributes)] label Dec 30, 2020
@m-ou-se
Copy link
Member

m-ou-se commented Jan 1, 2021

The change this PR makes in compiler/rustc_parse/src/parser/mod.rs somehow makes rustfmt apply this formatting:

-macro foo($type_name: ident, $docs: expr) {
+macro foo($type_name:ident, $docs:expr) {

Without the parser/mod.rs changes of this PR, rustfmt leaves this line untouched and does not remove the spaces.

@m-ou-se
Copy link
Member

m-ou-se commented Jan 1, 2021

Ah, apparently rustfmt doesn't format the macro parameters if it cannot format/parse the body. The full test case is:

macro foo($type_name: ident, $docs: expr) {
    #[allow(non_camel_case_types)]
    #[doc=$docs]
    #[derive(Debug, Clone, Copy)]
    pub struct $type_name;
}

The body contained a #[doc=$docs] which it couldn't understand before, but can be handled fine now with this change, causing it to no longer skip formatting of the entire macro definition.

Mystery solved.

@pthariensflame
Copy link
Contributor

I passed over this issue the first time through the list, but…does this need relnotes? On a closer look it seems like people would want to know about new capabilities for proc macros.

@petrochenkov
Copy link
Contributor Author

@pthariensflame
This is still only available on nightly, relnotes for new features are usually applied to stabilization PRs.

@pthariensflame
Copy link
Contributor

Understood!

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 9, 2021
ast: Remove some indirection layers from values in key-value attributes

Trying to address some perf regressions from rust-lang#78837 (comment).
jyn514 added a commit to jyn514/rust that referenced this pull request May 18, 2021
 # Stabilization report

 ## Summary

This stabilizes using macro expansion in key-value attributes, like so:

 ```rust
 #[doc = include_str!("my_doc.md")]
 struct S;

 #[path = concat!(env!("OUT_DIR"), "/generated.rs")]
 mod m;
 ```

See the changes to the reference for details on what macros are allowed;
see Petrochenkov's excellent blog post [on internals](https://internals.rust-lang.org/t/macro-expansion-points-in-attributes/11455)
for alternatives that were considered and rejected ("why accept no more
and no less?")

This has been available on nightly since 1.50 with no major issues.

 ## Notes

 ### Accepted syntax

The parser accepts arbitrary Rust expressions in this position, but any expression other than a macro invocation will ultimately lead to an error because it is not expected by the built-in expression forms (e.g., `#[doc]`).  Note that decorators and the like may be able to observe other expression forms.

 ### Expansion ordering

Expansion of macro expressions in "inert" attributes occurs after decorators have executed, analogously to macro expressions appearing in the function body or other parts of decorator input.

There is currently no way for decorators to accept macros in key-value position if macro expansion must be performed before the decorator executes (if the macro can simply be copied into the output for later expansion, that can work).

 ## Test cases

 - https://github.com/rust-lang/rust/blob/master/src/test/ui/attributes/key-value-expansion-on-mac.rs
 - https://github.com/rust-lang/rust/blob/master/src/test/rustdoc/external-doc.rs

The feature has also been dogfooded extensively in the compiler and
standard library:

- rust-lang#83329
- rust-lang#83230
- rust-lang#82641
- rust-lang#80534

 ## Implementation history

- Initial proposal: rust-lang#55414 (comment)
- Experiment to see how much code it would break: rust-lang#67121
- Preliminary work to restrict expansion that would conflict with this
feature: rust-lang#77271
- Initial implementation: rust-lang#78837
- Fix for an ICE: rust-lang#80563

 ## Unresolved Questions

~~rust-lang#83366 (comment) listed some concerns, but they have been resolved as of this final report.~~

 ## Additional Information

 There are two workarounds that have a similar effect for `#[doc]`
attributes on nightly. One is to emulate this behavior by using a limited version of this feature that was stabilized for historical reasons:

```rust
macro_rules! forward_inner_docs {
    ($e:expr => $i:item) => {
        #[doc = $e]
        $i
    };
}

forward_inner_docs!(include_str!("lib.rs") => struct S {});
```

This also works for other attributes (like `#[path = concat!(...)]`).
The other is to use `doc(include)`:

```rust
 #![feature(external_doc)]
 #[doc(include = "lib.rs")]
 struct S {}
```

The first works, but is non-trivial for people to discover, and
difficult to read and maintain. The second is a strange special-case for
a particular use of the macro. This generalizes it to work for any use
case, not just including files.

I plan to remove `doc(include)` when this is stabilized. The
`forward_inner_docs` workaround will still compile without warnings, but
I expect it to be used less once it's no longer necessary.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 18, 2021
…=petrochenkov

Stabilize extended_key_value_attributes

Closes rust-lang#44732. Closes rust-lang#78835. Closes rust-lang#82768 (by making it irrelevant).

 # Stabilization report

 ## Summary

This stabilizes using macro expansion in key-value attributes, like so:

 ```rust
 #[doc = include_str!("my_doc.md")]
 struct S;

 #[path = concat!(env!("OUT_DIR"), "/generated.rs")]
 mod m;
 ```

See Petrochenkov's excellent blog post [on internals](https://internals.rust-lang.org/t/macro-expansion-points-in-attributes/11455)
for alternatives that were considered and rejected ("why accept no more and no less?")

This has been available on nightly since 1.50 with no major issues.

## Notes

### Accepted syntax

The parser accepts arbitrary Rust expressions in this position, but any expression other than a macro invocation will ultimately lead to an error because it is not expected by the built-in expression forms (e.g., `#[doc]`).  Note that decorators and the like may be able to observe other expression forms.

### Expansion ordering

Expansion of macro expressions in "inert" attributes occurs after decorators have executed, analogously to macro expressions appearing in the function body or other parts of decorator input.

There is currently no way for decorators to accept macros in key-value position if macro expansion must be performed before the decorator executes (if the macro can simply be copied into the output for later expansion, that can work).

## Test cases

 - https://github.com/rust-lang/rust/blob/master/src/test/ui/attributes/key-value-expansion-on-mac.rs
 - https://github.com/rust-lang/rust/blob/master/src/test/rustdoc/external-doc.rs

The feature has also been dogfooded extensively in the compiler and
standard library:

- rust-lang#83329
- rust-lang#83230
- rust-lang#82641
- rust-lang#80534

## Implementation history

- Initial proposal: rust-lang#55414 (comment)
- Experiment to see how much code it would break: rust-lang#67121
- Preliminary work to restrict expansion that would conflict with this
feature: rust-lang#77271
- Initial implementation: rust-lang#78837
- Fix for an ICE: rust-lang#80563

## Unresolved Questions

~~rust-lang#83366 (comment) listed some concerns, but they have been resolved as of this final report.~~

 ## Additional Information

 There are two workarounds that have a similar effect for `#[doc]`
attributes on nightly. One is to emulate this behavior by using a limited version of this feature that was stabilized for historical reasons:

```rust
macro_rules! forward_inner_docs {
    ($e:expr => $i:item) => {
        #[doc = $e]
        $i
    };
}

forward_inner_docs!(include_str!("lib.rs") => struct S {});
```

This also works for other attributes (like `#[path = concat!(...)]`).
The other is to use `doc(include)`:

```rust
 #![feature(external_doc)]
 #[doc(include = "lib.rs")]
 struct S {}
```

The first works, but is non-trivial for people to discover, and
difficult to read and maintain. The second is a strange special-case for
a particular use of the macro. This generalizes it to work for any use
case, not just including files.

I plan to remove `doc(include)` when this is stabilized
(rust-lang#82539). The `forward_inner_docs`
workaround will still compile without warnings, but I expect it to be
used less once it's no longer necessary.
jackh726 added a commit to jackh726/rust that referenced this pull request May 19, 2021
…=petrochenkov

Stabilize extended_key_value_attributes

Closes rust-lang#44732. Closes rust-lang#78835. Closes rust-lang#82768 (by making it irrelevant).

 # Stabilization report

 ## Summary

This stabilizes using macro expansion in key-value attributes, like so:

 ```rust
 #[doc = include_str!("my_doc.md")]
 struct S;

 #[path = concat!(env!("OUT_DIR"), "/generated.rs")]
 mod m;
 ```

See Petrochenkov's excellent blog post [on internals](https://internals.rust-lang.org/t/macro-expansion-points-in-attributes/11455)
for alternatives that were considered and rejected ("why accept no more and no less?")

This has been available on nightly since 1.50 with no major issues.

## Notes

### Accepted syntax

The parser accepts arbitrary Rust expressions in this position, but any expression other than a macro invocation will ultimately lead to an error because it is not expected by the built-in expression forms (e.g., `#[doc]`).  Note that decorators and the like may be able to observe other expression forms.

### Expansion ordering

Expansion of macro expressions in "inert" attributes occurs after decorators have executed, analogously to macro expressions appearing in the function body or other parts of decorator input.

There is currently no way for decorators to accept macros in key-value position if macro expansion must be performed before the decorator executes (if the macro can simply be copied into the output for later expansion, that can work).

## Test cases

 - https://github.com/rust-lang/rust/blob/master/src/test/ui/attributes/key-value-expansion-on-mac.rs
 - https://github.com/rust-lang/rust/blob/master/src/test/rustdoc/external-doc.rs

The feature has also been dogfooded extensively in the compiler and
standard library:

- rust-lang#83329
- rust-lang#83230
- rust-lang#82641
- rust-lang#80534

## Implementation history

- Initial proposal: rust-lang#55414 (comment)
- Experiment to see how much code it would break: rust-lang#67121
- Preliminary work to restrict expansion that would conflict with this
feature: rust-lang#77271
- Initial implementation: rust-lang#78837
- Fix for an ICE: rust-lang#80563

## Unresolved Questions

~~rust-lang#83366 (comment) listed some concerns, but they have been resolved as of this final report.~~

 ## Additional Information

 There are two workarounds that have a similar effect for `#[doc]`
attributes on nightly. One is to emulate this behavior by using a limited version of this feature that was stabilized for historical reasons:

```rust
macro_rules! forward_inner_docs {
    ($e:expr => $i:item) => {
        #[doc = $e]
        $i
    };
}

forward_inner_docs!(include_str!("lib.rs") => struct S {});
```

This also works for other attributes (like `#[path = concat!(...)]`).
The other is to use `doc(include)`:

```rust
 #![feature(external_doc)]
 #[doc(include = "lib.rs")]
 struct S {}
```

The first works, but is non-trivial for people to discover, and
difficult to read and maintain. The second is a strange special-case for
a particular use of the macro. This generalizes it to work for any use
case, not just including files.

I plan to remove `doc(include)` when this is stabilized
(rust-lang#82539). The `forward_inner_docs`
workaround will still compile without warnings, but I expect it to be
used less once it's no longer necessary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: #[attributes(..)] F-extended_key_value_attributes `#![feature(extended_key_value_attributes)] merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

Creating a doc attribute from stringify! or concat! in a macro works in some situations but not others