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

C# Design Notes for Jan 28, 2015 #180

Closed
MadsTorgersen opened this issue Jan 30, 2015 · 38 comments
Closed

C# Design Notes for Jan 28, 2015 #180

MadsTorgersen opened this issue Jan 30, 2015 · 38 comments

Comments

@MadsTorgersen
Copy link
Contributor

C# Design Meeting Notes for Jan 28, 2015

Quote of the day:

It's not turtles all the way down, it's frogs. :-)

Agenda

  1. Immutable types
  2. Safe fixed-size buffers
  3. Pattern matching
  4. Records

See also Language features currently under consideration by the language design group.
#1. Immutable types

In research prototypes we've experimented with an immutable modifier on types, indicating that objects of the type are deeply immutable - they recursively do not point to mutable fields. Issue #159 describes the proposal in more detail.

How do we construct types which once fully constructed can't be changed?

  • all fields are readonly, and recursively have immutable types
  • can only inherit from other immutable types (or object)
  • the constructor can't use "this" other than to access fields

unsafe or some other notation could be used to escape scrutiny, in order to create "observable immutability" while cheating under the hood (typically for performance reasons). You could factor such unsafeness into a few types, e.g. Lazy<T>.

The feature is designed to work with generics. There would be a new constraint where T: immutable. However don't want to bifurcate on Tuple<T> vs ImmutableTuple<T> just based on whether the content type is constrained to immutable.

Instead an immutable Tuple<T> would instantiate to immutable types only if type arguments are all immutable. So Tuple<int> would be immutable, Tuple<StringBuilder> wouldn't be. Immutable generic types would allow type parameters in fields, because that still maintains the recursive guarantee.

Immutable interfaces are also part of the proposal. Somewhat strangely an immutable interface can only be implemented by types that pass all their non-immutable type parameters to the interface!

What's the value? It's mostly a tool for the compiler to help you ensure you are following your intent of writing deeply immutable types.

Why is that a valuable property to ensure in objects?

  • it would allow safe parallel operations over them (modulo holes, see below)
  • it would allow burning an object graph into the DLL by compile time evaluation of static initializers
  • they can be passed to others without defensive copying

Immutable delegates are ones that can only bind to methods on immutable types. At the language level, that means closures would need to be generated as immutable when possible - which it won't often be, unless we adopt readonly parameters and locals (#98, #115).

As for choice of keyword: readonly indicates "shallow", that's why immutable may be a better word.

Given the restrictions, you'd expect that any method call on an immutable type would have side effects only on data that was passed in to the method - so a parameterless method (or one taking only immutable parameters) would essentially be pure.

Unfortunately that is not quite true. This expectation can be undermined by two things (other than the built-in facility for cheating): mutable static fields and reflection. We can probably live with reflection, that's already a way to undermine so many other language level expectations! However, mutable statics are unfortunate, not just for this scenario but in general. It would be a breaking change to start disallowing them, of course, bu they could be prevented with a Roslyn analyzer.

Even then, while not having side effects, calling such a method twice with the same arguments might not yield the same result: even returning a new object isn't idempotent.

Given the holes and gotchas, the question is whether it is still valuable enough to have this feature? If it's not a full guarantee but mostly a help to not make mistakes, maybe we should do this through attributes and analyzers? The problem with analyzers is that you can't rely on other folks to run them on their code that you depend on. It wouldn't e.g. prevent defensive copies.

In our research project, this turned out to be very valuable in detecting bugs and missed optimization opportunities.

The object freezing could be done without the feature just by carefully analyzing static fields. But the feature might better help people structure things to be ripe for it.

IDE tooling benefits: Extract method would not need to grab all structs by ref.

We would have to consider making it impossible to implement an immutable interface even via the old compiler. Otherwise there's a hole in the system. Something with "modrec"?

If we added a bunch of features that introduce readonly objects to C# 7 (like records, tuples, ...) and then add this feature later, would we end up being in trouble? Only if we violated the rules we would end up applying.

