Check discriminant types before span_mirbug #100498
Conversation
rustbot
added
the
T-compiler
|
r? @lcnr (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
| }); | ||
|
|
||
| if base_ty.builtin_index().is_some() | ||
| if self.is_builtin_index(&e, *base_ty, index_ty, base.span, index.span) | ||
| && index_ty == self.fcx.tcx.types.usize |
There was a problem hiding this comment.
You can move this into the is_builtin_index function too I think
| fn is_builtin_index( | ||
| &mut self, | ||
| e: &hir::Expr<'_>, | ||
| base: Ty<'tcx>, |
There was a problem hiding this comment.
(nit, not needed) maybe just me, but I prefer Ty<'tcx> variables to be called something_ty
| base_span: Span, | ||
| index_span: Span, | ||
| ) -> bool { | ||
| let mut ret = false; |
There was a problem hiding this comment.
Remove this in favor of return true or return false where needed.
| index_span: Span, | ||
| ) -> bool { | ||
| let mut ret = false; | ||
| if (&base).builtin_index().is_some() { |
There was a problem hiding this comment.
| if (&base).builtin_index().is_some() { | |
| if base.builtin_index().is_some() { |
Also, we could invert this, like:
if base.builtin_index().is_none() {
return false;
}
That saves us an indentation level.
| let resolved_base_ty = self.resolve(base, &base_span); | ||
| let resolved_index_ty = self.resolve(index, &index_span); | ||
| self.tcx().infer_ctxt().enter(|infcx| { | ||
| let mut selection_context = SelectionContext::new(&infcx); |
There was a problem hiding this comment.
nit: I think it's conventional to call the selection context selcx
| self.tcx().infer_ctxt().enter(|infcx| { | ||
| let mut selection_context = SelectionContext::new(&infcx); | ||
|
|
||
| let def_id = self.tcx().require_lang_item(LangItem::Index, Some(e.span)); |
There was a problem hiding this comment.
nit: We could probably use a let else here, and just return false; if this lang item is missing rather than erroring.
| if let Ok(Some(ImplSource::Param(..))) = results { | ||
| ret = false; | ||
| } else { | ||
| ret = true; | ||
| } | ||
| }); | ||
| } | ||
| ret |
There was a problem hiding this comment.
| if let Ok(Some(ImplSource::Param(..))) = results { | |
| ret = false; | |
| } else { | |
| ret = true; | |
| } | |
| }); | |
| } | |
| ret | |
| // Due to coherence, the only `impl` that could apply is the built-in one | |
| if let Ok(Some(ImplSource::UserDefined(..))) = results { | |
| return true; | |
| } | |
| }); | |
| } | |
| false |
|
Going to close this down as it differs greatly from the original solution. |
The compiler is unable to relate types when a user tries to write a bound forstd::ops::Index<usize>, and gives out an error, we bypass that by proving LHS and RHS are discriminantly the same types.We now check if the
exprisLangItem::Indexwhile insidefix_index_builtin_exprif so we don't execute the function.Mentored by @compiler-errors
The problem regarding the issue is explained greatly here by @danielhenrymantilla
Fixes #91633