-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Handle mismatched generic param kinds in trait impls betterly #96717
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
Handle mismatched generic param kinds in trait impls betterly #96717
Conversation
@bors r+ rollup |
📌 Commit 52916657e53bfd5f91190953708120a5be5b2641 has been approved by |
@bors r- |
5291665
to
e64b4d1
Compare
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 you add a test for the following? after that r=me
trait Trait {
fn func<U, const N: usize>() {}
}
impl Trait for () {
fn func<const N: usize, U>() {}
}
7ee1b25
to
0cf7604
Compare
0cf7604
to
fea1d76
Compare
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.
2 more nits, then r=me
} { | ||
let make_param_message = |prefix: &str, param: &ty::GenericParamDef| match param.kind { | ||
Const { .. } => { | ||
format!("{} const parameter with type `{}`", prefix, tcx.type_of(param.def_id)) |
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.
format!("{} const parameter with type `{}`", prefix, tcx.type_of(param.def_id)) | |
format!("{} const parameter of type `{}`", prefix, tcx.type_of(param.def_id)) |
For me "of type" seems more natural here, idk 😅
Considering that this closure doesn't capture anything, can you move it out of the loop to ty_const_params_of
?
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.
Changed the error wording.
I don't think it'd be good to have the closure be by ty_const_params_of
since then its really far away from the code actually calling the closure. I pushed code moving it furthur down so its closer to the call site, imo makes it easier to understand the code.
@bors r+ rollup |
📌 Commit e4b8ed5 has been approved by |
Rollup of 6 pull requests Successful merges: - rust-lang#96717 (Handle mismatched generic param kinds in trait impls betterly) - rust-lang#96725 (Expose process windows_process_extensions_main_thread_handle on Windows) - rust-lang#96849 (Move some tests to more reasonable places) - rust-lang#96861 (Use Rust 2021 prelude in std itself.) - rust-lang#96879 (rustdoc: search result ranking fix) - rust-lang#96882 (Don't subst an AdtDef with its own substs) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
r? @lcnr