Hacker News new | ask | show | jobs
by alpaca128 1185 days ago
> I’m not sure what the downside is

The downside is that you may get a weird bug and only after a while see that you accidentally overwrote a function parameter and the Rust compiler didn't even warn you about it.

For this reason I always add the following line to my projects to enable warnings:

    #![warn(clippy::shadow_reuse, clippy::shadow_same, clippy::shadow_unrelated)]
You can also use "deny" instead of "warn" to make it an error. I also like "#![deny(unreachable_patterns)]", which detects bugs in enum pattern matching if you accidentally match "Foo" instead of "Type::Foo" - I honestly don't know why this isn't set by default.
3 comments

> The downside is that you may get a weird bug and only after a while see that you accidentally overwrote a function parameter and the Rust compiler didn't even warn you about it.

If you “overwrite” a function parameter without using it, the compiler will warn you of an unused variable.

If you “overwrite” a function parameter because you’re converting it, it’s a major use case of the feature.

> I honestly don't know why this isn't set by default.

Because the author of the match can’t necessarily have that info e.g. if you match on `Result<A, B>` but `B` is an uninhabited type (e.g. Infallible), should the code fail to compile? That would make 95% of the Result API not work in those cases. Any enum manipulating generic types could face that issue.

IIRC it was originally a hard error, and was downgraded because there were several edge cases where compilation failed either on valid code, or on code which was not fixable (for reasons like the above).

> If you “overwrite” a function parameter because you’re converting it, it’s a major use case of the feature.

Or it's unintended and thus a bug. I personally almost never intentionally shadow variables so I turned it into warnings.

> e.g. if you match on `Result<A, B>` but `B` is an uninhabited type (e.g. Infallible), should the code fail to compile?

This specific example you chose is probably the least relevant here, as the Result type doesn't require you to write "Result::Err(_)" instead of just "Err(_)", both will correctly match. Which can of course also be done for custom enums by "importing" their variants ("use EnumName::*;"). But in my experience it's easy to accidentally omit the type in the match pattern and then suddenly it matches everything. I personally can't imagine a situation where this is intentional and have spent way too much time debugging this specific issue, hence I choose to turn it into an error.

> The downside is that you may get a weird bug and only after a while see that you accidentally overwrote a function parameter and the Rust compiler didn't even warn you about it.

It will absolutely warn about this:

    fn foo(i: u32) -> u32 {
        let i = 42;
        i
    }

    fn main() {
        dbg!(foo(42));
    }
results in

    warning: unused variable: `i`
     --> src/main.rs:1:8
      |
    1 | fn foo(i: u32) -> u32 {
      |        ^ help: if this is intentional, prefix it with an underscore: `_i`
      |
      = note: `#[warn(unused_variables)]` on by default
I don't know what the difference was, but 1-2 years ago it definitely did not warn me. Perhaps it doesn't show a warning when you assign a different datatype?
The unused variable warning has been around since before 1.0, and works even when the types of the variables are different.
There's only one case where shadowing has bitten me in the past: long methods with loops dealing usize almost exclusively, where shadowing external bindings inside the loop might make sense, but any mistake would be silent. This was in the context of terminal layout code. The solution there has been extensive testing, but what I should have done is split the megafunction into multiple smaller ones.
> you accidentally overwrote a function parameter

To "accidentally" overwrite it, you have to either:

  a) explicitly mark the parameter binding as mutable: fn foo(mut bar: T)
  b) explicitly re-bind the variable with let (let bar: T = …)