[IDEA] Implied enum types

language design
21

Hey, thanks for the feedback. This is also an issue in just normal functions. When you are writing in an ide, it would be shown in intellsence. Even so, this change is TINY. It would not take lots to implement and would make code much cleaner.

// Chinese has last name first
fn greet(last: String, first: String) {
    println!("Hello {} {}", last, first);
}

// English (Danial Jackson
greet("Jackson", "Danial")
// Chinese (王孜继)
greet("王",  "孜继");

In this example, there is no way for the compiler to know if you want the first name or last. I don't want to have one million imports for enums/structs. If you don't like the features, you don;t have to use it, but i think the people that use swift love it.

22

Sure, and I would say that that function (greet) is problematic because it has two parameters of the same type that can easily be swapped - in my preferred coding style, once we're sticking with this function, we'd use wrapper types with trivial conversions to ensure that you can't swap the two names around.

Saying that there's other cases where you can already make a similar mistake doesn't help with either of the justifications: you need to do one of two things:

  1. Convince people that nobody would want to lint against this suggestion. So far, you've failed to do that - indeed, I see you suggesting that people should lint against it if they don't like it.
  2. Convince people that this feature enables things that we couldn't otherwise express, and thus that even with people mostly linting against it, we'd still want it.

You say you don't want to have too many imports, and that IDE support will make this feature work well - but if you're using an IDE, you should be able to configure it to automatically "fold" imports and thus hide them from you.

Since you're trying to convince us that this feature is valuable as a language feature, you need to decide whether you expect people to be using IDEs, or just plain text views.

