Variable shadowing in same scope / reusable variable names

[nloum]

Some languages allow variable shadowing where the two variables are in the same scope. For example, Rust:

let shadowed_binding = 1;

// This binding *shadows* the previous binding
let shadowed_binding = 2;
println!("shadowed: {}", shadowed_binding);

It would be nice if C# allowed this as well.

Examples

Case 1: Variable re-definition

Console.WriteLine("Please enter your age:");
var age = Console.ReadLine();
var age = int.Parse(age); // Note we're shadowing the string age with an int age

Case 2: out variable

Console.WriteLine("Please enter your age:");
var age = Console.ReadLine();
if(!int.TryParse(age, out var age))
{
  Console.WriteLine("Error: the age was not a whole number")
}
age++;

Case 3: Casting

public void Bark(Animal animal)
{
  var animal = (Dog)animal;
  // the animal variable at this point is known to be a Dog, so it's okay to call Dog-specific methods on it:
  animal.Bark();
}

Case 4: as expressions

public void Bark(Animal animal)
{
  var animal = animal as Dog;
  if (animal is null) return;
  animal.Bark();
}

Case 5: is expressions

Animal animal = GetAnimal();
if (syntax is not Dog animal)
{
    return;
}
animal.Bark();

Examples 5 and possibly 2 require control flow analysis so they might be too hard but the basic idea is that some of these examples would work.

Possible problems

Problem 1: Mutable values

public void StringSplit(char splitChar, List<string> result) { ... }

public IEnumerable<string> GetFoldersToLookForExecutablesIn()
{
    var paths = new List<string>();
    paths.Add(Environment.CurrentDirectory);
    var path = StringSplit(';', Environment.GetEnvironmentVariable("PATH"), out var paths);
    // Uh oh, CurrentDirectory is no longer in list of paths.
    // The point of this problem is that sometimes when you shadow a variable
    // with a new variable of the same type, it might not be obvious that state can be lost
}

Problem 2: Nested out variables

public void GetNumber(out int x)
{
    int.TryParse("a", out x);
    int.TryParse("2", out var x); // I guess this x would not be returned via GetNumber's out parameter? not sure
}

Problem 3: Lambdas

int x = 3;
Task.Delay(1000).ContinueWith(_ => Console.WriteLine(x));
var x = 4;
x++;
// I guess we would print out 3 in this case?

Problem 4: Disposable values

using var output = new FileStream(...);
using var output = new StreamWriter(output);
output.WriteLine("blah blah blah");
// When does the FileStream get disposed of? It would have to be at the end of the scope, not when it is shadowed

[HaloFour]

I personally don't like this behavior in those languages that allow it. I think that names are cheap and it's worth it having separate identifiers for the sake of clarity.

There are some cases where the scope of identifiers does create a nuisance but I'd rather address that by allowing redeclaration where definite assignment does not overlap, which will help in scenarios such as pattern matching with leaky scope.

[nloum]

I guess you could split this idea into two options:

1. The idea where you can have a new value of the variable (cases 1 and 2)
2. The idea where you have the same value but the variable's type is more specific (like in cases 3, 4 and 5)

Example of option 2:

void DoSomething(Animal animal) {
    if (animal is Dog animal) {
        // In every line of code in this function, the animal variable
        // refers to the same exact object, but in this if block
        // the compiler knows the animal is a Dog and therefore we can
        // call dog.Bark here:
        dog.Bark();
    }
    // Outside the above if block, we don't know if the animal variable
    // is a dog, so we can't call dog.Bark() here
}

Is this the kind of semantics you're talking about when you say the following?

I'd rather address that by allowing redeclaration where definite assignment does not overlap, which will help in scenarios such as pattern matching with leaky scope.

The nice thing about option 2 is there is no new syntax and the code is actually less confusing, because now you have only one name pointing to the same value instead of two.

[HaloFour]

@nloum

Is this the kind of semantics you're talking about when you say the following?

No, I was thinking more the following:

object obj = ...;
if (obj is Person { Name: var name }) {
    // ..
}
else if (obj is Business { FirmName: var name }) {
    // ..
}

In this case due to leaky scope the name variable exists outside of the if statement and the declaration in the else block is illegal, however in that case the variable is also not definitely assigned and cannot be used except as the target of an assignment. These are the cases where I think there may be value in allowing the variable to be redeclared and/or to define scope to more narrowly follow definitely assignment, as it does in Java.

[HaloFour]

The nice thing about option 2 is there is no new syntax and the code is actually less confusing, because now you have only one name pointing to the same value instead of two.

I disagree. I think it's much more clear to assign a new name that better describes the narrower type, e.g. dog, as it'd be easy for a minor refactor to end up completely redefining which variable ends up getting used through the rest of the method.

[CyrusNajmabadi]

I'm slightly amenable to work in this space. However, a hard requirement for me would be some sort of syntax that makes it clear that you are intending to shadow a variable. That way accidental collisions keep getting blocked, but the user has a way to express intent here that allows for this.

[smoothdeveloper]

var + #nowarn for a new "on by default warning" is expressing the intent IMO while preserving the defaults of the language.

People who wants more expressive language will disable this particular warning.

It is already possible to shadow unqualified field access, it didn't hurt the language.

[theunrepentantgeek]

This is getting very close to creation of a new language dialect, which has historically been considered in a very negative light.

[theunrepentantgeek]

Eric Lippert had a whole series looking at the reasons behind the current behaviour of C# and how he improved the error messages in the Roslyn compiler.

In one of those he wrote:

