Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Local variable shadowing (CS0136) #8016

Closed
orthoxerox opened this issue Jan 18, 2016 · 7 comments
Closed

Local variable shadowing (CS0136) #8016

orthoxerox opened this issue Jan 18, 2016 · 7 comments
Labels
Area-Language Design Discussion Resolution-Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on.

Comments

@orthoxerox
Copy link
Contributor

Why not allow us to declare a local variable with the same name as a local variable in an enclosing block? For example,

        public static void Main(string[] args)
        {
            var i = 1;
            {
                var i = i++;
                //i==1
                {
                    var i = "foo";
                    //i=="foo"
                }
                //i==1
            }
            //i==2
        }

would declare three separate variables. Perhaps they should be marked with an additional modifier to prevent accidental shadowing, like methods are?

        public static void Main(string[] args)
        {
            var i = 1;
            {
                new var i = i++;
                //i==1
                {
                    new var i = "foo";
                    //i=="foo"
                }
                //i==1
            }
            //i==2
        }
@HaloFour
Copy link

@orthoxerox Because it was decided during the design of C# 1.0 that said behavior is confusing and the cause of unexpected logic errors do to setting the wrong variable? I think it was a good idea then and you'll probably need a fantastic argument to demonstrate why the same potential problems don't still exist today.

@alrz
Copy link
Contributor

alrz commented Jan 18, 2016

As long as it's C# it's no good, for example in Rust it's rather useful (to change mutable binding to immutable etc) although you can make it a warning, still, shadow_unrelated is on by default, which is not currently applicable in C#, because we don't have such analysis and it just doesn't make sense as long as we don't have any ownership notation — for example in let foo = foo.uwrap(); the first variable becomes unusable. See #161 comment.

@ghost
Copy link

ghost commented Jan 18, 2016

It would get complicated, mayge you could use some kind of keyword to go up a level, like the this keyboard when doing it with local vs instance variables?

public class Thing
{
    string Name;

    public Thing(string Name)
    {
        this.Name = Name;
    }
}

@HaloFour
Copy link

@dan2002s And have, what, a parameter to specify the number of levels "up" you want to go? That sounds unnecessarily complicated, all to avoid having to think of a different identifier.

@bondsbw
Copy link

bondsbw commented Jan 19, 2016

I would prefer the exact opposite, that C# get rid of the ability to have overlapping names (such as using the same name for class member and a local variable or parameter). Unfortunately it would break BC to make that change now.

@gafter gafter added Resolution-Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on. Discussion labels Jan 26, 2016
@Soulzityr
Copy link

Soulzityr commented Jul 23, 2016

I was assigned this ticket through codetriage. I believe that this should not be implemented. Can we close this issue as a won't fix?

@Pzixel
Copy link

Pzixel commented Apr 27, 2020

With introduction of pattern matching this feature would be really great. I hate I have to write code like

var maybeSomething = foo?.Bar?.Baz;

if (maybeSomething is {} something1) { ... }
... // in multiple cases branches cannot be merged with single check
if (maybeSomething is {} something2) { ... }
...
if (maybeSomething is {} something3) { ... }
...

Is this resolution final? It seems like introducing variable shadowing shouldn't be a breaking change since it relaxes rules rather than the other way around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Language Design Discussion Resolution-Won't Fix A real bug, but Triage feels that the issue is not impactful enough to spend time on.
Projects
None yet
Development

No branches or pull requests

8 participants