How to avoid accidentally redeclaring a variable inside a code block?


#1

Please forgive me if that’s a newbie question, but let’s imagine I’m modifying a (not so small) method that someone else wrote or that I wrote a couple of years ago and I come across the need to declare a variable. I go ahead and do:

let temp = 2;

Great! Now how can I be sure that I haven’t erased a previous declared variable with the same name?

Do I have to read the entire method, keep track of all the variable names in my head to then be safe that I’m not declaring a variable that was previously declared accidentally?

That feels unsafe to me. I would expect the compiler to tell me: "Hey! This variable is already taken, please choose another name.

How do you manage this situation in a safe way?


#2
  1. Use blocks to scope the new variable to the smallest possible region. This avoids “polluting” the rest of the surrounding code. If the block is too big to see all the pertinent variables at once, make it smaller. If you can’t, consider breaking it out into a function.

  2. Don’t use vague names like “temp”. Name the variable for what it contains. If you have a name collision, that’s a strong indication that the new variable is either redundant, or you’re confusing matters and shouldn’t call it that.

  3. Use clippy if you want to prevent shadowing entirely.

Also, as an aside, shadowing is in no way unsafe by any reasonable definition of safety in Rust.


#3

At the risk of sounding preachy, I would strongly reiterate @DanielKeep’s first point; the fact that rust allows shadowing is clearly no accident, and it is a feature found by many to be extremely desirable in modern languages. Whether it is a danger or a boon largely comes down to coding style, and as long as most variables are declared close to where they are used, you have a fair bit to gain and little to worry about.

The most obvious place where it is useful is in closures; the majority of closures are simple 1-liners that shouldn’t have to worry about context, and there is little reason to call most closure variables anything but x. By using such a name you are making a statement: this variable is really short-lived and you should have no trouble seeing how it is used.

// option 1
test_results.filter(|x| x.iter().all(|&&x| x))
// option 1b
test_results.filter(|t| t.iter().all(|&&c| c))
// option 2
test_results.filter(|test| test.iter().all(|&&case| case))

When I look at #1 I don’t “see” variables named x; I focus on the shape and method names. When I look at #2 my eyes must quickly scan ahead to see whether test is used a second time. So while #2 has its merits, in this respect, I think it is also noisy.

In such a short closure, it is very difficult to accidentally shadow something without noticing, as it is often the difference between |x| center + x and |x| x + x.


Notice that rust even permits shadowing within the same scope, i.e. rebinding.

// without shadowing
let iter = vec.into_iter();
let modified = modify(iter);
let doubled = double(modified);

for x in modified { ... }

//-----------
// with shadowing
let iter = vec.into_iter();
let iter = modify(iter);
let iter = double(iter);

for x in iter { ... }

In the second version, you can easily delete one of the function calls or insert another when the logic needs to be revised.

(edit: the for x in modified was both entirely unintentional and yet quite exemplary!)


Caveat: There is one real danger of shadowing, which is when you rename a variable in an outer scope; this is a circumstance where e.g. |x| y + x can turn into |x| x + x without your immediate knowledge. I would exercise a bit of caution using find and replace, and hope that any more sophisticated tooling for refactoring would catch this.


#5

I hesitate to call this “rebinding” because that implies the original binding is lost, but that’s not true. This is an extremely small semantic nit :smile:


#6

But isn’t it, at least effectively?

To clarify, when I say “binding” I mean a name-value association. Or perhaps what I rather see is a sort of name-memory association (in some model where we pretend that every local lives on the stack (no registers) and assume that separate stack memory is reserved for every let statement in a function). In these terms, I call it a “rebinding” when a new name-memory association replaces an old one in such a manner that permanently hides it; it cannot cannot be recovered at any later point in the function code

Then, under my definition, I say that:

  • Mutation (including reassignment) does not rebind: let mut x = vec![]; x = vec![];
  • Nested scopes may shadow, but they never rebind since the original bindings are recovered when you leave the scope: assert_eq!(0, { let x = 0; { let x = 1; }; x });
  • Individual loop iterations… erm… technically don’t rebind under this definition. (unfortunate, since I would tend to say that for i in xs does create “new bindings” for i each iteration… but then how do I truly define a “binding?” Aaaagh, get outta my head, Socrates!)
  • Successive let bindings do rebind: let x = 0; let x = 0; (the memory slot for the first x can never be accessed again).

#7

In my mind, if the original binding is lost…

let x = 5;

{
    let x = 6;
}

// does x exist here?

Shadowing means that when the shadow goes away, the original binding is still there. Re-binding would mean that x would no longer happen, as it would be unbound and then re-bound.

This behavior is important in Rust, it allows for code like this:

let s: String = ...; // (i have a String from somewhere)
let s = &s[..];

Here, we know s will be valid, because the underlying binding, while not accessible, is still there. So the reference still exists.


#8

As far as I can see, a binding is not a prerequisite for live data:

fn get_a_string() -> String { "I was never here!".to_string() }

fn main() {
    // this method has no bindings to Strings
    let s: &str = &get_a_string()[..];
    println!("{}", s);
}

#9

I am… shocked that this works.


#10

As was I! :slight_smile:


#11

C++ allows this for const&, as discussed in GotW #88, but I had no idea Rust does anything like this. I even tried it with 1.0.0, successfully, so this is nothing new.

It doesn’t have to be “const” like C++ either – &mut get_a_string()[..] still works. So does let s: &str = &get_a_string(); without slicing.

However, get_a_string().as_str() does not work!

error: borrowed value does not live long enough
 --> <anon>:5:42
  |
5 |     let s: &str = get_a_string().as_str();
  |                   --------------         ^ temporary value dropped here while still borrowed
  |                   |
  |                   temporary value created here
6 |     println!("{}", s);
7 | }
  | - temporary value needs to live until here
  |
  = note: consider using a `let` binding to increase its lifetime

#12

@cuviper: Most curious! So indexing is the exception, not the rule…

I found this issue:

pnkfelix has an in-depth comment on this describing the cause. I didn’t read it too closely yet, but it looks like similar semantics happen for all lvalue expressions (*thing, thing.x, thing[..]), and that the difference is embedded in the grammar.

Edit: Perhaps “lvalue expression” isn’t the right term (in the linked discussion, they are referred to as rvalues with “extended lifetimes”)


#13

Thanks for finding that issue! From there, I guess the other one to watch is the tracker for RFC66: