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

Tracking issue for #![register_attr] #66080

Closed
petrochenkov opened this issue Nov 4, 2019 · 6 comments · Fixed by #101123
Closed

Tracking issue for #![register_attr] #66080

petrochenkov opened this issue Nov 4, 2019 · 6 comments · Fixed by #101123
Assignees
Labels
A-attributes Area: #[attributes(..)] B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@petrochenkov
Copy link
Contributor

petrochenkov commented Nov 4, 2019

This is a direct replacement for now removed #![feature(custom_attribute)] (#29642), except it doesn't rely on implicit fallback from unresolved attributes to custom attributes (which was always hacky and is the primary reason for the removal of custom_attribute) and requires registering the attribute explicitly.

#![register_attr(my_attr)]

#[my_attr] // OK
fn main() {}

It's not yet clear whether this should go through stabilization or not.
It's quite possible that all the uses should migrate to #![register_tool] (#66079) instead.

@jonas-schievink jonas-schievink added A-attributes Area: #[attributes(..)] B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Nov 4, 2019
bors added a commit that referenced this issue Nov 10, 2019
Support registering inert attributes and attribute tools using crate-level attributes

And remove `#[feature(custom_attribute)]`.
(`rustc_plugin::Registry::register_attribute` is not removed yet, I'll do it in a follow up PR.)

```rust
#![register_attr(my_attr)]
#![register_tool(my_tool)]

#[my_attr] // OK
#[my_tool::anything] // OK
fn main() {}
```

---
Some tools (`rustfmt` and `clippy`) used in tool attributes are hardcoded in the compiler.
We need some way to introduce them without hardcoding as well.

This PR introduces a way to do it with a crate level attribute.
The previous attempt to introduce them through command line (#57921) met some resistance.

This probably needs to go through an RFC before stabilization.
However, I'd prefer to land *this* PR without an RFC to able to remove `#[feature(custom_attribute)]` and `Registry::register_attribute` while also providing a replacement.

---
`register_attr` is a direct replacement for `#![feature(custom_attribute)]` (#29642), except it doesn't rely on implicit fallback from unresolved attributes to custom attributes (which was always hacky and is the primary reason for the removal of `custom_attribute`) and requires registering the attribute explicitly.
It's not clear whether it should go through stabilization or not.
It's quite possible that all the uses should migrate to `#![register_tool]` (#66079) instead.

---

Details:
- The naming is `register_attr`/`register_tool` rather than some `register_attributes` (plural, no abbreviation) for consistency with already existing attributes like `cfg_attr`, or `feature`, etc.
---
Previous attempt: #57921
cc #44690
Tracking issues: #66079 (`register_tool`), #66080 (`register_attr`)
Closes #29642
Centril added a commit to Centril/rust that referenced this issue Nov 17, 2019
…sper

rustc_plugin: Remove `Registry::register_attribute`

Legacy plugins cannot register inert attributes anymore.

The preferred replacement is to use `register_tool` ([tracking issue](rust-lang#66079)).
```rust
#![register_tool(servo)]

#[servo::must_root]
struct S;
```

The more direct replacement is `register_attribute` ([tracking issue](rust-lang#66080))
```rust
#![register_attr(must_root)]

#[must_root]
struct S;
```
, but it requires registering each attribute individually rather than registering the tool once, and is more likely to be removed rather than stabilized.
JohnTitor added a commit to JohnTitor/rust that referenced this issue Nov 17, 2019
…sper

rustc_plugin: Remove `Registry::register_attribute`

Legacy plugins cannot register inert attributes anymore.

The preferred replacement is to use `register_tool` ([tracking issue](rust-lang#66079)).
```rust
#![register_tool(servo)]

#[servo::must_root]
struct S;
```

The more direct replacement is `register_attribute` ([tracking issue](rust-lang#66080))
```rust
#![register_attr(must_root)]

#[must_root]
struct S;
```
, but it requires registering each attribute individually rather than registering the tool once, and is more likely to be removed rather than stabilized.
@joshtriplett
Copy link
Member

It's quite possible that all the uses should migrate to #![register_tool] (#66079) instead.

We discussed this in today's @rust-lang/lang meeting, and we agreed: anything new should be scoped, and anything existing we need to support for backwards compatibility shouldn't require calling register_tool first, so we can just keep the hardcoding of the handful of exceptions in the compiler.

@rfcbot close

@rfcbot
Copy link

rfcbot commented Jun 29, 2022

Team member @joshtriplett has proposed to close this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), 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 rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. labels Jun 29, 2022
@nikomatsakis
Copy link
Contributor

@rfcbot reviewed

@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Jul 12, 2022
@rfcbot
Copy link

rfcbot commented Jul 12, 2022

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

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Jul 12, 2022
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Jul 22, 2022
@rfcbot
Copy link

rfcbot commented Jul 22, 2022

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

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

@joshtriplett joshtriplett added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. labels Aug 10, 2022
@scottmcm
Copy link
Member

Hello folks! This can't be closed yet, despite the FCP, because the feature still exists in the compiler under the register_attr feature gate.

If anyone would like to make a PR to remove the associated code (and tests) under the gate, it'd be appreciated! rg should help find anything relevant
image

@JohnTitor JohnTitor self-assigned this Aug 24, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 30, 2022
Remove `register_attr` feature

Closes rust-lang#66080

Signed-off-by: Yuki Okushi <jtitor@2k36.org>
@bors bors closed this as completed in c18292f Aug 30, 2022
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: #[attributes(..)] B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-close This PR / issue is in PFCP or FCP with a disposition to close it. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants