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

Proposal: omit 'var' from nested 'var' patterns inside positional patterns #1052

Closed
orthoxerox opened this issue Oct 27, 2017 · 20 comments
Closed

Comments

@orthoxerox
Copy link

At the very least it should be allowed inside let statements, since this will match the style of tuple deconstruction:

var (x1, y1) = p;
let Point(x2, y2) = p;

var (address,) = email; //oneples do not yet exist
let Email(address) = email;

var would look superfluous:

let Point(var x2, var y2) = p;
let Email(var address) = email;

Potential drawbacks:

Typos in positional patterns will become new variables and not errors.

@VisualMelon
Copy link

VisualMelon commented Oct 29, 2017

Typos in positional patterns becoming new variables not errors strikes me a very grave (and definite) drawback, which would present unfamiliar behaviour which would undoubtedly lead to hard-to-find bugs. The static checker is there to help us, and it can't help us if we don't tell it what we want to do, and we can't do that if what we type can be ambiguous.

C# is a relatively good language with an old and relatively helpful static checker, and the proposed would be a surprise to anyone that stumbles (perhaps in fit of rage at their compiler) across the behaviour when they 'make a typo' and assume they haven't because they know that the C# compiler isn't so useless as to let them get away with assigning an undeclared variable.

A further implication of this drawback is that it can result in the let statements changing existing variables without it being obvious:

void MyMethod(string name)
// snip
let Thing(name) = h; // oops, just overwrote my parameters!
// snip
Console.Writeline(name);

I would prefer that (if my understanding of explicit tuple deconstruction is correct (I've yet to use it in real code, and honestly don't anticipate doing so)) something more similar to the var syntax of tuple deconstruction, where if you var the whole tuple, then all the parameters become new variables, and trying to reuse a variable is an error.

var (a, b) = (1, 2);
var (b, c) = (1, 2); // error: CS0128 A local variable or function named 'b' is already defined in this scope

Personally I deplore this syntax as well (just personal taste), but it is unambiguous, so you have to make 2 errors to not notice you've shot yourself in the foot, and the behaviour can't be changed by any code above it. There is a lot of sense (I think) in requiring that the deconstructed 'variables' are new variables/readonly-bindings, though it isn't necessarily the right thing in the context of C#.

@gafter
Copy link
Member

gafter commented Oct 29, 2017

We're not planning to have a let statement, so the argument made in this proposal no longer makes much sense.

@DavidArno
Copy link

@gafter,

Will the reasons for that (rather depressing) decision by the LDM ever be shared with the community, or is your statement here the extent of the communication from the team?

@gafter
Copy link
Member

gafter commented Oct 30, 2017

@DavidArno It was discussed in the context of the scoping rules for pattern variables, and (our consideration of the related guard statement) is part of the reason we are considering an if !() statement. Having said that, the fact that it is not currently in our plans doesn't mean it never could/will be, if let/guard proves a significant improvement over the alternatives we would probably consider it.

@gafter
Copy link
Member

gafter commented Oct 30, 2017

As for the specific examples:

var (x1, y1) = p;
let Point(x2, y2) = p;

that's a good example of why the let statement isn't needed.

var (address,) = email; //oneples do not yet exist
let Email(address) = email;

this can be expressed

var address = email.Address;

or, if you're a bit of a masochist,

email.Deconstruct(out var address);
let Point(var x2, var y2) = p;

this can be written

var (x2, y2) = p;

@DavidArno
Copy link

DavidArno commented Oct 30, 2017

@gafter,

Your examples aren't equivalent though, eg

var (x1, y1) = p;
let Point(x2, y2) = p;

creates x1 and y1, which are mutable variables and x2 and y2, which are immutable. One of the key points of let was that variables introduced via the pattern would be read-only. Removing plans for let kills that idea.

Further, using an example domain object from a post by Mark Seeman, by having let, the current use of a negation and "leaky" out var:

if (!UserName.TryParse(name, out var username)) throw new SomeException("Not a valid name);

// use "leaked" username here

could be replaced with the far more elegant:

let Username(username) = name else throw new SomeException("Not a valid name);
// use username here

Of course the "leaky out var" boat has sailed, but "we already have a clunky way of doing things" seems a pretty poor excuse for not offering an elegant way of doing the same thing.

@DavidArno
Copy link

@VisualMelon,

Regarding:

void MyMethod(string name)
    // snip
    let Thing(name) = h; // oops, just overwrote my parameters!
    // snip
    Console.Writeline(name);

My understanding of how let would have worked is that let Thing(name) = h; would be a syntax error as name already exists and cannot be overwritten by a let expression. However, without let, but with positional patterns, you'll still be able to do:

void MyMethod(string name)
     // snip
    if (h is Thing(name)) // oops, just overwrote my parameters, because I forget "var"
    // snip
    Console.Writeline(name);

@alrz
Copy link
Member

alrz commented Oct 30, 2017

So to be clear on #1052 (comment) above, if I understand it correctly, you won't need to repeat var for each variable if you also omit the type name,

// if the target type is known to be a Point,
case Point(var x, var y):
// would be equivalent to
case var (x, y):

However, for single out deconstructions, if we require type name to disambiguate ( #1054 (comment)) you still won't need to repeat var, because there is only one!

case C(var celsius): ...

@VisualMelon
Copy link

VisualMelon commented Oct 30, 2017

@DavidArno perhaps I have misunderstood; I was under the impression (as implied by the 'potential drawback') that predefined variables would be overwritten, or error if they were declared in the let with var. I think what you have described is similar to how tuple decomposition already works, and if that is the case, then there would indeed have been no ambiguity possible with let (though one could still argue that having more than one way to do anything is bad (as I think is suggested in this proposal, but re-reading it again I am not so sure), and invites ambiguity in the future, as I certainly would).

I have not read too deeply into the positional patterns proposals (not a huge fan, and it's a big topic), but if they act as you describe, then I don't see any great issue (so long as the syntax and semantics are consistent between 'pattern' features), as the developer has to make 2 'errors' to confuse themselves: they have to omit var and they have to collide the names. More importantly, the lack of var is unambiguous, and can be reviewed in-place. The important thing is that the common mistake of making 1 'error' (omitting var, or mis-spelling an existing variable name) will give the compiler cause to shout at the programmer.

Having anything 'optional' will always inflict misery upon someone, but if there is only one behaviour then there is no concrete issue at present. In the context of let, if there is only one behaviour (always create a new variable; always error if it already exists), then the choice of whether to include var or not every time (and enforce it) should depend on how the rest of the language 'feels', which I shan't comment on now.

Addendum: having just reread the original post and confused myself, I shall quickly lay out my understanding of the 'drawback': (when attempting to overwrite an existing variable) 'typos in positional patterns will become new variables and not errors.' Perhaps the OP misunderstood the proposal if 'always create a new variable' is the intended behaviour? (Or more likely I did)

@HaloFour
Copy link
Contributor

I was always under the assumption that when the design of pattern variables changed to mutable and leaky that the necessity of the let statement largely evaporated. All of the behaviors it offered would already be covered by existing positional deconstruction or through recursive patterns in is expressions. Granted, the latter does require wrapping in something like an if statement but forcing the developer to deal with the potential of fallibility is probably not a bad thing. The pattern variables would not be readonly but I don't think that behavior was coming from the let statement but rather from the design of pattern variables at that time.

@gafter
Copy link
Member

gafter commented Oct 30, 2017

A simple name in a pattern will be interpreted as a reference to a constant declaration (which is taken as the value of a constant pattern). It is an error if no accessible constant declaration is found. This is true no matter where patterns are used in the language.

using static System.Int32;

switch (i)
{
    case MaxValue: ...
    case MinValue: ...
}

@DavidArno
Copy link

@HaloFour,

I think you are right. If I can write,

if !(name is Username(var username)) throw new SomeException("Not a valid name);
// use "leaked" username here

Wanting to write it as:

let Username(username) = name else throw new SomeException("Not a valid name);
// use username here

really just becomes a case of not wanting to use the "leaky" vars feature. Clearly that feature is here to stay and we lost the argument on it being a bad choice. So it would take a really compelling argument to revive pattern matching via let.

@gafter
Copy link
Member

gafter commented Oct 30, 2017

@HaloFour @DavidArno You're both on the right track. I would add that I disagree with @HaloFour 's assertion

Granted, [existing positional deconstruction or through recursive patterns in is expressions] does require wrapping in something like an if statement but forcing the developer to deal with the potential of fallibility is probably not a bad thing.

The previous let proposal required the programmer to deal with failure if there was the possibility of failure. However, the status quo does not require handling some kinds of failure

Point p = null;
var (x, y) = p; // NullReferenceException

However, I believe the status quo allows you to avoid "handling failure" for anything you could have done with a single irrefutable pattern and a let.

@HaloFour
Copy link
Contributor

@gafter

I was referring specifically to the inability to use is as a statement.

That's a good point about the potential fallibility of deconstruction, though. Although it's seemingly only a problem if you deconstruct a reference type and any of the types I've done that with so far have been value types, e.g. tuples, KeyValuePair. Hopefully non-nullable reference types will help catch the potential NRE vector, and I seriously hope that nobody is throw exceptions from within Deconstruct for lack of recursive positional patterns.

@gafter
Copy link
Member

gafter commented Mar 1, 2018

We are very unlikely to add the let pattern-matching statement. That was the point of our current scoping rules.

A pattern Point(x, y) under this proposal would be ambiguous in its meaning, as either x or y or both could be the name of constants defined in an enclosing scope. So that's not good.

If the type of the matched input is Point, you'll be able to write var (x, y) as a shorthand (being added in C# 8). Otherwise Point(var x, var y).

@orthoxerox
Copy link
Author

var (x, y) is great. Will var (x) work too?

@gafter
Copy link
Member

gafter commented Mar 1, 2018

Yes

@orthoxerox
Copy link
Author

Nice, thank you.

@DavidArno
Copy link

@orthoxerox, why do you want var (x)? What would it do differently to var x?

@orthoxerox
Copy link
Author

orthoxerox commented Mar 2, 2018

@DavidArno I expect it to deconstruct the rhs into x. I still want to be able to deconstruct right there in the method header, but that was another issue.

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

No branches or pull requests

6 participants