If people are using IDEs, why does this feature need to go in the language, not the IDE (an IDE can hide the enum prefix from you, after all, if it's unambiguous from context, and display it on mouse-over or other action)?

If people aren't using IDEs, how does this make the code clearer in total? I can see why it makes the code easier to write, but most code is read far more often than it's written, and I can't see how having to look up the function declaration to find out what the parameter actually is would help read the code.

2 Likes
23

Yeah, even though it just may take a few more seconds to import the enum using intelisence, it still takes time out of my day if i'm writing huge amounts of code. Example:

.add_col(
    ColumnDef::new("example", ColumnTypes::VarChar(64))
)

.add_col(
    ColumnDef::new("example", ::VarChar(64))
)

This is especially important when working with sql and migrations because those take a long time to write and are offten cluttered and repeated a lot.

In text highlighters like github with no intelsence, I don't think it's very clean and easy to read if we write long hand expresions.

24

But why can't your IDE notice you typing :: and fill in that you meant ColumnTypes::? That'd get you exactly what you want for writing code without needing a language change, plus it would prompt you as soon as you write :: if you've

And if you're using the same enum repeatedly, use crate::ColumnTypes::* or use crate::ColumnTypes as CT shrinks the expression. Then, when reading the file in a text viewer, I can see the use and know that unqualified enum variants (or CT::VarChar) is actually ColumnTypes. Again, your IDE can hide the use statement from you if that's a problem.

1 Like
25

This has been mentioned as a drawback of this feature in previous conversations (and I tend to agree), but I just realized that this would be relatively trivial to address with a lint: check all arguments in a method call, collect all arguments that are enum types, check whether there are overlapping variant names, check if any of the arguments passed in are using the implied enum syntax on any of those. If so, complain and request the full qualified path be used.

3 Likes
26

Depending on how you write this lint, it's likely to be either noisy or not fire often enough.

If it lints because you've used a variant where two different enums are visible and have a variant of that name, you get a lot of noise from things like crate::dangerous::DangerousOptionsDoNotTouch has a variant Disabled, but so does the locally defined enum SafeThingsToPlayWith, and you really did mean to call set_the_switches(switch_position: SafeThingsToPlayWith) -> Result<(), ErrorType> with SafeThingsToPlayWith::Disabled.

If it only lints when there are multiple enum types in the function signature, then you get the problem of me typing set_the_switches(Disabled) meaning SafeThingsToPlayWith::Disabled, not realising that set_the_switches in scope here has signature set_the_switches(switch_position: crate::dangerous::DangerousOptionsDoNotTouch) -> Result<(), ErrorType>.

This is a lot of why I'd like the code to be explicit about anything non-local, with the IDE hiding the noise. One of my problems with C++ is that when reviewing patches, you end up having to look all over the place to see exactly what the compiler is going to choose to do when you do anything (thanks to Koenig lookup, copy constructors and other features to let you implicitly make things work). In contrast, Rust code tends to only let you elide things when the context is local, and thus I need only look up or down a couple of lines to see what's meant.

1 Like
27

I don't know why it's not explicit. Can you explain? I am not really an expert in compilers.

28

Take the following code snippet:

fn do_the_thing() {
    // This `let` line is implicit and local
    let x = "I am a string".to_string();

    // This line is explicit: about the parameter type for `foo_fn`, but
    // has an implicit return type.
    let y = foo_fn(FooEnum::Baz);

    // And this makes the return type explicit
    let z: FooEnum = foo_fn(FooEnum::Quux);

    // FooEnum's variants are too long to keep typing out,
    // so let's make the `FooEnum::` implicit
    use FooEnum::*;
    let a = foo_fn(Plugh);

    // Still OK because no ambiguity. Yet.
    let b = foo_fn(NotPresent);

    use std::env::VarError::*;
    // No longer allowed, because this could be FooEnum::NotPresent
    // or VarError::NotPresent
    let c = foo_fn(NotPresent);

    use FooEnum::NotPresent;
    // And now disambiguated by the explicit import
    let d = foo_fn(NotPresent);
}

Rust does allow some implicitness, where you don't specify things in the code, and the compiler knows what you mean, but that implicitness is used with care. Implicitness can be a benefit by making the code more concise - as in the let x line, where no experienced Rustacean would expect x to be any type other than String, but it can also make the code harder to understand, as in the let y line, where you cannot see what type y has from the code in front of you until you see the let z line.

It also lets you add more implicitness with use statements, where the result is unambiguous and where you think it makes the code more readable. If you use a wildcard in your use statement (as I did with use FooEnum::*), this only imports the names that are unambiguous, and you need to disambiguate when you're unclear.

C++ makes it worse with the ability to find constructors to call for you - if Rust adopted C++'s level of implicitness, I wouldn't even be able to see that foo_fn took a FooEnum as parameter, because C++ would implicitly add the equivalent of .into() calls to make it work out.

There's also a useful concept called "local reasoning" - in short, humans find it easier to reason things out by just looking at the stuff in front of them, and take shortcuts as you make them look further afield for the "right" answer. Local reasoning is extremely powerful, since the more local your reasoning can be, the better other humans are at repeating it (and hence at spotting errors in your code). Rust is heavy on making local reasoning work - where there's non-local action, you tend to have to invoke it deliberately (e.g. where C++ does implicit conversions via single-parameter constructors, Rust uses .into and ::from methods), which makes it clear to the reader that there's cleverness here.

4 Likes
29

Well imagine that a function can only have one type of enum which is true. The _:: operator represents the enum for that given function. It is only posible for the enum to be of the same type as the argument. I agree that c++ makes a lot of horrible implicit things like auto but this is not that:

auto foo(bool option) {
    if (option) { return 1; }
    return 2;
}

:face_vomiting::face_vomiting::face_vomiting:.

This is horrible because it is ambiguous, if you return a float in one of the return types by accident, it will give you an error. but because you use auto, you don't know if the return type is meant to be float or int.

When you would try to fix it, it leads to the float being trimmed but maybe you wanted to round it.

auto foo(bool option) {
    if (option) { return 1.5; }
    return 2.2;
}

int main() {
    // 1.5 -> 1
    int a = foo();
    return 0;
}

I think this is much different to the C++ auto and it's much better.

30

Counter-example, and thus why I think this should be an IDE feature:

I have a function set_wifi_state(config: RadioConfig). I realise I've fouled up my design because I forgot about the Zigbee and Bluetooth radios, and this function should be split into two: set_radio_state(config: RadioConfig) and set_wifi_state(config: WifiConfig), but the two enums have variants with the same name.

With the current world, the moment I change set_wifi_state to take a WifiConfig, all calls to set_wifi_state fail to compile. I can now look at each one, and determine the correct outcome: should set_wifi_state(RadioConfig::Disabled) in each location be set_wifi_state(WifiConfig::Disabled) or set_radio_state(RadioConfig::Disabled)?

With your suggested change, all of the calls to set_wifi_state(Disabled) are ambiguous; did I mean set_wifi_state(WifiConfig::Disabled), or did I mean set_radio_state(RadioState::Disabled)? The compiler will assume the former - which is wrong if it really was meant to be the latter.

I can't have this ambiguity in today's Rust; if I'd written use RadioConfig::* to make set_wifi_state(Disabled) work, then it fails to compile as I'd expected. If I add use WifiConfig::*, then Disabled is ambiguous, and Rust complains.

With your change, the compiler has an unambiguous and wrong resolution to the ambiguity I've introduced by fixing my design fault - if, in my earlier faulty design, I'd written set_wifi_state(Disabled), where the new design wants it to be set_radio_state(Disabled), the compiler happily adds bugs to my code, because it re-infers Disabled to be WifiConfig::Disabled, where I really, really meant RadioConfig::Disabled.

This is why I think this should be IDE functionality - you want one thing that can add the use Enum::*; statement for you, and thus make the code much shorter while keeping it clear, and another that hides the Enum:: prefix in Enum::Variant for people who prefer the code to look shorter.

6 Likes
31

Thats not a language problem, if you decide to refactor, you would need to fix it yourself? Also, If you don't want to use it, you don't have to.

32

If you use the feature, and I refactor later because your design was inadequate to today's problem, it is a language problem.

Without your change, I attempt my refactor, and the compiler complains that I've made a mistake - it's unambiguous to the compiler that I've got it wrong, because I've got a type error - and this applies even if you use the use RadioConfig::* trick to allow you to write set_wifi_state(RadioConfig::Disabled) as set_wifi_state(Disabled). With your suggested change, the compiler sees a correct variant of my new WifiConfig enum, and doesn't flag up to me that I've missed a spot.

This is fundamental to anything that makes things more implicit - you need only spend time on users.rust-lang.org to see users getting surprised when the various lifetime elision rules result in a lifetime surprising them - for example, the following generates a lifetime error that people get surprised by (playground link:

fn box_it<T: Trait>(it: T) -> Box<dyn Trait> {
    Box::new(it)
}

The explanation for the error is simple; this function has an implicit lifetime, and is in fact a short-hand way of writing:

fn box_it<T: Trait>(it: T) -> Box<dyn Trait + 'static> {
    Box::new(it)
}

But even this level of implicitness is confusing to some people - because while the compiler error suggests a fix (changing the bound from T: Trait to T: Trait + 'static), it doesn't explain why it suggests that change, or how you'd fix it if you wanted to allow it to be bounded by an arbitrary lifetime instead of 'static.

This is why I think this should be an IDE feature, not a language feature - my IDE of choice already knows how to add to what I'm typing (e.g. it knows to convert foo.letm to let mut = foo and put my cursor between the mut and the =), how to add in type inlays and display fn foo(&self) -> &str as fn<'self> foo(&'self self) -> &'self str, and it can also remove things from display - so it can display the code the way you want it, and make it easier to type.

Finally, "Also, If you don't want to use it, you don't have to." is not helpful - if anyone I collaborate with uses a language feature, I have to deal with it. If that's the only way you can handle an objection, then you're basically saying that the feature shouldn't be in the language because it can't be used at all.

33

But that is different from a language feature, that's a bug in your code. The compiler shouldn't tell you if you changed the wifi state instead of the radio state. You are just telling the computer what to do! This is out of the scope for the compiler. The implicit lifetimes are different because they effect compiles and memory safety.

34

I am completely confused about what you're trying to say here - I showed a language feature that leads to confusion already (elided lifetimes in Box<dyn Trait>), and I showed code that, in current Rust, is a compile error, and in your proposed change is compilable code with a bug in it.

Bearing in mind that "memory safety" is simply a name for a well-understood class of bug, I'm lost trying to understand you. You're saying that your proposed feature (which affects compilation and introduces a class of bug) is different to an existing feature because the existing feature affects compilation and a class of bug)' this doesn't make sense to me.

35

What I mean is that that is a logic problem that is out of scope of the compiler. I feel as if your saying the compiler should be checking if the function your calling is correct. These bugs are totally different. I don't know but it feels like you're saying:

fn add(num1: usize, num2: usize) -> usize {
    num1 * num2
    // ^ Error: did you mean add instead of multiply?
}
36

I think this is the crux of our difference of opinion. I, like a lot of people in the community (but not all), find Rust attractive because it's designed to make it easier to write correct code (bug-free code). If the compiler could issue the error you describe, that feels like a great thing to do - you've got a logic error, and the compiler is telling you that you've made a mistake so that you can fix it as early as possible in the process. Given the context you've supplied it in, am I right to believe that you think the compiler shouldn't issue an error in this case, even if it could detect that this was a logic bug? If so, that's the core difference of opinion.

And note that the entire design of unsafe is built around this assumption - memory safety bugs are just a type of logic errors that happen to have disastrous consequences for the program. If we thought the compiler shouldn't error on logic errors, then we'd get rid of unsafe, and just require you to not have logic errors, otherwise the compiler does what you told it to do.

3 Likes
37

I like this kind of inference and don't think it really adds bugs. It's kinda a fact of life that having enums of any kind either brakes namespacing, is much less useful than you'd like, or requires somewhat complicated inference rules like discussed here (see Niklaus Wirth's design rationale for omitting enums in Oberon-07). The fact that rust has use Enum::* at all makes them more useful than some other "strong" enum implementations.

I find this particularly egregious when matching over enums, generally once the match statement opens I always want the enumerators of the enum I'm matching over in-scope, and inferring things like this makes code more readable in most cases (you already know good and well what the enum type is since it's often in the match expressions head).

2 Likes
38

The case with a match makes some sense, but one of my reasons for preferring Rust over C++ is that C++ is hard to review in patch form.

If I give you:

fn do_the_thing() -> Result<(), Error> {
    set_radio_state(AllUnblocked)?;
    set_wifi_state(Disabled)?;
    set_zigbee_state(Channel5)?;
    Ok(())
}

and ask you what types are being passed to set_radio_state, set_wifi_state and set_zigbee_state, you're not going to be able to - you'll need to look at the function definitions to see what's going on.

In contrast, with:

fn do_the_thing() -> Result<(), Error> {
    use RadioState::*;
    use WifiState::*;
    use ZigbeeState::*;
    set_radio_state(AllUnblocked)?;
    set_wifi_state(Disabled)?;
    set_zigbee_state(Channel5)?;
    Ok(())
}

you have a reasonable chance of guessing what enums are being passed to which functions. And if I go fully explicit, you can see what I'm doing in isolation.

fn do_the_thing() -> Result<(), Error> {
    set_radio_state(RadioState::AllUnblocked)?;
    set_wifi_state(WifiState::Disabled)?;
    set_zigbee_state(ZigbeeState::Channel5)?;
    Ok(())
}

One thing that I think needs restating is that I am almost always on the side of the maintenance programmer trying to make sense of and fix the code years down the line over the person writing it to begin with - making it quicker to write code is only helpful if it also makes it easy for the maintenance programmer. It's not great if you make the original author's life a little easier at the expense of the reviewer and the maintenance programmers later on - in much the same way as it's not helpful if I use abbreviations only I understand here, just to save me time at the expense of all the people who read this comment.

