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

Pattern matching scope #2329

Closed
VBAndCs opened this issue Mar 11, 2019 · 23 comments
Closed

Pattern matching scope #2329

VBAndCs opened this issue Mar 11, 2019 · 23 comments

Comments

@VBAndCs
Copy link

VBAndCs commented Mar 11, 2019

Why pattern matching variable scope goes is the outer scope, while it is the inner scope in other blocks like for loop?

           for (int i = 0; i<10; i++)
            {

            }
            Console.WriteLine(i); // i out of scope
            if ("" is string s)
            {

            }
            Console.WriteLine(s); // s is in scope

This inconsistency is confusing, and makes coding harder in some cases like this:

            if (num.m_value is byte n1)
                return new Number(n1 + value);

            if (num.m_value is sbyte n2)
                return new Number(n2 + value);

            if (num.m_value is short n3)
                return new Number(n3 + value);

            if (num.m_value is ushort n4)
                return new Number(n4 + value);

            if (num.m_value is int n5)
                return new Number(n5 + (long)value);

            if (num.m_value is uint n6)
                return new Number(n6 + (long)value);

            if (num.m_value is long n7)
                return new Number(n7 + (decimal)value);

            if (num.m_value is ulong n8)
                return new Number(n8 + (decimal)value);

            if (num.m_value is float n9)
            {
                var d9 = (double)value;
                if (n9 > float.MaxValue - d9)
                    return new Number(n9 + d9);

                return new Number(n9 + value);
            }

            if (num.m_value is double n10)
                return new Number(n10 + value);

            var n11 = Convert.ToDecimal(num.m_value);
            var d11 = (double)n11;
            if (value > maxDecimal - d11)
                return new Number(d11 + value);

            return new Number(n11 + value);

        }

all n1 to n11 vars are not needed ouside if scope, and it would be faster to copy-paste code directly using the same var name!
So, why did C# take this design decision that is inconstant with other block scopes, and can this be fixed?

@YairHalberstadt
Copy link
Contributor

@VBAndCs
If you would like to ask questions about why C# made certain design decisions, www.gitter.im/dotnet/csharplang is a good place to do that.

@HaloFour
Copy link
Contributor

why did C# take this design decision that is inconstant with other block scopes

It was an explicit design decision based on feedback on using the feature:

dotnet/roslyn#12597

and can this be fixed?

It can't be "changed", that would break existing code.

@HaloFour
Copy link
Contributor

@VBAndCs

In fact I want this to be fixed!

Feel free to fork the repo and build your own compiler.

@DavidArno
Copy link

In fact I want this to be fixed!

You can't have it fixed. The decision was made and that's the way the language now works. Changing it would not only break a lot of code, it would deeply upset many users that want it to work in this "broken" fashion.

@VBAndCs
Copy link
Author

VBAndCs commented Mar 11, 2019

@HaloFour

It was an explicit design decision based on feedback on using the feature

Could you please do a developer survey b4 taking such exotic decisions that breaks the standard expected of millions of developers?

It can't be "changed", that would break existing code.
So what? The few hundreds that may use this exotic scope that way surely need to review there code!
The state of the matched variable outside if scope is unknown and C# gives the error "use of unsigned var"

object o = 1;
if (o is string s)
{

}
Console.WriteLine(s); 

so, there is no code that can be broken, and defiantly this is a bug in defining the scope that should be fixed.

@HaloFour
Copy link
Contributor

@VBAndCs

No, it's not a bug, and you not liking the behavior doesn't make it a bug.

This behavior is not going to change. End of story.

@YairHalberstadt
Copy link
Contributor

YairHalberstadt commented Mar 11, 2019

if (!(o is string s))
   return;
Console.Writeline(s);

There must be thousands upon thousands of lines of code relying on this behaviour. It cannot be 'fixed'.

@HaloFour
Copy link
Contributor

@VBAndCs

Could you please do a developer survey b4 taking such exotic decisions that breaks the standard expected of millions of developers?

No, the LDM makes the decisions. They posted their findings on Github and there was a discussion on the subject, which did result in some minor modifications. Lots of people didn't like it, myself included. But it avoided a lot of additional and unnecessary work to come up with more language features in order to expose those variables in the many cases where the developer would want to.

This repo is not a democracy. The LDM are the ultimate and final arbiters to the evolution of this language.

So what? The few hundreds that may use this exotic scope that way surely need to review there code!

