Why shadowing is implicit?

I just realize shadowing variable is implicit. From my humble perspective, it can bring some mess is big functions.

A lot of things are explicit in Rust, is there a particular reason why shadowing is not?

I'm just curious.

Thanks in advance! :smile:

1 Like

You can use clippy and warn on shadowing if you're into that sort of thing. Personally I rather like shadowing and miss it in c#. Shadowing in Rust in more useful than in many other languages because it can be used to switch a local variable to or from a mutable binding without having to pick a new name. E.g.:

{
    let x = ...;
    // do stuff that does not modify x.
    let mut x = x;
    // do stuff that modifies x.
    let x = x;
    // x is immutable once more.
}

Without shadowing you would either have to pick a new name for x several times, or (which I think is more likely) you would simply declare x as mutable for the entire block of code, which is a shame if it is unnecessary.

11 Likes

This most likely comes from Rust's ML heritage, where shadowing is common. I don't think it is too much of a problem, because Rust warns you about unused assignments and type errors. It is also nice for unfreezing variables, or when working with options.

2 Likes

Note that I split the shadow lint into three lints for this exact reason; only shadow_unrelated is warn by default. At least shadow_same is considered completely rusty by many, and I've seen recursive code that would trip shadow_reuse, which looked quite elegant with the rebinding (think let x = x + 1 before recursing).

Also you could create another scope for your let mut x = x outside of which x would be immutable. That way you'd need one less rebinding.

I understand that shadowing is an important part of Rust. But why there is not a syntax to explicitly express the developer intention?
Something like:

let x =3;
shadow x=5;

1 Like

Well, in theory, we could support syntax like this:

let x = 3;
let shadow x = 5;

The reason that both let and shadow would be required is that the LHS of let is a pattern. This would let us write things like

let x = 3;
for (i, shadow x) in iter.enumerate() { ... }

let closure = |shadow x| x + 4;

match foo {
    Some(shadow x) => { ... },
    None => { ... },
}

...but generally speaking, the rust language isn't too big on giving optional syntax to the programmer. We wouldn't want to have shadow unless we also wanted it to become the One True Way (i.e. deprecate shadowing without the shadow keyword). This would cause pain to users like me who really appreciate the ability to shadow, and who feel it is oftentimes the most idiomatic way to write code.

6 Likes

I don't understand, what is the problem of shadowing? Maybe providing an example could help illustrate the issue, if any.

The scenario to a nasty bug is this:
A new developer has to add something to an old code from another developer. He goes in the middle of a function and add something super simple. He is naively 100% sure that it will not break anything:

let x = foo();
println!(x);

He compiles the code and runs it. The new code works as expected.

But the old programer used the "x" variable already for something else. He initialized it at the beggining and used it at the end of the function. Now the "x" is shadowed and at the end has a complete different value.
The new programmer wasn't warned that he is shadowing an "old" variable.
This type of bug is very difficult to find later.
Other languages would warn the programmer, that this name is already taken.

2 Likes

The new programmer is maintaining a program written by someone else. If the new programmer wants to add local debugging variables, then it is the new programmer's responsibility to deconflict potential variable name reuse. The easiest way to do that is for the new programmer to ensure that added debugging checks are local in scope.

{ 
let x = foo();
println!(x);
}
5 Likes

This situation is also likely (but not guaranteed) to produce an "unused binding" warning for the original x.

1 Like

I am with @TomP, here: imho, if time is so much of the essence that the programmer skips looking for function locals up to the point where they are injecting code, then the least they can do is scope their injected code to prevent issues, since shadowing will not be the only issue when injecting unscoped unchecked code.

declaration vs mutation

Moreover, shadowing and (additional) declaration assignment (e.g., let z = x + y;) may prevent mutation assignments (e.g., x = x + y;), which leads to a more readable data flow.

declaration: new binding vs shadowing

Now, regarding let z = x + y; vs let x = x + y;, it obviously depends on what the code is trying to accomplish. But when reading code, if I see the latter I know I can forget about the outer x (readability-wise, the main benefit of imperative mutation), allowing me to offload my mental "local values" representation.

2 Likes

Here is the problem I have with this scenario; x probably lives far longer than it needs to. Every time I use a variable that was defined somewhere above the screen, I cringe. So either x is used in places all over the function (making it impossible for a new developer to not see it), or else something like this calls for:

  • Moving the declaration of x closer to its usage.
  • If it's used once early on and once later on, make two separate declarations in separate block scopes.
  • When all else fails, factor out the code in-between the uses of x into another function, to give the current function the responsibility of maintaining the lifetime of x.
  • If even that doesn't work, give it a long and unusual name that reflects its scope.

To me, every variable in scope at a given point in a function increases the cognitive overhead of maintaining that function, and decreasing that count is one of the major goals of refactoring.

7 Likes

Excellent advice from @expHP that I try to follow. Unfortunately, I still make that mistake, and, as pointed out in this thread, it's a really hard bug to find. A warning would be a big help.

On the other hand (My hobby is arguing against my own point.), with NLL I find myself shadowing variables to avoid borrow problems with self. I would find a warning really annoying in those cases.

Perhaps clippy can report shadowing the way it reports other style violations.

I've been coding in Rust for 2 years now and use shadowing extensively (I can't live without it). I have never had a problem so far.

What I'm saying is that in theory there maybe an issue like above, but in practice it is super rare, making benefits of shadowing outweigh the downsides.

2 Likes

Thank you.
This makes absolute sense to me. But the "old programmer" was somebody else. I cannot influence how he wrote the program two years ago. I don't even know him. I have to add something innocuous to this old code and not insert a new invisible bug.

I will try to use Clippy for that.

The problem I see here has nothing to do with shadowing, and everything to do with a programmer changing a function without even reading it. There are so many other things that could go wrong without even the likelihood of an error or warning that shadowing has that shadowing wouldn't even be near the top of the list -- between deadlocks, unmaintained invariants, etc, there are so many problems that can happen from blindly calling things in arbitrary places.

4 Likes

Shadowing is one of the few parts that feel a bit "icky" to me, coming from C++.

I also don't like to read here the reasoning that "if you have a bug due to shadowing, the fault lies on you for not programming better, rather than on the language for not protecting you from this class of errors". I don't like reading this because to me it is not fundamentally different from "automatic memory management is not useful, you just need to be cautious when performing manual memory management".

Humans make mistakes. Silly mistakes. That's why the proposition of rust is so attractive: it eliminates many classes of programmer mistakes, at compile time, and with zero-overhead.

To me the question in this thread is basically: is shadowing a sufficient ergonomic convenience, and are the problems it could introduce sufficiently rare and small to be worth the cost?

On one hand, I can measure the gain in ergonomics of having shadowing, because I miss it when I'm using languages that don't offer that facility, and the var_type idiom is a sad side-effect of not having it. On the other hand, I'm pretty sure that several times during my career in languages not supporting shadowing, my compilation aborted because I had unknowingly shadowed an existing variable, hence protecting me from a mistake. Granted, I can't remember of a time where it actually caused a bug (C++ has partial shadowing in inner scopes), but maybe it is just waiting to happen (in rust?). I find it odd that rustc is so pedantic about the mutability of a binding (which is also a useful lint in the same category as accidental shadowing), and yet lets the programmer shadow bindings without a (default-enabled) warning nor special syntax.

I feel the ideal way would be to explicitly enable that pattern, for instance using a shadow or rebind additional keyword (maybe something shorter), like was discussed in this thread e.g. in #5.
However, it feels like that ship has sailed for rust, since such a change would either mean coexisting with the current syntax or deprecating the current syntax (à la dyn for trait object), and besides it looks like implicit shadowing is seen as idiomatic rust by many rustaceans, so such a change would maybe not be welcomed by the community?

2 Likes

Why not only allow shadowing of a preexisting variable if the shadowed variable was part of the expression initializing the shadowing variable?
Leaving shadowing patterns (ie:let (rebind x,z)=y;) for the remaining cases of shadowing.

I can't think of any cases where I wanted to shadow a variable in which the shadowed variable was not part of the expression.

2 Likes

Exactly my thoughts! I think the idiomatic use of shadowing is mostly changing the type of the variable. For example:

use std::io;

fn main() {
    let index = String::new();

    io::stdio.read_line(&mut index)
        .expect("Failed to read line");

    let index: usize = index.trim().parse()
        .expect("Invalid number. Please enter an integer");

    // do something with index as an usize

It would be extremely unergonomic to have to use two names: one for the String version you use to get input from the console and another one for the actual usize you use to do arithmetic on.

4 Likes

Thinking further about this: it would be nice if we could have methods returning different data types from console input. I would like to be able to do something like this C++ code in Rust:

#include <iostream>

int main()
{
    // create an integer named index and init to default value (probably 0)
    int index {};

    // Get keyboard input on the console and try to parse it into the type of
    // the right operand (which is an int in this case).
    // If the parsing succeeds, fine, else cin fails and aborts.
    std::cin >> index;

    return 0;
}

Much shorter than the Rust code first creating a string and then parsing that. Probably (I don't know how cin or read_line() works on the nuts and bolts level) would be more efficient in terms of memory usage, too, since input isn't buffered twice, once in the input buffer and then in a string. And faster, since there is one less heap allocation and write to the allocated space.