Bring enum variants in scope for patterns

1

Like the comment about default lifetime parameters, this is something I'd like to see happen, but won't push forward.

The one way in which scoped enum variants is quite annoying is when you are matching on them:

match x {
    Foo::Bar  => ...
    Foo::Baz  => ...
    Foo::Quux => ...
}

Especially if the type name is long, it can get quite annoying. There have been various proposals to fix this (e.g. allowing _::Bar), here is mine.

For patterns, variants of the type of the scrutinee (I don't love this term!) should be brought into scope. This is basic typed based resolution, just like how methods on the type are in scope when the . operator is applied to a value. Nothing particularly magic here. Now, its very easy, you can just write variants:

match x {
    Bar  => ...
    Baz  => ...
    Quux => ...
}

However, this would be a breaking change because you could have used a nullary variant as an ident today, intending to match all patterns with that name. This would be a very poor choice, but it's allowed.

So I would propose to make this change at an edition boundary, with rust fix changing any code which uses a variant name as a binding pattern to change it to Bar @ _, in which Bar is therefore unambiguously a binding.

I read the discussion on namespacing enums last night (many users probably don't know that enum variants were not namespaced at all until shortly before 1.0), and this possibility was brought up very early on. It was even pointed out that this is how Java enums work (they are namespaced, but not in switch statements). In the 5 and a half years since that discussion (my god!), no additional issue with this way of doing things other than the incompatibility with very strange code has arisen, as far as I know.

28 Likes
2

Actually there is also an issue with consts:

const Bar: Foo = Foo::Bar;

match x {
    Bar => ...
}

Since this is not a binding, rustfix cannot change it to Bar @ _ obviously. I would suggest making this an ambiguity error just like if two methods were both in scope and for rust fix to disambiguate the path to the const.

EDIT: Actually, variants are prefered over consts for associated consts right now, so I guess we should do that here as well: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=4f67c35124881b71ccbd318221c01474

1 Like
3

Could we allow use in a match block?

Thus making the following work:

enum MyEnum {
    A,
    B,
    C,
    D
}

fn main() {
    let k = MyEnum::A;
    
    match k {
        use MyEnum::*;
        A => {},
        B => {},
        C => {},
        D => {},
    }
}
4

We could but I don't see a lot of benefit to that since the user can also just use in the same scope as the match block. The reason I don't do that today is not to avoid "poluting" that scope, but because its hard to discern where the trade off is that adding that import is less "mess and frustration" than just repeating the name of the type.

8 Likes
5

I think this would be a very valuable change even without any changes to scoping of enum variants. People so often accidentally write exhaustive patterns when they meant to match against a specific constant or variant that I think this change is worth it just by itself.

We do have a lint for this, but even then this seems like fairly surprising behavior that I wouldn't really expect from Rust.

9 Likes
6

There's another potential problem that makes this a breaking change: an enum variant could conflict with the top-level name of another type.

enum SomeEnum {
    Name1(u32),
    Name2(u64),
    OtherVariant(Name1, Name2),
}

struct Name1(u8);
struct Name2(u8);

Bringing SomeEnum::* into scope inside the match would conflict with the top-level Name1 and Name2 structs (or with any other structure name). So that would break code like this:

match x {
    SomeEnum::OtherVariant(Name1(5), Name2(10)) => { ... }
    ...
}
7

I wrote imprecisely, but I would expect type-based dispatch to depend on type of the scrutinee of the pattern at each level, not simply bring it into scope for the entire match statement. Its obviously an error to attempt to match the enum variant against the value of type Name1.

5 Likes
8

Ah, I see.

That would certainly dodge some potential conflicts, but it also seems much more error-prone to me, particularly in the face of multiple enums with the same variant names. I'm sure that the compiler could learn to understand such code, but I feel that would make the code less understandable at a glance.

Two specific concerns, In particular:

  1. If people use SomeVariantName in the pattern, they may reasonably expect they can also use it in a condition or the code in that pattern's code block; consider code like SomeVariantName(x) => SomeVariantName(x+1). Having different names in scope for the pattern and the subsequent code feels confusing to me.
  2. Worse yet, if it turns out that SomeVariantName is also the name of a different structure or enum variant or similar in scope, there's a name conflict. For instance, imagine having two different enums EnumA and EnumB, both with variants V1 and V2. (This is not at all unreasonable to do, if you have several different types in the same problem domain, and V1 and V2 are components of the problem domain. I have code like that.)

(Note: when evaluating this, don't just think about code that's already correct. Consider code that's in the process of being written and repeatedly refactored, and which goes through various states of "not currently working" in the editor. I don't want to reduce the ability of the compiler to help the developer find places they need to work on. I regularly do things like changing a type definition or function signature in one place, and let the compiler tell me everywhere else that needs to change.)

Personally, if we want to simplify matches on enums, I would prefer to just bring the variant names from the enum completely in scope for the entire match block (not just the patterns or parts of the patterns), and just have that be an edition change with rustfix support for migrating code. In the process, we could also detect any shadowing that might cause, and warn about it.

3 Likes
9

I'm just going to link to some other discussions that I think are also relevant:

1 Like
10

Stupid question: can't the compiler use the type info from the type of the match variable in this case? If the variable after the match keyword is an enum, it should do this? Or doesn't it know that yet when it would need to bring the variants into scope?

1 Like
11

Type based resolution in the compiler is already hard to understand, so I'd like to avoid either of these, but of the two, @withoutboats's proposal is the modular one which I feel is least invasive to how the compiler is organized. In terms of specification and plumbing, the former sounds like it would take the expected: Ty<'tcx> in fn check_pat and send it as hints to the resolution methods for PatKind::{Struct, TupleStruct, Path}. This would work for all contexts in which patterns exist. I believe you'd also need to take references and default match bindings into account (peeling them off), which is another complication here.

As for the motivation aspect here, I don't think it warrants the complication. I don't feel that removing the type's name from the variant produces more readable code in the common case, and in the cases where it does, usually a glob import, type alias, or Self::Variant(...) works. Moreover, it seems to me that removing the variant name would make rging for places where the variant occurs harder.

4 Likes
12

How would this whole idea interact with type-inference based on the patterns in a match?

Currently this compiles:

fn main() {
    let f = |x| match x { 1u8 => (), _ => () };
    println!("{:?}",f(2));
}

The type of x is clearly determined only by the pattern 1u8.

1 Like
13

How this happens: The scrutinee x is assigned an inference variable ?0 when its type checked because we don't know x's type yet. Later, each pattern in each arm is type checked against the scrutinee's type. We then unify the literal with ?0 and assign ?0 the type u8.

If after resolving the inference variable you still end up with an inference variable in the passed in expected-type-hint to the resolution methods, I believe you would have no choice but to give up bringing the variants into scope for that pattern.

1 Like
14

I guess another option is to allow the following:

match x {
    _::VarA => {},
    ...
}
8 Likes
15

I do very much like that idea.

1 Like
16

Since we're talking breaking changes, maybe we should change the syntax to forbid single identifiers in match arms; patterns (and sub-patterns) would have to be either bindings or unambiguous expressions.

To reuse your example:

const Bar: Foo = Foo::Bar;
const Bar2: Foo = Foo::Bar;

match x {
    Bar => something(),        // Enum variant
    (Bar) => something(),      // Constant
    Bar2 => something(),       // Syntax error
    Bar2 + 1 => something(),   // Syntactically valid, but type error
}

Being able to use identifiers in a way that makes them indistinguishable from enum variants when reading the code seems like an anti-feature to me.

1 Like
17

Looks kind of ugly.

2 Likes
18

I feel like every possible solution to this problem has been enumerated and all the pros/cons spelled out, and now it's mostly a matter of polling everyone, so:

IMO doing nothing and doing @withoutboats' "no extra syntax" proposal are about tied for the best options.

Allowing use or _ in more places seem like changes that, if they're worth doing at all, then all of the same motivation applies to "no extra syntax" too, so we might as well go all the way.

19

Would this only apply to the top-level scrutinee? I see matching on Options of enums relatively commonly, so would you also be able to write

match Some(Foo::Bar) {
    Some(Bar) => {}
    Some(Baz) => {}
    Some(Quux) => {}
    None => {}
}

It would also be useful to be able to do the same thing for manual struct+associated const enums as well, you can't even currently use them to bring the consts into scope :frowning: (though that is likely much more controversial)

#[derive(PartialEq, Eq)]
struct Foo(u8);

impl Foo {
    const BAR: Self = Self(0);
    const BAZ: Self = Self(1);
    const QUUX: Self = Self(2);
}

match Foo::BAR {
    BAR => {}
    BAZ => {}
    QUUX => {}
    _ => {}
}
2 Likes
20

This would be fantastic. I've had to deal with horror cases like:

let is_timeout = match error {
   LibraryError::FailedRequest(RequestError::ConnectionFailed(ConnectionError::Timeout))) => true,
}

I'd love to simplify this without adding several potentially-conflicting use statements:

let is_timeout = match error {
   FailedRequest(ConnectionFailed(Timeout))) => true,
}

This would also help matching types that can't be easily named. Sometimes libraries forget to export their dependency:

pub enum Error {
   MyDependencyError(unexported_crate::Error)
}

so I can't refer to unexported_crate (without pulling it separately, which is cumbersome and requires keeping major versions in sync), so I can't match on its variants:

match err {
    MyDependencyError(unexported_crate::Error::Variant) =>
}

but with automatic scope I could:

match err {
    MyDependencyError(Variant) =>
}
10 Likes