Bad diagnostic for match ergonomics

[antoyo]

Hi.
Match ergonomics can create quite confusing errors.
For instance:

fn main() {
    let option = MyOption::MySome(String::new());
    option.take();
}

enum MyOption<T> {
    MyNone,
    MySome(T),
}

impl<T> MyOption<T> {
    fn take(&mut self) -> Option<T> {
        match self {
            MyOption::MySome(value) => {
                *self = MyOption::MyNone;
                Some(value)
            },
            MyOption::MyNone => None,
        }
    }
}

(playground)
gives the following error message:

error[E0308]: mismatched types
  --> src/main.rs:16:22
   |
16 |                 Some(value)
   |                      ^^^^^ expected type parameter, found &mut T
   |
   = note: expected type `T`
              found type `&mut T`

This error message is not even helping the user to figure out what the problem is.
Here, it's like if value was of type &mut T instead of T.

So, to avoid these issues, please add an option to disable the match ergonomics feature.

Thanks a lot.

[hadronized]

I strongly agree. On a more general note, I strongly think that the stabilization and feature gates mechanism should be reviewed. Haskell has GHC extensions and some of them (even though always used) have been there for quite a while now (MultiParamTypeClasses, FlexibleInstances, FlexibleContexts, etc.).

I would recommend to stop stabilizing features. To be more accurate:

• Stabilizing a feature should only be a matter of having the feature discussed enough, fully implemented and not ambiguous.
• Stabilizing shouldn’t imply removing the feature gate anymore.
• Feature gates should be available on the stable channel.

That would enable people to use what they love (new features) by just adding the #![feature(…)] annotation in lib.rs the same way we already do it in nightly.

[est31]

Related thread on irlo: https://internals.rust-lang.org/t/allow-disabling-of-ergonomic-features-on-a-per-crate-basis/7669

[varkor]

This error message is not even helping the user to figure out what the problem is.
[...]
So, to avoid these issues, please add an option to disable the match ergonomics feature.

If the problem is with the error message, it seems that the thing to do is improve the error message, rather than disable the feature that causes it.

[oli-obk]

The error message is entirely correct. the only issue is that the borrow checker runs after the type checker completed successfully. In this case, since the type checker failed, you did not get a borrowcheck error.

If you add a deref to the place of the error, you should start getting the helpful errors. Additionally we should be pointing to the value that is matched.

In order to get to borrowcheck errors in case of "trivial" typeck errors like missing derefs I wonder if we can treat such errors as if a deref occurred in order to continue execution, but additionally poison the location to make sure we don't report "move out of borrowed value" errors

[antoyo]

@varkor It seems that the bad error message is by design since the match ergonomics feature is enabled.

[oli-obk]

@antoyo there is no match ergonomics feature that you can disable. And noone creates suboptimal error messages on purpose. This is a simple case of diagnostics that need improving.

Your request most definitely needs an RFC, since it requires a major change (sub-language support) that has no precedent.

[antoyo]

And noone creates suboptimal error messages on purpose.

That not what I meant and I'm sorry if it was interpreted that way.
What I mean is that with the match ergonomics feature, since it's more implicit, the error message can be more confusing.

[oli-obk]

Indeed there are error message regressions. But my stance is that one should first unregress the errors and then think about whether bigger picture things are still necessary.

Sorry about reading more into that statement than what was written.

[est31]

@oli-obk whole RFCs have been opened due to bad error messages: rust-lang/rfcs#2479 ... one of the alternatives mentioned in thread is to simply improve error messages and fail at a later point in time.

[antoyo]

Please let's get back for what I asked: option to disable match ergonomics.
Please open another issue for improving the error message for the match ergonomics and discuss about it there.
I only mentionned this one problem with match ergonomics as an example, but there are more.

[Manishearth]

Adding a different feature switching mechanism definitely needs an RFC.

Furthermore, this is a diagnostics issue first and foremost.

Yes, there is an RFC opened for a diagnostic issue, which was when there seemed to be sufficient reason to actually support a pattern for familiarity. You can open an RFC for this too, but I'm pretty certain folks will want to try to fix this purely with diagnostics first 😄

[Manishearth]

(If you're really interested in making this happen I'd suggest a thread on https://internals.rust-lang.org/ to flesh out ideas first)

[GuillaumeGomez]

This diagnostic issue is still present. Should we open an issue for it? Meanwhile, I'll reopen the issue with a different title (please close it when the other "clean" issue has been open).

[Manishearth]

I don't think it's helpful for triage or in general if the issue text and discussion is still focused around the former. Anyone can open the diagnostics issue as a separate one if they wish.

[GuillaumeGomez]

Opening a new issue with just saying "the diagnostic is bad" didn't seem very helpful to me considering that it might be very tricky to fix in here. So I was hoping someone with more insight knowledge might open another issue with some ideas in order to, if not starting a debate, propose an better diagnostic. Anyway, I'll open the issue with just "the diagnostic is bad" then to not lose track of it.

[M-PC]

Close it, thank you

…

On 17 Mar 2020 1:01 a.m., "Manish Goregaokar" ***@***.***> wrote: Closed #51743 <#51743>. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#51743 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGJEPHDC2BKQHLZZHWJ3GDDRHZLOBANCNFSM4FGTWNNQ> .