Add compiler warning for matching unit structs?

1

Someone recently pointed out this confusing code to me. It seems like there should be a warning for this. It is very confusing and I can't imagine when you'd ever intend to do this.

The case is when you have a unit struct S then it can be matched by the pattern S in let, if let or while let. It makes for very is confusing code.

Rust Playground

The code below prints DFDFDD. Extra instances of S are created and it isn't obvious that is what the code is doing. Specifically, the let statement does a match instead of an assignment so that value is dropped before the call to println!.

use std::fmt::{self, Display};

struct S;

impl Display for S {
    fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
        formatter.write_str("F")
    }
}

impl Drop for S {
    fn drop(&mut self) {
        print!("D");
    }
}

fn f() -> S {
    S
}

fn main() {
    let S = f(); // Confusing match instead of assigment
    print!("{}", S);
    
    match S
    {
        S => print!("{}", S)  // Confusing match against value of same type
    }
}

I would propose that two warnings be added. The first one seems more important and more likely to be done accidentally.

  1. Warning when matching unit structs in let, if let, and while let using the bare struct name. If the match is intended, it should be matched with S{} which is already legal Rust code.
  2. Warning when matching unit structs in match using the bare struct name if the type of the expression being matched is the struct type. Again, S{} could be used if the match is intended.

Am I missing some reason these should not be warnings?

Would adding these warnings require an RFC or can I create an issue in the compiler repo? (My reading of the guidelines seems to say an RFC is needed. That is more effort than I am willing to put in.)

3 Likes
2

I kinda see what you're getting at, but i'd like to point out that there's already a warning triggered by using S as a variable name:

warning: variable `S` should have a snake case name such as `s`

Or alternatively, if you called your struct definition lower-case s, you'd get

warning: struct is never constructed: `s`

So, when using the normal naming conventions, it's not possible to confuse a type (or enum variant) with a variable.

If you really want an automated check for this kind of thing, maybe also look into writing a lint for clippy?

3

Those warnings don't trigger for the code above. The compiler doesn't see that as a variable named S.

4

FWIW, the only reason this has any semantic difference is because of Drop. Otherwise the "extra" instances of this type waste nothing, since it is zero-sized.

1 Like
5

That does look like a bug to me. The line let S = f(); should trigger the warning, but it indeed doesn't in the playground link you provided.

6

S is a pattern there.

1 Like