Fix ICE when arg types can't be found in impl/trait methods while comparing #35877
Conversation
|
r? @pnkfelix (rust_highfive has picked a reviewer for you, use r? to override) |
|
Gah, this isn't as easy as I thought, we'd have to go through all wrapper types in the argument... |
| expected: ty::Ty<'tcx>, | ||
| found: ty::Ty<'tcx>) -> bool { | ||
| (impl_arg_ty == found && trait_arg_ty == expected) || | ||
| match (&impl_arg_ty.sty, &trait_arg_ty.sty) { |
There was a problem hiding this comment.
Hopefully I caught all the necessary types in this match...
|
Okay, discussions with @eddyb on IRC indicated that this enormous match against types is pretty hacky and inaccurate. Instead, we should either:
Both of which are much better than traversing through the type variants that I currently have in my implementation. |
|
The giant match is gone, and is now replaced with several passes to |
|
|
||
| if infcx.sub_types(false, origin, impl_sig.output, trait_sig.output).is_err() { | ||
| return (impl_m_output.span(), Some(trait_m_output.span())); | ||
| } |
There was a problem hiding this comment.
This should be last, because it's checked last in the whole function check.
|
Comments addressed. |
|
@bors r+ |
|
📌 Commit 3d9cf30 has been approved by |
|
I think this would have an ugly span when the mismatch is on the return type. e.g. when the trait fn is @bors r- |
| .unwrap_or_else(|| { | ||
| if infcx.sub_types(false, origin, impl_sig.output, | ||
| trait_sig.output).is_err() { | ||
| (impl_m_output.span(), Some(trait_m_output.span())) |
There was a problem hiding this comment.
@arielb1 I don't see a way to improve this? If we're not grabbing the span of the type, are we supposed to grab the span of the entire function declaration?
|
@bors r+ |
|
📌 Commit 3d9cf30 has been approved by |
|
|
||
| impl Foo for Bar { | ||
| fn foo(_: fn(u16) -> ()) {} | ||
| //~^ ERROR method `foo` has an incompatible type for trait |
There was a problem hiding this comment.
Nit: maybe this should be a ui test? It's hard to tell just what is being highlighted here. :)
|
I've not had a chance to read this patch carefully, but if @arielb1 is satisfied, I guess I am. ;) Still, I think a |
Fix ICE when arg types can't be found in impl/trait methods while comparing Fixes rust-lang#35869.
|
⌛ Testing commit 3d9cf30 with merge 698d4c9... |
|
⛄ The build was interrupted to prioritize another pull request. |
Fix ICE when arg types can't be found in impl/trait methods while comparing Fixes #35869.
|
Did this not fix the bug or is the playpen not updating anymore? |
Fixes #35869.