6 Likes
39

I think there's a question of whether this is a useful question to ask in isolation, and whether it can be answered with existing language features.

For example, I'd also have to look at the function definition to answer it for set_radio_state(Default::default()), or set_radio_state(bar()), or set_radio_state(x.into()).

What is it about set_radio_state(.AllUnblocked)?; -- where I'm using my usual placeholder syntax for this, since I would expect there to be some marker to keep it from conflicting visually with things like struct AllUnblocked; -- that means that one needs to be answer the "what type is it?" question locally (without looking at signatures), when you already can't in other situations that exist in Rust today?

Since Rust has powerful inference, the base case answer is that "what's the type of this?" isn't something that can be answered locally: even something entirely normal like let x = foo(); bar(x) you can't know the type of x without looking up foo or bar. (Even in qux(0) I don't know what the argument type is!)

As a contrived example, if I was really annoyed at typing RadioState:: I can make this work:

fn main() -> Result<(), String> {
    set_radio_state(|x| x.all_unblocked)?;
    Ok(())
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=2c746457a7555a6219e5ba132d00d759

And in that I have no idea what the types are -- inference will let me use that all_unblocked field without me giving any local suggestion whatsoever of what type x is, nor what the type of that field is.

In comparison, set_radio_state(.AllUnblocked)?; feels positively restrained.

4 Likes
40

I see what you're saying, which is that I can obfuscate types even with current Rust, but I'm still not convinced that .AllUnblocked is in any way better than having you write use RadioState::AllUnblocked in cases where that's a commonly needed variant.

And I'd note that your thing is radically different - you've had to change not just your code, but also the shared library code to make it work (or provide your own private wrapper). Given:

#[derive(Debug)]
enum RadioState {
    AllBlocked,
    ZigbeeOnly,
    WifiOnly,
    AllUnblocked
}

fn set_radio_state(state: RadioState) -> Result<(), Error>
    dbg!(state);
    Ok(())
}

I can't see a way to fix your thing to work such that you can supply a closure to set_radio_state that infers the right enum variant for you, without introducing a second function or a wrapper type.

Basically, what I'm saying is that I'm not happy with the idea that we change the code to make it harder for the maintenance programmer just to allow the original developer to avoid writing use RadioState::AllBlocked in their file.

3 Likes