-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Don't fix builtin index when Where clause is found #100598
Conversation
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.
Left a few style nits
src/test/mir-opt/issue-91633.rs
Outdated
fn hey<T> (it: &[T]) | ||
where | ||
[T] : std::ops::Index<usize>, | ||
{ | ||
let _ = &it[0]; | ||
} | ||
|
||
// EMIT_MIR issue_91633.bar.mir_map.0.mir | ||
fn bar<T> (it: Box<[T]>) | ||
where | ||
[T] : std::ops::Index<usize>, | ||
{ | ||
let _ = it[0]; | ||
} | ||
|
||
// EMIT_MIR issue_91633.fun.mir_map.0.mir | ||
fn fun<T> (it: &[T]) | ||
{ | ||
let _ = &it[0]; | ||
} | ||
|
||
// EMIT_MIR issue_91633.foo.mir_map.0.mir | ||
fn foo<T> (it: Box<[T]>) | ||
{ | ||
let _ = it[0]; | ||
} |
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.
nit: the index operation is getting compiled out of some of these, try making these return -> &T
or T
instead?
if let Some(elem_ty) = base_ty.builtin_index() { | ||
let exp_ty = typeck_results.expr_ty_adjusted(e); | ||
let resolved_exp_ty = self.resolve(exp_ty, &e.span); | ||
|
||
elem_ty == resolved_exp_ty && index_ty == self.fcx.tcx.types.usize | ||
} else { | ||
false | ||
} |
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.
Actually... it might be wrong to use expr_ty_adjusted
here. The un-adjusted expr should be &T
, so maybe try:
if let Some(elem_ty) = base_ty.builtin_index() { | |
let exp_ty = typeck_results.expr_ty_adjusted(e); | |
let resolved_exp_ty = self.resolve(exp_ty, &e.span); | |
elem_ty == resolved_exp_ty && index_ty == self.fcx.tcx.types.usize | |
} else { | |
false | |
} | |
let Some(expected_elem_ty) = base_ty.builtin_index() else { return false; }; | |
let Some(exp_ty) = typeck_results.expr_ty_opt(e) else { return false; }; | |
let ty::Ref(_, found_elem_ty, _) = self.resolve(exp_ty, &e.span).kind() else { return false; }; | |
expected_elem_ty == found_elem_ty && index_ty == self.fcx.tcx.types.usize |
This comment has been minimized.
This comment has been minimized.
Thanks so much for this fix! Can you squash this into one or two commits? Then r=me. @bors delegate+ |
✌️ @ouz-a can now approve this pull request |
@bors r+ |
Rollup of 9 pull requests Successful merges: - rust-lang#99576 (Do not allow `Drop` impl on foreign fundamental types) - rust-lang#100081 (never consider unsafe blocks unused if they would be required with deny(unsafe_op_in_unsafe_fn)) - rust-lang#100208 (make NOP dyn casts not require anything about the vtable) - rust-lang#100494 (Cleanup rustdoc themes) - rust-lang#100522 (Only check the `DefId` for the recursion check in MIR inliner.) - rust-lang#100592 (Manually implement Debug for ImportKind.) - rust-lang#100598 (Don't fix builtin index when Where clause is found) - rust-lang#100721 (Add diagnostics lints to `rustc_type_ir` module) - rust-lang#100731 (rustdoc: count deref and non-deref as same set of used methods) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Where clause shadows blanket impl for
Index
which causes normalization to not occur, which causes ICE to happen when we typeck.r? @compiler-errors
Fixes #91633