Marking an existing unsealed type as immutable would be a breaking change. If we introduce such classes in C# 7, it would be breaking to make them immutable later.

As a case study, has Roslyn suffered from the lack of this feature? There have been race conditions, but those would still have happened. Is Roslyn not a great example?

Probably not. Roslyn is not immutable. It's presenting an immutable view, but is mutable inside. Would that be the common case, though?

Some of the "cheating" in Roslyn (caching, free lists, etc) is for performance, some is for representing cycles. Insofar as the immutable types feature is also for performance, it seems that thhere's a tension between using it or not.

In summary, we are unsure of the value. Let's talk more.
#2. Safe Fixed-Size buffers

Why are fixed-size buffers unsafe? We could generate safe code for them - at least when not in unsafe regions. Proposal described at #126.

It might generate a lot of code. You could do it with different syntax, or switch on how you generate.

This is a very constrained scenario. It wouldn't be harmful, and a few people would celebrate, but is it worth our effort?

It would allow arbitrary types, not just primitive like today. That may be the bigger value.

Not a high-pri, not one of the first we should commit to.
#3. Pattern matching

A view on pattern matching:

A pattern is not an expression, but a separate construct. It can be recursive.
It's idempotent, doesn't affect the state of the program or "compute" something.

Sktech of possible pattern syntax:

pattern:
*
literal
var identifier
type identifieropt
type { member is pattern ... } identifieropt

expression:
expression is pattern

switch-label:
case pattern :

Actually we'd separate into simple and complex patterns and only allow some at the top level.

We'd have to think carefully about semantics to make it fit into existing is expressions and switch statements. Alternatively we'd come up with a new kind of switch statement. The syntax of switch is already among the least appealing parts of C# - maybe time for a revamp anyway?

Additionally we could imagine a switch expression, e.g. of the form:

match (e) { pattern => expression; ... ; default => expression }

This would result in a value, so it would have to be complete - exactly one branch would have to be taken. Maybe an expression would be allowed to throw. In fact, throw could be made an expression.

Expanded is operator

Here's an example scenario from the Roslyn framework. This should not be taken to say that this is a feature specific to building compilers, though! First, without pattern matching:

var e = s as ExpressionStatement;
if (e != null) {
    var a = e.Expr as AssignmentExpressionSyntax;
    if (a != null) {
        var l = a.Left as IdentifierName;
        var r = a.RIght as IdentifierName;
        if (l != null && r != null & l.Name.name == r.Name.name) ...

Ugh! With just the e is T x non-recursive pattern match we can do a lot better, handling everything in a single if condition:

if (s is ExpressionStatement e &&
    e.Expr is AssignmentExpressionSyntax a &&
    a.Left is IdentifierName l &&
    a.Right is IdentifierName r &&
    l.Name.name == r.Name.name) ...

Much more readable! The explicit null checks and the nesting are gone, and everything still has sensible names.

The test digs pretty deep inside the structure. Here's what if would look like with recursive T { ... } patterns:

if (s is ExpressionStatement {
        Expr is AssignmentExpressionSyntax {
            Left is IdentifierName { Name is val l },
            Right is IdentifierName { Name is val r } } }
    && l.name = r.name) ...

Here the pattern match sort of matches the structure of the object itself, nesting patterns for nested objects. It is not immediately obvious which is more readable, though - at least we disagree enthusiastically on the design team about that.

It is possible that the nesting approach has more things going for it, though:

  • patterns might not just be type tests - we could embrace "active patterns" that are user defined
  • we could do optimized code gen here, since evaluation order might not necessarily be left to right.
  • the code becomes more readable because the structure of the code matches the shape of the object.
  • the approach of coming up with more top-level variable names may run out of steam the deeper you go.

The T { ... } patterns are a little clunky, but would apply to any object. For more conciseness, we imagine that types could specify positional matching, which would be more compactly matched through a T (...) pattern.That would significantly shrink the recursive example:

if (s is ExpressionStatement(
        AssignmentExpressionSyntax(IdentifierName l, IdentifierName r)) 
    && l.name = r.name) ...

This is more concise, but it relies on you knowing what the positions stand for since you are not giving them a name at the consumption site.

So, in summary, it's interesting that the e is T x form in itself gives most of the value. The incremental improvement of having recursive patterns may not be all that significant.

Either way, the built-in null check is likely to help with writing null-safe code.

On the tooling side, what would the stepping behavior be? One answer is that there is no stepping behavior. That's already the case inside most expressions. Or we could think it through and come up with something better if the need is there.

The variables are definitely assigned only when true. There is a scoping issue with else if. It's similar to the discussions around declaration expressions in C# 6:

if (o is string s) { ... s ... }
else if (o is short s) { ... s ... } // error: redeclaration of s
else ...

All else equal, though not definitely assigned there, the string s introduced in the first if condition would also be in scope in the else branch, and the nested if clause would therefore not be allowed to introduce another variable with the same name s.

With declaration expressions we discussed having special rules around this, e.g. making variables introduced in an if condition not be in scope in the else clause. But this gets weird if you try to negate the condition and swap the branches: all of a sudden the variables would be in scope only where they are not definitely assigned.

Expanded switch statement

The above examples are in the context of if(...is...), but for switch statements you can't just put &&... after the initial pattern. Maybe we would allow a where ... (or when) in the case labels to add additional filtering:

switch (o) {
case ExpressionStatement(
        AssignmentExpressionSyntax(IdentifierName l, IdentifierName r)
        where (l.name == r.name):
    ...
}

If we add pattern matching to switch statements, one side effect is that we generalize the type of thing you can switch on to anything classified as a value.

object o = ...
switch(o) {
    case 1:
    case 2:
    case 3:
    case Color.Red:
    case string s:
    case *: // no
    case var x: // would have to be last and there'd have to not be a default:
    default:
}

We would probably disallow *, the wildcard, at the top level. It's only useful in recursive positional notation.

Evaluation order would now be important. For back compat, we would allow default everywhere, and evaluate it last.

We would diagnose situations where an earlier pattern hides a later one:

case Point(*, 2):
case Point(1, 2): // error/warning

We could allow case guards, which would make the case not subsume other cases:

case Point(var x, 2) where (x > 2): 
case Point(1, 2): // fine

User-defined is operator

We've sneaked uses of a positional pattern match above. For that to work, a type would have to somehow specify which positional order to match it in. One very general approach to this would be to allow declaration of an is operator on types:

public class Point
{
    public Point(int x, int y) { this.X = x; this.Y = y; }
    public int X { get; }
    public int Y { get; }
    overrides public int GetHashCode() ...
    overrides public bool Equals(...)...
    public static bool operator is(Point self out int x, out int y) {...}
}

The operator is is a particular way of specifying custom matching logic, similar to F# active patterns. We could imagine less ambitious ways, in particular if we just want to specify deconstruction and not additional logic. That's something to dive into later.
#4. Records

A value-semantics class like the above would be automatically generated by a "record" feature, e.g. from something like:

class Point(int X, int Y);

By default, this would generate all of the above, except parameter names would be upper case. If you want to supercede default behavior, you can give it a body and do that explicitly. For instance, you could make X mutable:

class Point(int X, int Y)
{
    public int X { get; set; } = X;
}

You could have syntax to create separate parameter names from member names:

class Point(int x:X, int y:Y)
{
    public int X { get; set; } = x;
}

Whether in this form or otherwise, we definitely want to pursue more concise type declarations that facilitate value semantics and deconstruction. We want to pursue the connection with anonymous types and we want to pursue tuples that mesh well with the story too.

@tpetrina
Copy link

One thing I would love to see with immutable and records is an easy way of creating new instances with one or more properties changed. Let's take a look at the class Point(int X, int Y). Can we do this:

var p1 = new Point(1, 2); // this one should just work
var p2 = new Point {X = 1, Y = 1}; // sometimes I prefer this
var p3 = new Point(); // X == default(int), Y == default(int)...done automatically

var q1 = new Point(p1) { Y = 3 }; // copy everything from p1 but use new Y
var q2 = new Point(p1, Y = 3); // automatic clone generation
var q3 = p1.WithY(3); // automatic With* generation

var r1 = new Point(p1, Y += 1); // expands to clone copy and evaluates expresion

I would love to use immutable for immutable views. Maybe we can use that on function parameters and functions to emulate const from C++.

@alexpolozov
Copy link

We would probably disallow *, the wildcard, at the top level. It's only useful in recursive positional notation.

I'm not sure I understand this rationale. In other languages, _ often comes at the top level as the "catch-all" case of a matching construct. Now, C# has default for it, but still — having extra implementation/testing/localization effort to forbid a trivial case doesn't look consistent. It looks like forbidding for(;;) or if(true): yes, this is not as common in typical code, but this is a trivial case, and trivial cases are essential for completeness of the theory.

Let's say I'm writing a code generator (e.g. using T4) that outputs if expressions with different pattern matching depending on different circumstances. If sometimes my autogenerated code is trivial (if should always succeed), what should I generate? if (x is var y) looks plain ugly, and requires to pick a fresh variable name.

@alexpolozov
Copy link

One more thing (I think we mentioned it once in the old thread on Codeplex). Why not allow identifiers on any patterns? This makes a grammar a little simpler and more consistent as well. Instead of:

pattern:
*
literal
var identifier
type identifieropt
type { member is pattern ... } identifieropt

we get:

pattern: primitive-pattern identifieropt
primitive-pattern:
*
literal
var
type
type { member is pattern ... }

It also enables some not-that-common-but-still-useful scenarios such as:

if (Configuration.Settings.Imagine.Longer.Property.Path.WebMethod is "POST" method) {
    // use variable 'method'
}

@AdamSpeight2008
Copy link
Contributor

I'd prefer a different construct than switch to make clear that it doing something different.
Nemerle uses match (an expression), I'd make a slight change and include the keyword default
Should be noted the it also uses ( , ) for Tuples.
'_' is a wildcard.

match ( x ) with
{
  | _ when x < 0 =>   // do this;  
  | 0                   =>    //  x == 0 so do this
  | 1
  | 2                   =>   // x is 1 or 2 
  default:
  // is none of the above
}

There order of the match statement is important as the first successful match is used, it exits it's match construct.

@MadsTorgersen your example from the Roslyn source maybe written with a match something like this.

match ( e , a ) with
{  
  | ( null, null ) => ;
  | ( _ , _ ) into 
       {l:= a.Left as IdentifierName;
        r:= a.Right as IdentifierName;
       } => match (l,r) with
            {
              | (null,null) =>
              | ( _ , _ ) when (a.Name.name == r.Name.name) =>
              default:
            }
  default:
}

Having match be an expression opens it usage in other areas like traits, code contracts, etc
It would be possible to have match follow a return.

 T foo<T> ( bar x )
{
 return match ( x ) with { ... };
}

@milutinovici
Copy link

Regarding pattern matching, I like the match expression. Returning of value and completeness are both very appealing. Even the syntax in your example is quite nice.

@AdamSpeight2008
Copy link
Contributor

@MadsTorgersen you should allowed to throw exceptiions, since you can embed it inside a try ... catch . C#6 will bring exception filters.

try
{
  var res = match( l, op , r ) with
            { 
              | ( _ ,'+', _ ) =>
              | ( _ ,'-', _ ) =>
              | ( _ ,'*', _ ) =>
              | ( _ ,'/'. _ ) =>
              | ( _ , _ , _ ) => throw new NotSupported();
            }
catch (ex) when NotSuppored
{

}

@danfma
Copy link

danfma commented Jan 30, 2015

I am being an intruder here, but I don't think that the switch keyword should be changed too. Maybe, it should just be transformed in one expression.

I think that the language must consider the match keyword, because it is clear what it will be doing and it is another keyword with a new behaviour. So, my opinion would be:

var result = match ( someObject ) {
   is 1 => "one";
   is 2 => "two";
   is int => "some integer";
   is long x => $"some long {x}"; // I really don't like the $"...";
   is double x if (x > 10.0) => "some double greater than 10";
   is (x, y) => "some tupple ({x}, {y})";
   else => "other thing";
}

// using when keyword

var result = match ( someObject ) {
   when 1 => "one";
   when  2 => "two";
   when  int => "some integer";
   when  long x => $"some long {x}"; // I really don't like the $"...";
   when  double x if (x > 10.0) => "some double greater than 10";
   when (x, y) => "some tupple ({x}, {y})";
   else => "other thing"; // else, default, otherwise...
}

Give a look at (http://kotlinlang.org/docs/reference/control-flow.html), just as another option.

@AdamSpeight2008
Copy link
Contributor

Tho following form of match will match against the types.

match type ( o ) with
{
  | ( String ) into s => ;
  | ( Short  ) into s => ; // not an error 
  // ...
}

It not an error because those are different s's, like in LINQ Query syntax.
's' is only in scope of that "match-case"

match type ( o ) with
{
  | ( String ) into s when (s != null) => ;
  | ( Short  ) into s when (s >= 0 ) => ; // not an error 
  // ...
}

@gafter
Copy link
Member

gafter commented Jan 30, 2015

I have sympathy for separating "match" from "switch", but I would like to note that it is not "doing something different", as the semantics can be carefully designed so that it is a superset of the existing switch statement. In other words any existing "switch" statement when thought of as a "match" statement still means the same thing that it did before. The pattern matching semantics can be designed to be carefully woven into the existing semantics of the switch statement.

Having said that, there is some syntactic baggage of the switch statement that we'd like to revise, including scoping of variables introduced in switch sections, and the requirement for "break" after switch blocks. Perhaps introducing a new "match" statement is an opportunity to go back to the drawing board on the syntax. But even if we introduce a new separate statement form, it is not clear that the value we get from changing these details (in the new form) is enough to overcome the confusion from the inconsistency in the language between the two forms.

There is an existing specification and prototype for this feature, which I'll post more about in a separate issue.

@fsoikin
Copy link

fsoikin commented Jan 31, 2015

This is more concise, but it relies on you knowing what the positions stand for since you are not giving them a name at the consumption site.

Since this is sugar for calling static method is, why can't it have named arguments, too?

if (s is ExpressionStatement(
        AssignmentExpressionSyntax(left: IdentifierName l, right: IdentifierName r)) 
    && l.name = r.name) ...

@AdamSpeight2008
Copy link
Contributor

@MadsTorgersen
The look and style of the proposed pattern-matching looks kludgey and even ugly.

@MgSam
Copy link

MgSam commented Feb 1, 2015

I'm surprised the pattern matching and records proposals aren't starting off where Neal's proposal left off in August. There was quite a bit of discussion in that CodePlex thread and I thought the prototype was in pretty good shape and mostly agreed upon by the community. I liked the shape of pattern matching and records much better in that then in this current proposal. I think the record keyword in particular is very important for the auto-generating of Equals, GetHashCode, ToString, and is operator.

The immutable keyword seems odd to me. As the design notes mention, it's not an absolute guarantee. I think the record keyword and shorthand syntax gets you most of the use cases where you want immutable types anyway, so this immutable syntax seems unnecessary and potentially misleading.

I think it's absolutely essential to consider a with operator to be added in concert with record types. Without it you force users to build a ton of boilerplate code in order to use immutable types (basically forcing them to write copy constructors by hand). I think record types should auto-implement the with operator in addition to its other duties.

@jaredpar
Copy link
Member

jaredpar commented Feb 1, 2015

@MgSam the guarantee provided by immutable is that the transitive closure of objects reachable by the reference cannot change. This guarantee is about as absolute as it gets in .Net. The only methods of breaking it are:

  • Reflection
  • Unsafe code

Both of those, in particular unsafe, can be used to violate a number of features in the language. They are generally not considered valid uses of a type.

What we were eluding to in the notes about not being guaranteed was method purity. Essentially that a method with all immutable inputs would always produce the same output. That cannot be guaranteed in C#, or really any .Net language, because it can be trivially circumvented with:

  • static caches
  • File system access
  • PInvoke
  • etc ...

These ambient authorities are always available to read and store state and hence method purity is next to impossible in .Net.

@fsoikin
Copy link

fsoikin commented Feb 1, 2015

@jaredpar I beg to differ on "ambient authorities".
You can definitely guarantee purity of a function by not allowing it to call any non-pure functions.

@gafter
Copy link
Member

gafter commented Feb 1, 2015

@MgSam the current pattern matching and records stuff is indeed based on the earlier work, we've just dropped the record keyword. It may of course change much more.

@jaredpar
Copy link
Member

jaredpar commented Feb 1, 2015

@fsoikin Sure, but how do you enforce that in a .Net language? There are so many easy ways to violate purity:

  • Threads
  • I/O
  • PInvoke
  • Statics
  • etc ...

I agree if you ban all of these items then indeed purity can be achieved. This really isn't practical though for a couple of reasons. Firstly because the list of impure functions is quite long and includes much, much more than I listed above. Secondly because it includes any function which transitively calls a impure function. That list is much larger and more difficult to determine.

For example you end up having to assume that all virtual / interface functions and delegates are impure (can't prove all implementations are pure). That actually turns out to be very restrictive. Would prevent calling even simple functions like ToString.

A better approach to solving that is to explicitly label pure functions. A pure function is limited to:

  • Reading instance fields
  • Calling other pure methods

This approach will work and give the purity requirements. But it is a much different feature than immutable.

@fsoikin
Copy link

fsoikin commented Feb 1, 2015

@jaredpar, yes, explicitly labeling pure functions is precisely what I meant. Sorry I wasn't clear enough.

@AdamSpeight2008
Copy link
Contributor

What does the attribute <System.ComponentModel.ImmutableObject(True)> currently do then?

@NN---
Copy link

NN--- commented Feb 2, 2015

Will be there immutable arrays such that generic constraint 'T : immutable' match them ?
Thanks.

@garora
Copy link

garora commented Feb 2, 2015

Same question as: @AdamSpeight2008

@jaredpar
Copy link
Member

jaredpar commented Feb 2, 2015

@AdamSpeight2008

I believe that is an attribute used to facilitate designers like WinForms. Essentially it says no property of this object is editable.

It doesn't seem to be a widely used attribute. Only use I could find ways on Image.

@NN--- The design allows for an immutable constraint on generic parameters.

@NN---
Copy link

NN--- commented Feb 2, 2015

@jaredpar I understand this, but currently there is no immutable array concept in CLR.
I am not sure whether it is going to appear in CLR 5.0 but if it appears it would be good to treat is as immutable by the constraint.

@mohitsharmaniit11
Copy link

I do like the Idea of NULL you mentioned in "C# Design Notes for Jan 21, 2015 #98". Expandable Switches are also Great.

@marhoily
Copy link

marhoily commented Mar 7, 2015

Did you consider just retyping variable after "is" operator like some other languages do?

object a = ...;
if (a is string && a.Length == 5) ...

Or do you not mention it because it might be breaking change?

@gafter
Copy link
Member

gafter commented Mar 7, 2015

@marhoily Yes, we considered it. I'm personally not a fan. What languages do that and how do people feel about those languages?

@fsoikin
Copy link

fsoikin commented Mar 7, 2015

@gafter, TypeScript does it, and people feel awesome about it.

@marhoily
Copy link

marhoily commented Mar 7, 2015

@gafter I'm only aware of jetbrains' katlin
I've never used any such language, I just see that the discussion goes between options a is T b and a is T {} when apparently there's a middle ground.
Unfortunately that would be a breaking change at least because some method resolution can go differently in case C# 7 retypes the original variable.

@marhoily
Copy link

marhoily commented Mar 7, 2015

Also I wonder how relevant the example from Roslyn with complicated if statement. Every textbook I've read advises to extract the condition into a separate method. The example I would care for is

var x = y as T;
if (x != null)
{
    x.F();
}

That is so common that Resharper has got built-in snippet. And that seems to go straight into pattern matching basket.

@gafter
Copy link
Member

gafter commented Mar 8, 2015

(y as T)?.F();

@gafter
Copy link
Member

gafter commented Mar 8, 2015

@fsoikin @marhoily Do any of the languages that do that have method overloading? I think that combination would be problematic.

@fsoikin
Copy link

fsoikin commented Mar 8, 2015

Well, TypeScript does have overloading.

Granted, it's not quite the same kind of overloading, since only signatures can be overloaded, but the body is always the same. In other words, all overloads always share the same implementation.

However, I think for the purposes of the overload resolution discussion, it is the same thing, because the return type depends on resolved overload, which means that occasional incorrect resolution would lead to incorrectly typed program.

So the bottom line is, I think it is worth looking at how TypeScript does it exactly. In fact, TypeScript's case is arguably even more complex, because TypeScript also has union types.

@marhoily
Copy link

marhoily commented Mar 8, 2015

Kotlin doesn't have full-fledged overloading madness of C# neither, but as well as in TypeScript retyping the variable can lead to compilation error in what previously was a correct code.

I guess that the central question with the approach in hand is how breaking the change it would be. Backwards compatibility is definitely not something that C# team is willing to sacrifice. I think that it would be nice to study actual code bases.

@KalitaAlexey
Copy link

I think this is ugly

if (s is ExpressionStatement e &&
    e.Expr is AssignmentExpressionSyntax a &&
    a.Left is IdentifierName l &&
    a.Right is IdentifierName r &&
    l.Name.name == r.Name.name) ...

More beautiful and meaningful that

if (s as ExpressionStatement e &&
    e.Expr as AssignmentExpressionSyntax a &&
    a.Left as IdentifierName l &&
    a.Right as IdentifierName r &&
    l.Name.name == r.Name.name) ...

It looks like variable declaration and it has sense.

@AdamSpeight2008
Copy link
Contributor

@KalitaAlexe have a look at #191 and see how that compares.

@gafter
Copy link
Member

gafter commented Mar 12, 2015

@KalitaAlexey When we considered this feature separately (that is, in the nonrecursive way as you are comparing possible syntax forms for), we informally called it the is/as operator. The logic for using is is that it returns a boolean. The as operator returns a T.

I prefer the recursive syntax

if (s is ExpressionStatement(
        AssignmentExpressionSyntax(IdentifierName l, IdentifierName r)) 
    && l.name = r.name) ...

@paulomorgado
Copy link

@KalitaAlexey, the is operator has always been a boolean operator while as is an operator of the type of the right expression.

If I look at it as s is (ExpressionStatement e), it still makes sense to me. If s is an ExpressionStatement (by any valid conversion), than e is s converted to ExpressionStatement.

s as (ExpressionStatement e) doesn't make any sense to me. It looks like something like I can already write today (e = s as ExpressionStatement).

If this was about using the as operator, with declaration expressions, we couild write it like this:

if (((var e = s as ExpressionStatement) != null) &&
    ((var e = e.Expr as AssignmentExpressionSyntax) != null) &&
    ((var e = a.Left as IdentifierName) != null) &&
    ((var e = a.Right as IdentifierName) != null) &&
    l.Name.name == r.Name.name) ...

But this is about the is operator. The new is just goes beyond identity check and also provide identity convertion.

@KalitaAlexey
Copy link

I've already figured out my mistake.

@gafter
Copy link
Member

gafter commented Apr 25, 2016

Design notes have been archived at https://github.com/dotnet/roslyn/blob/future/docs/designNotes/2015-01-28%20C%23%20Design%20Meeting.md but discussion can continue here.

@gafter gafter closed this as completed Apr 25, 2016
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