Improve diag when calling method on result of index op #125985
Conversation
rustbot
added
S-waiting-on-review
| } else { | ||
| let e = self | ||
| .tainted_by_errors() | ||
| .or_else(|| self.report_ambiguity_errors()) |
There was a problem hiding this comment.
I don't believe it's correct to call report_ambiguity_errors here. Please look at the definition of that function -- since it calls collect_remaining_errors, it essentially forces all obligations that are currently waiting on inference to be reported as errors. It's only meant to be called at the end of type checking.
There was a problem hiding this comment.
Calling report_ambiguity_errors cause the inference of recv_t has failed, so I want to check if there are any remaining errors and emit them. If no such errors, then just emit the inference failure of recv.
There was a problem hiding this comment.
Is there any ways to do such things? cause I'd like to suggest like what thing[i]; did.
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
@rustbot ready @compiler-errors I pushed the new change and it just replace the span. |
rustbot
added
S-waiting-on-review
| @@ -1330,7 +1330,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |||
| ) -> Ty<'tcx> { | |||
| let rcvr_t = self.check_expr(rcvr); | |||
| // no need to check for bot/err -- callee does that | |||
| let rcvr_t = self.structurally_resolve_type(rcvr.span, rcvr_t); | |||
| let rcvr_t = if let ExprKind::Index(_, index, _) = rcvr.kind { | |||
| self.structurally_resolve_type(index.span, rcvr_t) | |||
There was a problem hiding this comment.
This is not correct in case the ambiguity is not due to the index expression:
fn main() {
let x = vec![];
x[0usize].method();
}error[E0282]: type annotations needed for `Vec<_>`
--> /home/michael/test.rs:2:9
|
2 | let x = vec![];
| ^
3 | x[0usize].method();
| ------ type must be known at this point
See above how after this PR, we now underline the 0usize even though it is not ambiguous, and there is a single implementation that satisfies the index operator.
Again, I really don't think we should make this change in such a special case.
Systematically determining why the method receiver is ambiguous due to inference is difficult, and I'd rather not patch a single case since it doesn't seem to me more special than any other example.
There was a problem hiding this comment.
How about the new change?
There was a problem hiding this comment.
This is not correct for cases where the key's ambiguity doesn't affect the value's ambiguity. Consider HashMap<i32, _>.
mu001999
force-pushed
the
fix/125924
branch
3 times, most recently
from
1dbaf61 to
6b7c6d6
Compare
There was a problem hiding this comment.
This is still not the right approach, and I really don't think there is going to be a correct approach to handle this without a very involved algorithm to deduce the best "blame" span for an ambiguity. Anything simpler has a very high risk of causing false-positives where we point at the completely wrong span for the ambiguity. That's what I meant above when I said:
Systematically determining why the method receiver is ambiguous due to inference is difficult, and I'd rather not patch a single case since it doesn't seem to me more special than any other example.
For this specific approach, calling report_ambiguity_errors will cause us to report errors for any ambiguity we still have in the function body, even if it has nothing to do with the inference failure of the method call for an indexed receiver.
For example:
fn main() {
// A totally unrelated variable, which is ambiguous until after the index operation happens below.
let mut y = Default::default();
// Now we have an unconstrained vec, and we try to call a method on its element (which is unconstrained).
let x = vec![];
x[0usize].method();
// We constrain the unrelated variable. This is *okay*, since `y` has nothing to do with `x` or the method call above.
y = 1i32;
}error[E0790]: cannot call associated function on trait without specifying the corresponding `impl` type
--> /home/michael/test.rs:3:17
|
3 | let mut y = Default::default();
| ^^^^^^^^^^^^^^^^^^ cannot call associated function of trait
|
help: use a fully-qualified path to a specific available implementation
|
3 | let mut y = </* self type */ as Default>::default();
| +++++++++++++++++++ +
error: aborting due to 1 previous error
|
@compiler-errors Thanks for the example! It's very clear to me. |
|
@compiler-errors I am trying to check if the index ty is known at this point, then use |
|
@compiler-errors I pushed new change, it only uses |
|
☔ The latest upstream changes (presumably #126016) made this pull request unmergeable. Please resolve the merge conflicts. |
…on result of index op
There was a problem hiding this comment.
Sorry for all the back and forth, but I'm going to go ahead and repeat myself by saying that this is probably not possible to fix in general, and I don't think we need to have this special casing for such a rare case.
Supporting this in general, which is probably what @oli-obk wanted in the original diagnostic issue, would require far more invasive changes to the way we track ambiguity in the type checker.
| @@ -1330,7 +1330,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |||
| ) -> Ty<'tcx> { | |||
| let rcvr_t = self.check_expr(rcvr); | |||
| // no need to check for bot/err -- callee does that | |||
| let rcvr_t = self.structurally_resolve_type(rcvr.span, rcvr_t); | |||
| let rcvr_t = if let ExprKind::Index(_, index, _) = rcvr.kind { | |||
| self.structurally_resolve_type(index.span, rcvr_t) | |||
There was a problem hiding this comment.
This is not correct for cases where the key's ambiguity doesn't affect the value's ambiguity. Consider HashMap<i32, _>.
rustbot
added
S-waiting-on-author
|
I'm gonna go ahead and close this since I don't think there's a simple tweak to this PR to make it work in general |
Fixes #125924