-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Remove unnecessary is_empty
checks
#141377
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
Remove unnecessary is_empty
checks
#141377
Conversation
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.
Box::new([].iter()) | ||
}; | ||
iter.filter(move |item| { | ||
tcx.associated_items(assoc_items_of).filter_by_name_unhygienic(ident.name).filter(move |item| { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. 😄
Thanks for the detailed explanations and the update! @bors r+ rollup |
…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
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`
Part of #137978.
r? @GuillaumeGomez