Assuming that the LDM did want to change it (and they don't) it would still break pretty much all code using pattern variables in if statements. The ship has sailed and this behavior will not be changed. End of story.

@VBAndCs
Copy link
Author

VBAndCs commented Mar 11, 2019

@HaloFour

This behavior is not going to change.

As I said, this will not break any significant code if any code at all.

End of story.
Is this a formal response or just your opinion?
Sorry, I don't know if you are a team member or an admin or what?

@HaloFour
Copy link
Contributor

@VBAndCs

As I said, this will not break any significant code if any code at all.

You don't make that determination.

Is this a formal response or just your opinion?

My opinion.
The LDM made their formal response when they implemented the feature.

@VBAndCs
Copy link
Author

VBAndCs commented Mar 11, 2019

@HaloFour

The LDM made their formal response when they implemented the feature.

So, why do we give feedback here?

@YairHalberstadt
Copy link
Contributor

You give feedback before a feature has shipped. Not afterwards.

@HaloFour
Copy link
Contributor

@VBAndCs

So, why do we give feedback here?

The LDM does take feedback into consideration, but this repo is only one (relatively small) avenue for that feedback. It was user feedback here that made the LDM decide to walkback their original decision regarding this scope where it came to while loops as it was demonstrated that would cause unexpected mutability of captured scope.

But once a feature has shipped based on explicit design choices the likelihood that the feature will be changed approaches nil, especially if it will break code written using that feature in a prescribed manner.

I would recommend that if you want to have a discussion on the design process that Gitter would be the appropriate venue.

@VBAndCs
Copy link
Author

VBAndCs commented Mar 11, 2019

@YairHalberstadt
Fair enough. So, is there anyway to notify developers about such unexpected behaviors , so that they can say their opinions? VS news section for example can say: A debate about new feature".
Anyway, a late feedback is not always a bad thing. Consider it as : please don't do this again :) .
I will close this.
thanks.

@VBAndCs VBAndCs closed this as completed Mar 11, 2019
@YairHalberstadt
Copy link
Contributor

@VBAndCs
The LDC post their decisions and proposals here. I don't know of anyway of following this short of subscribing to notifications on this repo.

@VBAndCs
Copy link
Author

VBAndCs commented Mar 11, 2019

Some wraparound for the sake of copy-paste:

            {
                if (num.m_value is byte n)
                    return new Number(n + value);
            }
            {
                if (num.m_value is sbyte n)
                    return new Number(n + value);
            }
            {
                if (num.m_value is short n)
                    return new Number(n + value);
            }
            { 
            if (num.m_value is ushort n)
                return new Number(n + value);
            }
            {
                if (num.m_value is int n)
                return new Number(n + (long)value);
            }
            {
                if (num.m_value is uint n)
                return new Number(n + (long)value);
            }
            {
                if (num.m_value is long n)
                return new Number(n + (decimal)value);
            }
            {
                if (num.m_value is ulong n)
                return new Number(n + (decimal)value);
            }
            {
                if (num.m_value is float n)
                {
                    var d9 = (double)value;
                    if (n > float.MaxValue - d9)
                        return new Number(n + d9);

                    return new Number(n + value);
                }
            }
            {
                if (num.m_value is double n)
                    return new Number(n + value);
             }
             {  
                var n = Convert.ToDecimal(num.m_value);
                var d = (double)n;
                if (value > maxDecimal - d)
                    return new Number(d + value);

                return new Number(n + value);
            }
        }

@CyrusNajmabadi
Copy link
Member

As I said, this will not break any significant code if any code at all.

This is absolutely not true.

If you would like to ask questions about why C# made certain design decisions, www.gitter.im/dotnet/csharplang is a good place to do that.

I would highly recommend following that advice.

@CyrusNajmabadi
Copy link
Member

So, why do we give feedback here?

CSharplang is not a feedback site for features that have shipped and have been in the wild for this long. You can propose changes you'd like to see. However, the proposal should be fully fleshed out, and should address things like the potential for breaking changes.

This repo is not for general gripes, or questions about how things are done. If you want to do either of those, please use gitter.im/dotnet/csharplang.


Note: i was the LDM member who pushed for and got this scoping done as part of the pattern work we did. I have a lot of experience in this area. I can verify that it's practically impossible this would change.

@VBAndCs
Copy link
Author

VBAndCs commented Mar 11, 2019

I was the LDM member who pushed for and got this scoping done as part of the pattern work we did. I have a lot of experience in this area. I can verify that it's practically impossible this would change.

May God forgive you :)
Can you give me the link to the discussion about this decision? I want to be aware of any good usage of this, just in case I need it.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Mar 11, 2019

May God forgive you :)

For what?

I want to be aware of any good usage of this, just in case I need it.

Examples were given in the thread. For example, the "guard pattern":

if (!(foo is string s))
{
     return;
}

// use 's' here.

--

Note: I'm also working on a proposal where you can write the above as:

if !(foo is string s)
{
     return;
}

// use 's' here.

@YairHalberstadt
Copy link
Contributor

@CyrusNajmabadi #882

@VBAndCs
Copy link
Author

VBAndCs commented Mar 12, 2019

if (!(foo is string s))
{
     return;
}

// use 's' here.

Is the twisted way to do this:

if (foo is string s)
{
     // use 's' here.
}

return;

I don't see this as areal need to violate the scoping standards.

@CyrusNajmabadi
Copy link
Member

Is the twisted way to do this:

yes.

I don't see this as areal need to violate the scoping standards.

You just identified the reason about why it was important to be done. There is no violation here as there is no scoping standard for patterns. We got to define whatever it was. By definition it's not a violation because this is the scoping standard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants