Skip to content

Conversation

nnethercote
Copy link
Contributor

Part of #137978.

r? @GuillaumeGomez

It was added in rust-lang#140052, but the subsequent changes in rust-lang#140252 means it
is no longer necessary. (Indeed, `Ident`s cannot be empty any more.)
`Ident`s can no longer be empty, so the test always succeeds.
@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-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels May 22, 2025
Box::new([].iter())
};
iter.filter(move |item| {
tcx.associated_items(assoc_items_of).filter_by_name_unhygienic(ident.name).filter(move |item| {
Copy link
Member

Choose a reason for hiding this comment

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

Unless I missed something, https://github.com/rust-lang/rust/blame/master/compiler/rustc_middle/src/ty/assoc.rs#L255 is still failing if the provided Symbol is empty. How come it's not triggering the assert anymore?

Copy link
Contributor Author

@nnethercote nnethercote May 22, 2025

Choose a reason for hiding this comment

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

As per the commit message, "the subsequent changes in #140252 means it is no longer necessary. (Indeed, Idents cannot be empty any more.)" More specifically, this part means the empty string case is handled earlier.

Copy link
Member

Choose a reason for hiding this comment

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

Should this assert be removed as well then?

Copy link
Member

Choose a reason for hiding this comment

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

(Can be done in a follow-up though)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The assertion in filter_by_name_unhygienic? The function takes a Symbol, which can be empty (unlike Ident) so I figure it was reasonable to leave it there.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see. A bit confusing. 😄

@GuillaumeGomez
Copy link
Member

Thanks for the detailed explanations and the update!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented May 22, 2025

📌 Commit c309065 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 May 22, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request May 22, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#141355 (ci: improve citool job db errors)
 - rust-lang#141359 (Fix `FnOnce` impl for `AsyncFn`/`AsyncFnMut` self-borrowing closures in new solver)
 - rust-lang#141362 (Normalize aliases to correct kind of error term)
 - rust-lang#141377 (Remove unnecessary `is_empty` checks)
 - rust-lang#141381 (try_cast_aligned: avoid bare int-to-ptr casts)
 - rust-lang#141382 (ci: convert distcheck to free runner)
 - rust-lang#141389 (ci: prepare aws access keys for migration)
 - rust-lang#141390 (Don't allow `poly_select` in new solver)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 74b980d into rust-lang:master May 22, 2025
6 checks passed
@rustbot rustbot added this to the 1.89.0 milestone May 22, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 22, 2025
Rollup merge of rust-lang#141377 - nnethercote:rm-unnecessary-is_empty-checks, r=GuillaumeGomez

Remove unnecessary `is_empty` checks

Part of rust-lang#137978.

r? `@GuillaumeGomez`
@nnethercote nnethercote deleted the rm-unnecessary-is_empty-checks branch May 22, 2025 18:07
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-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