[WIP] Use Into::into in operator ?#60796
Conversation
|
r? @varkor (rust_highfive has picked a reviewer for you, use r? to override) |
|
So yes, it did break inference in quite a few places in In rust/src/librustc_metadata/encoder.rs Lines 104 to 112 in 6968e77 AFAICT it should be both getting and returning |
This comment has been minimized.
This comment has been minimized.
|
I found the same |
This comment has been minimized.
This comment has been minimized.
|
Now that I have a green check mark, I'm moving to S-waiting-on-team to evaluate feasibility, please. |
|
I'd like to a see a crater run to see what sort of breakage we are talking about. |
|
@bors try |
[WIP] Use `Into::into` in operator `?` #38751 proposes using `into` for `?`, and while commenters suggested problems with inference, I couldn't find any evidence where this was actually attempted -- so here we go!
| fn test_try() { | ||
| fn try_result_some() -> Option<u8> { | ||
| let val = Ok(1)?; | ||
| let val = Ok::<_, NoneError>(1)?; |
There was a problem hiding this comment.
This is a huge ergonomic regression 😟
There was a problem hiding this comment.
Personally I'm not too bothered by this as Ok(1)? instead of Some(1)? is a silly thing to do; I don't know why Ok(1)? works in the first place...
There was a problem hiding this comment.
I guess inference can tell that the only possible From for NoneError is the blanket identity impl<T> From<T> for T, versus now inferring some T: Into<NoneError>.
|
💔 Test failed - checks-travis |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Yeah... I'm thinking this won't go anywhere unless we can improve inference. If that's feasible, I'm willing to explore it, but I could use some pointers. |
|
☔ The latest upstream changes (presumably #60949) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Triage: Should we just close this? The required inference changes would probably need to be discussed by T-compiler outside of this issue, and might even need an RFC (it doesn't seem clear how these changes would even look like). |
|
I would be interested in cratering this PR to see what the fuller outcome is. |
|
Ping from triage @cuviper This PR looks like conflicts just need to be resolved. |
Yeah, that's fine. At least now we have rough experimental data for #38751. |
#38751 proposes using
intofor?, and while commenters suggested problems with inference, I couldn't find any evidence where this was actually attempted -- so here we go!