... this really does address a real source of bugs. I have myself accidentally-and-not-on-purpose introduced a local variable in one scope that shadows another and it took me literally hours of debugging to discover why it was that what I thought was one variable was suddenly changing values halfway through a function.

Eric doesn't specify which language was in use when he did this - but it may have been C++.

The series is very much worth a read.

• Simple names are not so simple, part one
• Simple names are not so simple, part two
• Confusing errors for a confusing feature, part one
• Confusing errors for a confusing feature, part two
• Confusing errors for a confusing feature, part three

[Richiban]

I have to admit I don't really understand the object that a lot of people have with variable shadowing, but perhaps that's because I don't happen to have been bitten by it in the past.

I, for one, really appreciate being able to have a variable that replaces another in F#, such as:

let m(items: IEnumerable<_>) =
    let items = items.ToList() // I'm doing this so that I can change the type of the existing variable `items`

    ...

[HaloFour]

I'd suggest that it's "less problematic" in functional languages due to variables being immutable by default, so it's not typically possible to confuse shadowing and reassignment. You're much more likely to trigger a compiler error by getting it wrong than to silently introduce a bug.

[CyrusNajmabadi]

but perhaps that's because I don't happen to have been bitten by it in the past.

FWIW, i literally just hit this twice today. The examples were me passing a SourceText text into a method, but hten wanting to do var text = SourceText.From(token.ToString()). had that been allowed, it would have broken things badly. I very much appreciated this being caught so i could smack my head and go: oh right, i already have this here, so let me name both of these values something reasonable now that i have two in scope at the same time.

[jnm2]

The one case where I would use shadowing is when using the original variable name would be incorrect. That could also be satisfied with a mechanism that indicates "error if this variable name is used from this point forward in the current scope."

This could be done with an analyzer:

var list = items.ToList(); // Happy
Make.Unusable(items);
foreach (var item in items) // Error from analyzer on 'items'
{

public static class Make
{
    [Conditional("KEEP_MAKE_UNUSABLE_CALLS")]
    public static void Unusable(object variable) { }
}

[iam3yal]

I'd like to shadow variables if and only if the type is exactly the same. I mostly encounter it inside conditionals and foreach.

[smoothdeveloper]

Coming here to say that at least, in C# you can partially shadow fields (you can still access them with qualifier, so it is not true shadowing in a sense), and that variable shadowing has legit usages (fsharp/fslang-suggestions#1299).

It can always come with "on by default" warnings if the language design, from the team & community perspective, feels more appropriate to the ethos of C#, than enabling it in full.

I think, going this direction, this would also requires reconsideration on ambiguous resolution for symbols and namespaces, to be more graceful when there is no ambiguity based on the type checking.

For example, I happen to have a Span<T> type from way back it started existing in System and it doesn't share any constructor with the one that made it in the BCL. I have to qualify usage everywhere it is constructed, and I don't have a "sticky type alias" feature either, but maybe using global using?

I think it is fine for C# to loosen a bit (with warnings for shadowing, rather than error) and be more conservative in what it disallow, if there is no ambiguity, in general sense.

Upvoting this suggestion.

[DougLeonard]

Just a food for thought, C# doesn't presently even allow this:

        ...
        int x=0;
	for (int i = 0; i < 1; i++) {
		int x = 5;
	  }
	Console.WriteLine (x);
        ...

That of course won't compile, but this, or the named function version will:

        ...
	int x = 0;
	new Action(() => {
	  for (int i = 0; i < 1; i++)	{
		  int x = 5;
		}
	})();
	Console.WriteLine (x);
        ...

...and prints 0 as expected. When the language was created, local functions weren't allowed, so this wasn't an issue, but now the contrast seems funny to me. In these cases, and even the OP, I think the intent to shadow is quite explicit, but a reviewer or unsuspecting editor accustomed to old C# ways may not be on the lookout for it, not even in the case I show. I'm not sure anyone ever would be in the cases the OP shows. Anyway, I would assume that if shadowing were added, it would have to allow for this less controversial example as well.

[HaloFour]

Shadowing wasn't originally permitted in lambdas or local functions either, that was relaxed after the fact: #634

[tacosontitan]

I can't think of a single usage for true variable shadowing that doesn't border on just promoting lazy naming conventions. Every use case you gave could benefit from better names. For cases 1 and 2, the first definition of age isn't the age, it's the user's response, and for cases 3, 4, and 5, you're working with a Dog at that point, so maybe call it a dog for clarity?

From the Wikipedia article you linked:

This outer variable is said to be shadowed by the inner variable, while the inner identifier is said to mask the outer identifier. This can lead to confusion, as it may be unclear which variable subsequent uses of the shadowed variable name refer to, which depends on the name resolution rules of the language.

_{Emphasis mine.}

Why would you want to promote something, that in its very definition, claims to leads to confusion?

[DougLeonard]

Shadowing outer variables is common. C# already allows shadowing of class/object variables and allows local functions to shadow, but not local if/for blocks. But changing variable scope without even a change of block scope does seem to be another matter.

[tacosontitan]

IMHO that's a "just because you can doesn't mean you should" situation.

[DougLeonard]

That's a different point. though Once the ambiguity is possible, good intentions don't make you certain later that you declared a local somewhere above as opposed to forgetting a "this" or an m_ that you normally use. C++ doesn't allow that problem but does allow if/for/whatever-block shadowing, probably because IDE highlighting didn't always exist but for/if blocks are usually small. C# also allows shadowing things from namespaces, which I think can actually be very helpful, and anyway, is allowed.

One limit that is still standing though, I think, is that scope doesn't change within the same block, which is now what the OP wants.