[nightly] Unexpected shadowing behavior of the `match` expression

1

I’ve encountered an very weird issue that concerns the shadowing behavior of the match expression, but I’m not sure if it’s intended or not. I’m using rustc 1.23.0-nightly (fa26421f5 2017-11-15).

First off, I’m using Diesel and it is in that context that I’ve encountered this behavior, and thus the code sample reflects this. However I do not believe that crate to be the direct cause (i.e. this could happen in any crate, in principle).

As for the issue itself, consider this block of code:

{
    let result: Result<Vec<QueriedMsg>, DieselError> = db.query_all();
    match result {
        Ok(messages) => {
            let messages: Vec<QueriedMsg> = messages;
            // More code
        },
        Err(diesel_err) => {
            println!("Error querying the DB: {:#?}", diesel_err);
        },
    }
}

Consider the Ok(messages) case. As-is, the code will compile fine. However, originally messages was msgs, which causes the following compile error:

error[E0308]: mismatched types
   --> src/bin/amplify/vis_server.rs:100:28
    |
100 |                         Ok(msgs) => {
    |                            ^^^^ expected struct `std::vec::Vec`, found struct `amplify::database::schema::msgs::table`
    |
    = note: expected type `std::vec::Vec<amplify::database::QueriedMsg>`
               found type `amplify::database::schema::msgs::table`

error[E0308]: mismatched types
   --> src/bin/amplify/vis_server.rs:101:57
    |
101 |                             let msgs: Vec<QueriedMsg> = msgs;
    |                                                         ^^^^ expected struct `std::vec::Vec`, found struct `amplify::database::schema::msgs::table`
    |
    = note: expected type `std::vec::Vec<amplify::database::QueriedMsg>`
               found type `amplify::database::schema::msgs::table`

error[E0308]: mismatched types
   --> src/bin/amplify/vis_server.rs:101:33
    |
101 |                             let msgs: Vec<QueriedMsg> = msgs;
    |                                 ^^^^ expected struct `std::vec::Vec`, found struct `amplify::database::schema::msgs::table`
    |
    = note: expected type `std::vec::Vec<amplify::database::QueriedMsg>`
               found type `amplify::database::schema::msgs::table`

error: aborting due to 3 previous errors

error: Could not compile `amplify-core`.

To learn more, run the command again with --verbose.

Cargo-Process exited abnormally with code 101 at Thu Nov 16 16:35:13

Here’s the odd part: while the named “type” exists, it’s in a module that I don’t explicitly use anywhere in the module i.e. in amplify::database::schema::msgs. I do have a glob import:

use amplify::database::schema::msgs::dsl::*;

However, even if that imports something that could potentially collide, I expect local name bindings in general to bluntly shadow the other identifier and thus not cause a collision. However, that seems to be exactly what’s happening there. Could anybody help clear this behavior up?

2

Have you got a

const msg: table = ...

anywhere? It seems to be matching that.

3

I don’t do that explicitly, but it’s possible that Diesel does this as part of its code generation. Either way, why doesn’t the local lexical binding msg shadow the less-local one? That’s the behavior I expect to be honest, and I don’t really see why it should matter if the thing being shadowed refers to a module or a value.

4 
match my_option {
    None => {}
    _ => {}
}

If things we shadow didn't matter then None would be interpreted as a new local variable shadowing None from Option. And in general, single-identifier patterns like None would be impossible.

1 Like
5

The thing is msgs has the exact same “priority” here as Ok or Err. All three are identifiers that are brought into scope outside of your code - the only difference is that you intended to use Ok and Err, and didn’t intend to use msgs as an identifier. This is similar to what petrochenkov said.

I’d recommend either removing the import which brings msgs into your scope if possible, or choosing a different identifier.

6

I ultimately did the latter. It just struck me as extremely weird that all else equal, it matters which name I give to a local binding, and on the face of it not something that should be a part of the Rust language.

Now I at least understand that it’s not a bug, as well as the reason for the status quo :slight_smile:

7

Cool!

Yeah, it is a pain point, and an interesting problem. The tradeoff of being able to specify things like Some and None directly, or being able to use all identifiers clearly.

I remember when all code had to be like this:

match x {
   Option::Some(a) => ...,
   Option::None => ...,
}

I think in my codebases I mostly address this by just never using ::* imports. I usually use the mod_name::Struct anyways, so I just import modules rather than anything else. That has its advantages and disadvantages, but one advantage is that without ::*, you always know exactly what’s in the scope for you to avoid using.

2 Likes
8

Since the naming schemes of types, constants and bindings are completely disjunctive, this should normally not be a problem. Rust already lints against const foo: T and suggests const FOO: T

1 Like
9

Indeed. I think this is why it’s in diesel code specifically that this issue hit me: it generates code that diverges from those naming standards. Having used it, I can understand why the diesel developers did it*, but I think I would still have made a different tradeoff in favor of the regular naming conventions (i.e. use just use CamelCase for table names, assuming it matters). Especially since to ensure there are no naming collisions, I’m now writing out the full namespace for every table and column name, which is rather verbose.

*They did it because it results in code that is closer to what you would normally write in various SQL dialects, especially w.r.t. table and column names.

1 Like
10

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.