Skip to content

rustdoc: Remove AttributesExt trait magic that added needless complexity #135428

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

Merged
merged 3 commits into from
Jan 15, 2025

Conversation

camelid
Copy link
Member

@camelid camelid commented Jan 13, 2025

The new code is more explicit and avoids trait magic that added needless
complexity to this part of rustdoc.

It's never overridden, so it shouldn't be on the trait.
The two implementations were identical, so there's no need to use a
trait method.
@rustbot
Copy link
Collaborator

rustbot commented Jan 13, 2025

r? @GuillaumeGomez

rustbot has assigned @GuillaumeGomez.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jan 13, 2025
The new code is more explicit and avoids trait magic that added needless
complexity to this part of rustdoc.
.flat_map(|attr| attr.meta_item_list().unwrap_or_default())
let mut cfg = if doc_cfg_active || doc_auto_cfg_active {
let mut doc_cfg = attrs
.clone()
Copy link
Member

Choose a reason for hiding this comment

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

Before it didn't need to clone, is there a reason it needs to now?

Copy link
Member Author

@camelid camelid Jan 15, 2025

Choose a reason for hiding this comment

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

It's not really cloning the attributes. Before attrs was the attributes themselves, and we used the extension trait to make an iterator from them. Now, the iterator is created by the caller of this function and attrs is the iterator. So this is just cloning the iterator, which is a very cheap operation and equivalent to creating a new one using .iter() like before.

Copy link
Member

Choose a reason for hiding this comment

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

Ah I see, thanks for the explanation!

@GuillaumeGomez
Copy link
Member

Thanks!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jan 15, 2025

📌 Commit f92b32c has been approved by GuillaumeGomez

It is now in the queue for this repository.

@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 Jan 15, 2025
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jan 15, 2025
…omez

rustdoc: Remove `AttributesExt` trait magic that added needless complexity

The new code is more explicit and avoids trait magic that added needless
complexity to this part of rustdoc.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 15, 2025
…llaumeGomez

Rollup of 6 pull requests

Successful merges:

 - rust-lang#131806 (Treat other items as functions for the purpose of type-based search)
 - rust-lang#132654 (std: lazily allocate the main thread handle)
 - rust-lang#135003 (deprecate `std::intrinsics::transmute` etc, use `std::mem::*` instead)
 - rust-lang#135428 (rustdoc: Remove `AttributesExt` trait magic that added needless complexity)
 - rust-lang#135498 (Prefer lower `TraitUpcasting` candidates in selection)
 - rust-lang#135529 (remove outdated FIXME)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 15, 2025
…llaumeGomez

Rollup of 6 pull requests

Successful merges:

 - rust-lang#132654 (std: lazily allocate the main thread handle)
 - rust-lang#135003 (deprecate `std::intrinsics::transmute` etc, use `std::mem::*` instead)
 - rust-lang#135428 (rustdoc: Remove `AttributesExt` trait magic that added needless complexity)
 - rust-lang#135498 (Prefer lower `TraitUpcasting` candidates in selection)
 - rust-lang#135507 (TRPL: incorporate all backward-compatible Edition changes)
 - rust-lang#135529 (remove outdated FIXME)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b1d047b into rust-lang:master Jan 15, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Jan 15, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 15, 2025
Rollup merge of rust-lang#135428 - camelid:attr-cleanup, r=GuillaumeGomez

rustdoc: Remove `AttributesExt` trait magic that added needless complexity

The new code is more explicit and avoids trait magic that added needless
complexity to this part of rustdoc.
@camelid camelid deleted the attr-cleanup branch January 17, 2025 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants