Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

rustc_plugin: Remove Registry::register_attribute #66344

Merged
merged 1 commit into from
Nov 17, 2019

Conversation

petrochenkov
Copy link
Contributor

Legacy plugins cannot register inert attributes anymore.

The preferred replacement is to use register_tool (tracking issue).

#![register_tool(servo)]

#[servo::must_root]
struct S;

The more direct replacement is register_attribute (tracking issue)

#![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.

@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(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 12, 2019
@petrochenkov
Copy link
Contributor Author

cc @SimonSapin @rust-lang/dev-tools

@SimonSapin
Copy link
Contributor

@petrochenkov Would you be ok with waiting a bit before landing this? The alternative only landed very recently, and upgrading the Rust version used in Servo is currently blocked on fixing some new warnings: servo/servo#24684

@SimonSapin
Copy link
Contributor

SimonSapin commented Nov 12, 2019

I’ve verified that removing Registry::register_attribute calls produces errors that are fixed either by adding #![register_attr(…)] attributes, or by adding a #![register_tool(…)] attribute and a corresponding prefix path component to each use of the relevant attributes.

I’m afraid the latter renders the lint ineffective since it relies on TyCtxt::has_attr which takes a single Symbol. I didn’t find an equivalent for an attribute whose path has two components, but it shouldn’t be too hard to write.

@matthewjasper matthewjasper 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 12, 2019
Copy link
Contributor

@matthewjasper matthewjasper left a comment

Choose a reason for hiding this comment

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

Changes look good. I'm marking as blocked while Servo makes changes.

@bors
Copy link
Contributor

bors commented Nov 13, 2019

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

bors-servo pushed a commit to servo/servo that referenced this pull request Nov 15, 2019
Use `#![register_tool]` instead of `Registry::register_attribute`

CC rust-lang/rust#66344, rust-lang/rust#66079
bors-servo pushed a commit to servo/servo that referenced this pull request Nov 15, 2019
Use `#![register_tool]` instead of `Registry::register_attribute`

CC rust-lang/rust#66344, rust-lang/rust#66079
@SimonSapin
Copy link
Contributor

Alright, Servo should be unblocked now: servo/servo#24750

@SimonSapin SimonSapin 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 Nov 15, 2019
@petrochenkov
Copy link
Contributor Author

@bors r=matthewjasper

@bors
Copy link
Contributor

bors commented Nov 16, 2019

📌 Commit 8575743 has been approved by matthewjasper

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 16, 2019
Centril added a commit to Centril/rust that referenced this pull request 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 pull request 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.
bors added a commit that referenced this pull request Nov 17, 2019
Rollup of 11 pull requests

Successful merges:

 - #65739 (Improve documentation of `Vec::split_off(...)`)
 - #66271 (syntax: Keep string literals in ABIs and `asm!` more precisely)
 - #66344 (rustc_plugin: Remove `Registry::register_attribute`)
 - #66381 (find_deprecation: deprecation attr may be ill-formed meta.)
 - #66395 (Centralize panic macro documentation)
 - #66456 (Move `DIAGNOSTICS` usage to `rustc_driver`)
 - #66465 (add missing 'static lifetime in docs)
 - #66466 (miri panic_unwind: fix hack for SEH platforms)
 - #66469 (Use "field is never read" instead of "field is never used")
 - #66471 (Add test for issue 63116)
 - #66477 (Clarify transmute_copy documentation example)

Failed merges:

r? @ghost
@bors bors merged commit 8575743 into rust-lang:master Nov 17, 2019
@petrochenkov petrochenkov deleted the noregattr branch February 22, 2025 18:33
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants