Skip to content

Conversation

DaniPopes
Copy link
Contributor

No description provided.

@rustbot
Copy link
Collaborator

rustbot commented Nov 3, 2023

r? @b-naber

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 3, 2023
@@ -41,8 +41,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
}
// Merge attributes into the inner expression.
if !e.attrs.is_empty() {
let old_attrs =
self.attrs.get(&ex.hir_id.local_id).map(|la| *la).unwrap_or(&[]);
let old_attrs = self.attrs.get(&ex.hir_id.local_id).copied().unwrap_or(&[]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let old_attrs = self.attrs.get(&ex.hir_id.local_id).copied().unwrap_or(&[]);
let old_attrs = self.attrs.get(&ex.hir_id.local_id).copied().unwrap_or_default();

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't feel strongly about it, but very slightly prefer unwrap_or: it's both fewer keystrokes and slightly less cognitive load for the reader (no need to consider the type or what its Default implementation is).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both are present in the codebase, I don't think this should be changed, but if anything I think it should be in a follow-up PR.

@b-naber
Copy link
Contributor

b-naber commented Nov 17, 2023

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Nov 17, 2023

📌 Commit 2736430 has been approved by b-naber

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 Nov 17, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 17, 2023
@nnethercote
Copy link
Contributor

How did you find these? Is it a clippy lint?

@DaniPopes
Copy link
Contributor Author

DaniPopes commented Nov 17, 2023

How did you find these? Is it a clippy lint?

Apparently there is one: https://rust-lang.github.io/rust-clippy/master/#/map_clone
But I just used a regex: \.map\(\|[^,]+\| \*\w+\)

bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 17, 2023
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#117338 (Remove asmjs)
 - rust-lang#117549 (Use `copied` instead of manual `map`)
 - rust-lang#117745 (Emit smir)
 - rust-lang#117964 (When using existing fn as module, don't claim it doesn't exist)
 - rust-lang#118006 (clarify `fn discriminant` guarantees: only free lifetimes may get erased)
 - rust-lang#118016 (Add stable mir members to triagebot config)
 - rust-lang#118022 (Miri subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit aa2289d into rust-lang:master Nov 18, 2023
@rustbot rustbot added this to the 1.76.0 milestone Nov 18, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 18, 2023
Rollup merge of rust-lang#117549 - DaniPopes:more-copied, r=b-naber

Use `copied` instead of manual `map`
@DaniPopes DaniPopes deleted the more-copied branch November 18, 2023 01:01
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-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants