Locals and parameter shadowing in local functions

[jnm2]

I'm running into a need to name a local function parameter the same as a local from the containing method. Shadowing local and parameter names from the enclosing function should be just as possible as shadowing class field names.

It's more convenient than having to make up new names when there is legitimately nothing to add to the name in that context, especially for those times when you want to explicitly pass locals into the local function rather than magically referencing them via a closure.

It's also an amazing safeguard preventing you from accidentally referring to the outer local instead of the local function parameter, which can change the meaning of the program.

[svick]

Previous discussion about this: dotnet/roslyn#15793. It was closed by @gafter:

The LDM […] are concerned that permitting this may make it too easy to introduce bugs, so we do not intend to move forward with this as a championed proposal. The original motivaring example can be [re]written [in a way that] does not cause any additional allocation compared to the requested feature.

[jnm2]

Thanks @svick.

[alrz]

This is revised for C# 8.0. (notes)

[agocke]

Yup, this is officially back on the table. I'll champion it.

[CyrusNajmabadi]

Small piece of feedback. this was one of the rules we had to turn off in our TypeScript linter because it just blocked such a useful coding pattern. i.e. you'd have:

const users = getUsers();
const prices = getPrices();
await Promise.all([users, prices]).then(([users, prices]) => {
    // ...
});

Here we want the inner lambda (which contains a deconstruction), to just be able to reuse those names inside the body. And, we actually do want the inner lambda to hide the outer values. Having to name these things differently is actually just annoying, and unhelpful. i.e.

const usersPromise = getUsers();
const pricesPromise = getPrices();
await Promise.all([usersPromise, pricesPromise]).then(([users, prices]) => {
    // ...
});

it's just so unweildy and non-idiomatic.

--

So, tldr, i def support this because i actually think it's a reasonable and sensible coding pattern to want to hide the locals (to prevent captures) and to want to name the parameters the exact same as the passed in variables.

[CyrusNajmabadi]

Note: i know the above is aobut local functions, not lambdas. But the same arguments applied for us for local functions as well. So definitely 👍 on this.

[alrz]

i know the above is about local functions, not lambdas.

This has been considered under static local functions but notes state that if it gets implemented it should be applied to lambdas as well as local functions whether it's static or not, precisely because the inconsistency doesn't do any good.

[333fred]

@alrz the notes don't seem to say this, but we definitely came to the consensus in ldm that this would apply to all local functions, and lambdas would be a potential later consideration (fyi @agocke).

[alrz]

Ah right, thanks for clarifying.

[dancojocaru2000]

Is this a dead suggestion?

I would really love this feature. I've used it in Rust and it's just pure awesome.

An example for why this would be very useful is this one:

someone.UpdateReceived += (sender, update) => {
    if (sender == null) return;
    CustomSomeone sender = (CustomSomeone)sender; // No longer need to cast everytime sender is used
    sender.MemberOfCustomSomeone = ...
}

[alrz]

Just to mention, in Rust that is extremely useful because of ownership - you may want to redeclare a variable with a different lifetime to capture in a closure, for example. In C# you don't need to do any of that.

[dancojocaru2000]

Go also mostly allows redeclaring variables, so lifetimes aren't the only reason. Fair, in Go functions return errors and it's useful to reassign the returned error to the same variable, but this goes to show that there are multiple uses.

[smoothdeveloper]

It can be used to explicitly shadow a binding you don't want remaining of the code to have access to. It can be used implicitly to avoid coming up with separate slight variations of a name which can lead to writing bugs if you pick the wrong one in a list of alternates. It can be used to change a mutable variable into a readonly one.

It reduces the scope of a binding, (achievable through shadowing or anonymous scope), which is generally good.

I think those are some reasons why F# allows this type of shadowing.

[agocke]

@dancojocaru2000 The specific proposal in C# 8.0 was to allow variables inside local functions and lambdas to shadow the parent scope. Your example is a local variable shadowing a parameter. That was not proposed.

Given that your example could have been legal in C# 1.0 and it was chosen to be an error, I don't think there's any appetite for allowing shadowing of parameters via locals.

[CyrusNajmabadi]

I would be fine with an explicit way to do this. I.e.

shadows var sender = ...

Then it would be clear that this was the intent, and there wasn't accidental shadowing going on.

I almost accidentally shadow a variable several times a month, so I very much appreciate the default rules c# has here. Though I recognize the value in making it possible to do this.

[dancojocaru2000]

shadows var sender = ... would be so awesome!

Here's another usecase that would be really awesome:

var code = Console.ReadLine();
if (code == null) {
    // handle EOF
    return;
}
if(!int.TryParse(code, out shadows var code)) {
    // handle not an int
    return;
}

// code is now an int variable

The initial string code was only needed in order to be converted from the raw input to the actual code, and handle the failure otherwise. While currently, naming that variable something like codeInput would be possible, I've had situations when I would pass that variable that I no longer need as parameter to a method instead of the new one (for example when a method had multiple overloads).

A possible alternative would be a way to make a variable explicitly go out of scope before the scope actually ends. After that, a new one with the same name could be declared, since the old one no longer is in scope.

And even if reusing a name isn't allowed, telling the compiler that I finished what I had to do with a variable could prevent some errors.