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

Remove unnecessary forward_inner_docs hack #83230

Merged
merged 1 commit into from
Mar 19, 2021
Merged

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Mar 17, 2021

and replace it with extended_key_value_attributes feature.

This is #79150, but for compiler/.

and replace it with `extended_key_value_attributes` feature.
@jyn514 jyn514 added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 17, 2021
@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 Mar 17, 2021
@jyn514 jyn514 mentioned this pull request Mar 17, 2021
6 tasks
@joshtriplett
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Mar 18, 2021

📌 Commit bb7c04a has been approved by joshtriplett

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 18, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 19, 2021
Remove unnecessary `forward_inner_docs` hack

and replace it with `extended_key_value_attributes` feature.

This is rust-lang#79150, but for compiler/.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 19, 2021
Remove unnecessary `forward_inner_docs` hack

and replace it with `extended_key_value_attributes` feature.

This is rust-lang#79150, but for compiler/.
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 19, 2021
Rollup of 11 pull requests

Successful merges:

 - rust-lang#82500 (Reuse `std::sys::unsupported::pipe` on `hermit`)
 - rust-lang#82759 (Remove unwrap_none/expect_none from compiler/.)
 - rust-lang#82846 (rustdoc: allow list syntax for #[doc(alias)] attributes)
 - rust-lang#82892 (Clarify docs for Read::read's return value)
 - rust-lang#83179 (Extend `proc_macro_back_compat` lint to `actix-web`)
 - rust-lang#83197 (Move some test-only code to test files)
 - rust-lang#83208 (Fix gitattibutes for old git versions)
 - rust-lang#83215 (Deprecate std::os::haiku::raw, which accidentally wasn't deprecated)
 - rust-lang#83230 (Remove unnecessary `forward_inner_docs` hack)
 - rust-lang#83236 (Upgrade memmap to memmap2)
 - rust-lang#83270 (Fix typo/inaccuracy in the documentation of Iterator::skip_while)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 827ad66 into rust-lang:master Mar 19, 2021
@rustbot rustbot added this to the 1.52.0 milestone Mar 19, 2021
@jyn514 jyn514 deleted the remove-macros branch March 19, 2021 18:24
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 29, 2021
Remove (lots of) dead code

Builds on
- [ ] rust-lang#83161
- [x] rust-lang#83230
- [x] rust-lang#83197.

Found with https://github.com/est31/warnalyzer.
See rust-lang#77739 for a similar change in the past.

Dubious changes:
- Maybe some of the dead code in rustc_data_structures should be kept, in case someone wants to use it in the future?

TODO:
- [ ] check if any of the comments on the deleted code should be kept.
- [x] update the compiler documentation; right now it fails to build
- [x] finish moving `cfg(test)` changes into rust-lang#83197

cc `@est31`
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.
spikespaz pushed a commit to spikespaz/dotwalk-rs that referenced this pull request Aug 29, 2024
Remove (lots of) dead code

Builds on
- [ ] rust-lang/rust#83161
- [x] rust-lang/rust#83230
- [x] rust-lang/rust#83197.

Found with https://github.com/est31/warnalyzer.
See rust-lang/rust#77739 for a similar change in the past.

Dubious changes:
- Maybe some of the dead code in rustc_data_structures should be kept, in case someone wants to use it in the future?

TODO:
- [ ] check if any of the comments on the deleted code should be kept.
- [x] update the compiler documentation; right now it fails to build
- [x] finish moving `cfg(test)` changes into rust-lang/rust#83197

cc `@est31`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants