search

dotnet / csharplang

The official repo for the design of the C# programming language
11.58k stars 1.03k forks source link

Discusson: Property-scoped fields #626

Closed jcouv closed 7 years ago

jcouv commented 7 years ago

@bondsbw commented on Tue Feb 24 2015

Here is a typical C# property that wraps a backing field, such that the property includes a little bit of additional behavior:

private string _myField;
public string MyProperty
{
    get { return _myField; }
    set
    {
        _myField = value;
        NotifyOfPropertyChange(nameof(MyProperty));
    }
}

Whenever I set MyProperty from within that class, I set the property instead of the field so that I ensure the extra behavior runs. This is always the intended mechanism for writing this data.

Problem

During maintenance, developers make changes to the class and sometimes manipulate the field directly, instead of setting the property as the class designer intended. The result is that the additional behavior is not run, which leads to subtle bugs. It is a simple mistake, one I would like to prevent if possible.

Solution

One way to fix this situation is to be able to scope fields to their associated property. Within the property, the field can be fully manipulated as expected. Outside of the property, the field is invisible. Methods within that class are only able to interact with the property, which fully encapsulates the field.

Here is an example of what this might look like:

public string MyProperty
{
    string myField;
    get { return myField; }
    set
    {
        myField = value;
        NotifyOfPropertyChange(nameof(MyProperty));
    }
}

In this case, myField is fully encapsulated within MyProperty and nothing outside of MyProperty has access to myField directly.

Another option

Instead of explicitly declaring the field, perhaps a keyword provides access to an implicit field identified by a keyword, such as field. This could look like the following:

public string MyProperty
{
    get { return field; }
    set
    {
        field = value;
        NotifyOfPropertyChange(nameof(MyProperty));
    }
}

Additional Considerations

Fields should probably be initialized in a way that bypasses the property setter, such as the following example. The field should be initialized to null, but any future sets must have a non-null value:

public string MyProperty
{
    string myField = null;
    get { return myField; }
    set
    {
        if (value == null) throw new ArgumentNullException("value");
        myField = value;
    }
}

Should constructors have special access to the backing field, to be able to set the field directly in the same manner? Dot notation cannot work here. Something different would be necessary, such as

public MyClass(string val)
{
    MyProperty@myField = val;
}

Can the property encapsulate multiple fields? This might be useful in some contexts. It would remove the second option that uses the field keyword.

@KalitaAlexey commented on Sun Mar 15 2015

I think instead of

MyProperty@myField

must be used

MyProperty

but the compiler must ensures no setter method will be called. Just assignment to a field. Like in Swift. Because using of setter in constructor may lead to unexpected side effects

@bondsbw commented on Wed Mar 18 2015

Agreed that calling setters in constructors are generally the non-preferred case. I'm just worried that using the MyProperty = someValue; syntax would be confusing, because in non-constructor code, that same syntax does something different.

@CnSimonChan commented on Wed Mar 25 2015

I think constructors use MyProperty.myField is ok because now myField belongs to MyProperty. EDIT: it's ambiguous between setting backing field of MyProperty and setting property of the object which `MyProperty returned.

An extra problem is a property must not define a field with same name appeared in the class, compiler should issue an exception indicating field already defined. EDIT: If the compiler will rename this field in original class or create a mini-class to hold these fields, it's not necessary.

I disagree adding a new keyword, first it will limit one property has only one field, or why this field has a special name when others maybe also affects its return value (int MyProperty { get { return field1 + field2; } }), second adds another keyword will be a breaking change.

Besides I think it's also ok to set the initial value to a custom-setter property with C# 6 syntax (string MyProperty { set { backing_Field = value; } } = "Text";), I don't understand why there is such a limitation.

@bondsbw commented on Thu Mar 19 2015

@CnSimonChan:

An extra problem is a property must not define a field with same name appeared in the class, compiler should issue an exception indicating field already defined.

You highlighted the main problem with the MyProperty.myField approach. But another problem is that it appears to be the same as property/field access on the instance returned by the call to MyProperty, which can cause further confusion.

@KalitaAlexey commented on Thu Mar 19 2015

@bondsbw I think about confusion. What if write in constructor like this

MyProperty.initialValue = <someValue>;

This makes no confusion. Access to backed field must be only from property and from constructor. So name of backed field is extra.

@dsaf commented on Wed Mar 25 2015

1551 is a slightly different issue focusing on improving auto-properties rather than traditional properties.

@GeirGrusom commented on Fri Mar 27 2015

Why do we need to be able to assign a value to the property scoped field from the constructor? Doesn't that violate the entire point of a property scoped field?

@KalitaAlexey commented on Fri Mar 27 2015

@GeirGrusom Sometimes we want to initialize property with some value, but as said @bondsbw writing something like MyProperty = <someValue>; may confuse someone, because someone expect that in constructor setter will not be called (I prefer this variant) and someone else expect that in constructor setter will be called. Syntax like MyProperty.initialValue will give up any confusion.

@GeirGrusom commented on Fri Mar 27 2015

@KalitaAlexey @bondsbw The backing field is, and should be, inaccessible to the constructor. If the constructor needs write access to the property this should be through the setter. If the constructor needs access to the backing field then the field shouldn't be property scoped, but rather be in a scope that is available to the constructor.

There is no need for syntax to except constructors from normal scoping rules. It isn't a useful feature, and it introduces typing constructs on properties. Syntax like MyProperty.initialValue will be ambiguous if the property already is a type (should it resolve to the type member, or should the field shadow it?). Syntax like MyProperty@value will add a new member access operator used to break scoping rules in the case of constructors which will introduce a lot of cruft in the parser, which I think is pointless

My opinion is: just let properties define variables inside their own scope. This feature alone is useful in my opinion. Don't overengineer it.

@KalitaAlexey commented on Fri Mar 27 2015

@GeirGrusom I've thought about any useful case of setting field in constructor without setter and I failed. Indeed if I need change some variable without invoking setter, then my architecture is weak and wrong.

@bondsbw commented on Fri Mar 27 2015

Good points. I agree that property scoped fields are worth it even without any direct backing field access from the constructor, whose use case is likely small.

If we did want to handle that case, however, another way to implement it is in reverse. Instead of the constructor having direct access to the backing field, the property can be told that it is being invoked from the constructor. Perhaps something related to CallerMemberNameAttribute can be used... but I don't have any specific ideas on the matter.

All in all, I'd rather have property scoped fields without the ability to distinguish constructor access from others, than to not have them at all.

@HaloFour commented on Wed Dec 23 2015

This creates a new form of scope which seems like it would be inherently problematic:

public class Foo {
    private int _value;

    public int Value {
        private int _value;  // legal?
        get {
            int _value = this._value; // which _value?  legal?
            return _value;
        }
    }
}

As for consideration number 2, field is a legal identifier in C# so this would break existing code.

@bondsbw commented on Wed Dec 23 2015

public class Foo {
    private int _value;
    public int Value {
        private int _value;  // legal?

Probably not. I can think of few arguments for it and many against.

As for consideration number 2, field is a legal identifier in C# so this would break existing code.

I'm not a huge fan of that one anyway. Explicit fields give you more power and don't require new keywords.

@alrz commented on Fri Dec 25 2015

Some suggestions,

class C 
{
    public int P1
    {
        int field;
        var initialized = false; // (1) multiple, (2) inferred
        get { }
        set { }
    }
    public int P2
    {
        int field; // (3) same name (well, definitely)
        readonly cache = new Dictionary< ... >(); // (4) readonly, inferred
        static let static_local = ...; // (5) accessible to both getter and setter
        get { }
        set { }
    }
}

and (6) Access modifiers should not be permitted.

@bondsbw commented on Wed Dec 23 2015

@alrz Did you leave out get/set bodies for brevity? Or is that some kind of implied body?

var, readonly, let, and static are probably fine, but I'm not necessarily interested in bloating this feature if doing so would delay it.

@paulomorgado commented on Sat Dec 26 2015

This creates a new form of scope which seems like it would be inherently problematic:

public class Foo {
    private int _value;

    public int Value {
        private int _value;  // legal?
        get {
            int _value = this._value; // which _value?  legal?
            return _value;
        }
    }
}

As for consideration number 2, field is a legal identifier in C# so this would break existing code.

From a semantic point of view, I would equate the "local" field to a local variable or parameter.

So, value would be the "local" field and "this.value" would be the "global" field.

But then you couldn't have local variables or parameters with the same name.

@bondsbw commented on Sat Dec 26 2015

If I could go back in time and if I had my way, I would remove the ability to create fields and local variables/parameters with the same name. I can't think of a single time it helped anything, and "this." is a code smell IMHO.

I certainly don't want to add to the confusion by allowing property-scoped fields to share names with class fields or local variables. I vote to restrict that capability.

@alrz commented on Sat Dec 26 2015

@bondsbw (1) If you use same convention for parameters and fields e.g. camel case, then this would be helpful. The language should not restrict you on this matter. (2) That's actually a good property of blocks and roslyn itself used it in its codebase alot. In my example above, initialized or cache are just the right names for these fields, so why should I use prefixed names (I think this is known as smurf naming convention) for each property when I can figure it out by context and the block it is used in. That's what scopes are good at afterall, as you mentioned in the title

@bondsbw commented on Sat Dec 26 2015

@alrz 1) Regardless of convention, you can find another name. There are all kinds of cases where I might be tempted to use the same name for a variable within a single method, but the C# compiler disallows it. That forces me to think of a different name for each, and when I do, I find better names. This contributes to better readability. Same would apply with the restriction I suggested.

2) The syntax variables in your Roslyn link are declared in separate scopes, so there can be no confusion. Prefixing and "smurf naming" may be ways to mitigate the different scopes but they aren't the only ways. You choose what you do, and if you don't like those strategies, do something different.

Don't worry. C# is stuck as it is and due to BC we will always have fields and local variables sharing names, so you'll get to keep those all you want. But for these new property-scoped fields, @HaloFour has a point... we don't have to add to the confusion.

@alrz commented on Sat Dec 26 2015

@bondsbw I don't argue about past and done, yet I'm happy with it, of course. But about this, I'd rather follow the existing rules for blocks.

int i; { int i; } // Error
{ int j; } { int j; } // OK

So,

// Error
class A {
  int i;
  int P { int i; get { ... } set { ... } }
} 

// OK
class B { 
  int P1 { int j; get { ... } set { ... } }
  int P2 { int j; get { ... } set { ... } }
}

@bondsbw commented on Sat Dec 26 2015

Ok I think we're on the same page. To go further:

// Error
class A {
  int i; 
  int P {
    int i; // Error, "i" already defined
    int j; 
    get { int j; ... } // Error, "j" already defined
    set { int j; ... } // Error, "j" already defined
  }
} 

// OK
class B { 
  int i; 
  int P1 { 
    int j; 
    get { int k; ... } 
    set { int k; ... } 
  }
  int P2 { 
    int j; 
    get { int i; ... } // Not an error; already allowed per C# spec
    set { int k; ... } 
  }
}

@lachbaer commented on Thu Feb 04 2016

Refering to my proposal #8364 this could be extended with the value keyword:

class A {
    public string TheProperty {
        value string _theBackingField = "Initial String";
        int anotherPropertyScopedVariable = 0;
        get;
        private set;
    }
}

Decorators like readonly should come after value. Property scoped fields should always be private in my opinion, otherwise they should be normal instance variables (in CLR they will probably be private instance variables nevertheless).

UPDATE Proposal #8364 has already changed during the discussion. It now makes use of the default keyword.

In my opinion the only meaningful usage of a locally scoped variable is the backing field itself. I cannot think of any real world use cases where having more than one property scoped variable enhances the clarity of the code. It would lead to spaghetti code and there are probably much better approaches for realizing the wanted behaviour (like e.g. an extra (nested) class that cares about the properties heavy implementation).

But again, I see much use of one property scoped variable, the backing field itself. So maybe it's worth having a look at #8364 (to the bottom of the proposal). In short: it declares a default var backingField; with explicit type or type inference. This default backing field can be used in an extended set => syntax.

@lachbaer commented on Fri Feb 05 2016

I had a look in the compiler code. It seems to be a good idea to let some kind of keyword introduce a block of property-scoped variables, e.g. var.

public int Foo {
    var {
        int a;
        int b;
    }
    get { ... }
    set { ... }
}

When re-using var within that declaration, it could imply the type of the property.

public int Foo {
    var {
        var _internalBacking;  // this one is of the type of Foo
        string b;
    }
    get { ... }
    set { ... }
}

@lachbaer commented on Fri Feb 12 2016

When more than one scoped field is allowed, then it is hard to tell a property from a function when only the first lines are inspected:

public int Foo
{
  int _height;
  int _width;
  int _depth;
...

public int Bar()
{
  int _height;
  int _width;
  int _depth;
...

Of you could tell by the () braces, but they might likely be overseen.

Therefore it is a bad idea to allow more than one property scoped field in this way. It leads to hard-to-read code! (btw, checking for only one field definition is easily done in the compiler)

If you definitely need more than one field then one could use an extra private class, what is probably the cleanest solution for such a scenario anyway.

public class AClass
{
  class LocalPropFields
  {
    public int height, width, depth;
  }
  public int Foo
  {
    LocalPropFields _field = new LocalPropFields();
    get { ... } set { ... }
  }
}

In the last example, only one property scoped field is used.

As implicit types with varare only allowed at method scope this special keyword can be reused to imply the type of the property (as in examples of previous comments).

The initialization of the field (LocalPropFields _field = new LocalPropFields();) is done before the constructor is called, i.e. at the same time as all other instance variables with initializers are initialized.

As with all blocks, the field name hides another field of the same name in class scope. The access modifier is always private (meaning no other modifiers are allowed), because the field can nowhere be seen outside of the property - what is the means of all this ;-) (hint: because internally the field is effectively defined at class scope as private instance field, it could be better to disallow property scoped field names of same name as instance fields?)

@alrz commented on Fri Feb 12 2016

@lachbaer "The access modifier is always private" This I agree or about not allowing the var, but "When more than one scoped field is allowed, then it is hard to tell a property from a function when only the first lines are inspected" this I don't. As you said the parentheses are enough to distinguish between two (I don't know why it is important though). It's a block and if you can declare variables in it, it shouldn't be restricted to a single declaration, as it is limiting rather than helping.

@lachbaer commented on Fri Feb 12 2016

@alrz I really think, that the braces () could really easily be overseen, and so not clearly enough distinguish between a method and a property, which both are completely different parts of a class! If only one declaration statement (one line) is allowed before the get and/or set comes, a property is more easy recognizable.

Of course it is limiting, but when you need more than one field than maybe you should rethink your code design. Can you tell me of any real world scenarios, where you need more than one 'backing' field? Remeber, you can always use tuples (from .NET 4 on).

addendum: p-scoped fields must not even be internal or protected. It may not be clear in a derived class whether you access a p-scoped field or class-scoped field of the parent class. Therefore only private makes sense when we want to stick with clear code. ;-)

@HaloFour commented on Fri Feb 12 2016

While I do question how many use cases there are for multiple scoped backing fields I don't think it's necessary to limit them. The presence of get and set accessors should be enough to distinguish the member as a property as well as the argument parenthesis. Even with a handful of fields one of the accessors should be clearly visible within range of the member declaration. The developer would have to practically go out of their way to make it confusing to themselves.

I completely agree that there either should be no accessibility modifier on the scoped field, or it should be restricted to private only. That field should be completely inaccessible from anywhere outside of the property accessors; that's the entire point of the proposal. As the identifier is scoped to the property and multiple properties could define fields with the same name the backing field name should always be compiler-generated.

As for a var block, I don't think anywhere else in the language supports identifiers declared in one block that remain visible in a sibling block. It makes more sense to have the identifiers declared in the parent containing scope.

@alrz commented on Fri Feb 12 2016

@lachbaer It's not about just a backing field, it is a new scope for declaring fields. You should declare properties and methods separately and wrap them in #region if you really want them to be separated. It can be "overseen" even with the current syntax if you can't distinguish the parentheses. So that "limition" could be easily hijacked with T foo, bar; syntax right? Does that really help? I think it's more about code style and preference, if you believe that more than one variable in a property block scope is too much, nobody forces you to do it against your will. :)

@alrz commented on Fri Feb 12 2016

@HaloFour I didn't understand your last paragraph, I meant something like this,

T P { var field = ... ; get { ... } }

which I agreed that is not a good idea.

@HaloFour commented on Fri Feb 12 2016

@alrz

I meant the example:

public int Foo {
    var {
        int a;
        int b;
    }
    get { ... }
    set { ... }
}

I'd expect a and b to go out of scope at the point that the var block is closed, not remain accessible in the sibling get or set blocks.

@alrz commented on Fri Feb 12 2016

Oh, I missed that. Yeah, as I said in the commnet above it's better to follow block scoping rules rather than invent what is this I don't even.

@lachbaer commented on Fri Feb 12 2016

The proposal with var { ... } is based on

@HaloFour of course at first sight you don't expect variables to be visible outside a block, but in respect to properties you also don't expect get and set (or remove and add) to create 2 methods instead of their blocks being in code flow of the parent (property scope) block. So properties are nevertheless a very special case of the language and therefore can have special syntax with special behaviour.

I personally think that it is important to seen with one glimpse (without contact lenses) whether it is a method or property

int Foo {
    get {
       // properties always start with 'get' or 'set'
    }
}

int Bar() {
    MethodsDontHave(get, or, set);
}

The var keyword was only a proposal, as this would allow more locally scoped variables in accordance with my required "optical" difference to methods. Only because of latter I suggested not having more than one field.

@alrz commented on Fri Feb 12 2016

you also don't expect get and set (or remove and add) to create 2 methods

To be honest, I do.

@lachbaer commented on Fri Feb 12 2016

@alrz you know, what I mean ;-) In a block that (nearly) starts like a method as a novice I would expect set and get to be 'in flow', meaning to be executed one after the other, not be new kinds of method definitions. And if you refer to you knowing about the "special" syntax of properties, then you would possibly also expect to be all 3 blocks to be siblings in the same context - therefor being the variable block in scope.

Nevertheless I understand what you mean and I would be glad if anyone would come up with a solution that respects both aspect, block semantics and optical difference from a standard method.

@alrz commented on Fri Feb 12 2016

As a novice I couldn't process var { } syntax/semantics because basically everything that I learned about blocks suddenly doesn't make sense and I would probably sue MS if I didn't faint.

meaning to be executed one after the other

They won't be executed one after the other (?).

Properties' syntax has nothing to do with methods but it is good to "respect" block semantics at least.

@bondsbw commented on Fri Feb 12 2016

The var { } syntax might make more sense if we already used it for all the other types of member or variable declarations.

class C
{
    var {
        private Foo _foo;
        private Bar _bar = new Bar(4);
    }

    public int X { get; private set; }

    public void Calculate() 
    {
        var {
            int a = X;
            double b = Math.Pow(a, Math.PI);
        }
        if (b > 1000) X++; 
        else X = _bar.Y * _foo.Z;
    }
}

But none of those places use that syntax today, and I don't recall any complaints about readability.

@alrz commented on Fri Feb 12 2016

@bondsbw Why would you possibly wrap your field declarations (_foo and _bar) in var { }?

@lachbaer commented on Fri Feb 12 2016

Because you would have 3 sibling blocks

public int Foo {
   var {}
   get {}
   set {}
}

that would clearly (optically) identify a property. That is my main concern. But otherwise I agree that this would be some kind of language breaking! What I definitely want is to distinguish properties from methods by something more than (), even if a part time programmer does not define #regions of properties and regions of methods and totally mix them up.

@lachbaer commented on Fri Feb 12 2016

I just see that there already is a contextual keyword property in C# for the attribute specification [https://msdn.microsoft.com/en-us/library/aa664616%28VS.71%29.aspx].

You could use this special keyword to declare a property from C# 7 on when you want to use new features.

// new property keyword introduces C# 7 property
public property int Foo {
    int _field1;
    double _field2;
    get {};
    set {};
}

// without property keyword
public int Foo {
    int _field1;  // compiler error: property scoped fields require the property keyword in property declaration
    get {};
    set {};
}

// classic (old style) properties also accept the 'property' keyword, though it is not mandatory
public property int Foo { get; set; } = -1;

There is a Property keyword for defining a property in VB, so why not using it in C#? It has so many declarative keywords like override, readonly, ...

Just a sidemark: this would also allow for upcoming features, that might need CLR support, like:

// an indexed property (because we already have a standard this[] indexer)
// instead of writing own get_Foo(int n) and set_Foo(int n, int value) methods
public property int Foo(int n) {
    get { /* lookup @ n-th position */};
    set { };
}

// or a read-only property with setter
public readonly property int Foo {
    get { }
    set { /* check value for initialization and correct it or throw exception */ }
}
...
var mc = new MyClass() { Foo: -2 };
...

(@DavidArno you might like the latter idea)

@lachbaer commented on Fri Feb 12 2016

I guess, property scoped fields will bring some overhead to the created CLR code. The compiler must create an internal (nested) class that carries all the declared fields and then assign a new instance to an internal backing field. This must be, because actually from the compilers view set and get are two seperate methods within the instance scope. They have access to to their "parent scope", i.e. the class. When the field names shall be overridable, meaning different properties can have the same property-scoped field names, then the compiler must somehow guarantee that in the assembly the field names do not overlap with class scoped fields. This is quite an overhead for maybe just defining one backing field within the scope. Or we simply agree on uniqueness of the field names within the class? But that again would somehow break the integrity of block scoped variabels, wouldn't it...?

@HaloFour commented on Fri Feb 12 2016

@lachbaer There'd be no overhead, the compiler would just emit a field with some kind of naming convention that guarantees its uniqueness. This isn't dissimilar to what the compiler does with auto-implemented properties, except that you'd be assigning named identifiers which allow accessing them within the property accessors. The exact naming convention would probably be deterministic between builds but also an implementation detail subject to change.

So you'd expect the following:

public string Name {
    string field;
    get { return field; }
    set { field = vaue; }
}

to translate to the following:

private string <>_Name_field;
public string Name {
    get { return this.<>_Name_field; }
    set { this.<>_Name_field = value; }
}

I see no reason why C# would need to modify it's current property syntax. Adding property as a contextual keyword would be silly and add no value. Having said contextual keyword change the behavior of properties would be even sillier and extremely unintuitive. Having both C# indexer syntax and VB.NET indexer syntax makes no sense.

None of that would require BCL/CLR changes, either. I assume for that you mean the object initializer. That problem is conceptual, not technical. The expectation there is that an immutable type will permit mutability after it has been created, which is a contradiction. Immutable initializers will either involve the constructor or copy/"wither" methods and that's unavoidable. The latter has been proposed but is loaded with potential issues.

@lachbaer commented on Fri Feb 12 2016

@HaloFour latter is another topic and should not discussed here. I only wanted to make clear, that a property keyword could be used to mark > C# 7 features, just in case of a needed CLR change.

But back to the fields. What you propose for the backing fields is somewhat a hack to how it is now! First of all, internally setters and getters are ordinary methods and are handled like that during the compilation process (I have implemented #7881 already and had a slight insight in this).

When you rewrite the named variables, then 3 steps need to be added to the whole procedure - and only in case it is property method:

  1. is it the value parameter field?
  2. is a p-scoped field name? If so, rename it and look it up in the class context (respect inheritance)
  3. is it a local-to-the-geter/setter field name? This would give the compiler-makers some sleepless nights ;-)

By now, if you nest blocks within a method you will get the following error:

       public void meth()
        {
            int i = 3;
            {
/* Error: "A local variable named 'i' cannot be declared in this scope because it would give a different
    meaning to 'i', which is already used in a 'parent or current' scope to denote something else" */
               int i = 1;
               Console.WriteLine(i);
            }
            Console.WriteLine(i);
        }

and because with p-scoped variables we are actually at class sope, may be it is a wise idea to have field names that are unambiguous to the class, meaning you cannot reuse the field names in other properties. The p-scope would only be a "nice-to-have" to hide those names from outside the property block. To reuse them would be nice, I agree, but to stay realistic, I think that it will be a huge re-design process of field-lookup in the compiler (apart from reflection), and won't come the next years. Actually it is only some kind of syntactic sugar that should lead to having cleaner code and less coding accidents by using the c-scoped field directly instead of the property.

About the property keyword: I definitely want some more distinguishability between properties and methods in case the contents of the block are extended! That's why I suggest to make property mandatory in C# 7, if some new property features are used. It is not a switch! If there is only a getter or setter, the old syntax still stays intact, but property may still be used. This keyword is just a marker to see with a glimps that the next lines are a property - like when using a manually defined event where this keyword directly makes clear that we define an event in the following block.

About the necassity of indexer like properties: maaaaany use cases I can think of! Just see some of the GetXXX(string key) methods in the framework itself, that could all be rewritten as properties. But again, this would need major CLR changes to work throughout .NET and so might just be step in the far future.

@bondsbw commented on Sat Feb 13 2016

@lachbaer Indexer properties were proposed in #2144. I don't feel they are related to the current issue.

If you want to talk about confusion in reading, sticking property in the middle of several other keywords would be more likely to be overlooked than the difference in parentheses at the end. And it would also be confusing to require the keyword property for this new feature to work, as though it's something different from the properties that don't require the keyword. Just my opinion, but the value it adds is less than the confusion it will cause.

Should it be required that the field be unique for all property scopes in a class/struct, or unique only within the enclosing property scope (including no repeating of class/struct member names)? The latter seems most consistent. In the existing language, a scope cannot see a sibling scope's names, and at the same time it does not make uniqueness judgments based on the members of a sibling scope. To suggest that it could would be to say that it sees into a sibling scope, which weakens the definition of scope.

@HaloFour commented on Sat Feb 13 2016

@lachbaer No, a hack would be defining a new nested type which must be allocated simply to contain the fields. Having the compiler generate a nonsensical name is pretty consistent across numerous other language features such as anonymous types, iterators, async methods and auto-implemented properties.

As for scoping, there are three options:

  1. The property-scoped fields must be unique across all fields defined in other property-scopes as well as the parent type scope. The property-scoping is purely compiler-candy and all of the fields are declared directly on the parent type with the same name and private accessibility.
  2. The property-scoped fields cannot shadow fields declared in the parent type scope but may be duplicated between different property-scopes. From within the property accessor methods you can reference these fields through the this keyword as well as shadow with local variables of the same name.
  3. The property-scoped fields cannot shadow fields declared in the parent type scope but may be duplicated between different property-scopes. From within the property accessor methods you cannot access the property-scoped field through the this keyword and you cannot shadow with a local variable.

The third option seems to be the most reasonable, especially since the property-scoped fields look and feel more like local declarations rather than field declarations.

I think that your ambiguity concerns simply don't stand. They would exist with normal properties just like they would with this "extended" form. The vast, vast, vast majority of these properties would contain one field declaration so the argument that the accessor methods somehow falls off of the screen and becomes impossible or confusing to see is unfounded. What would be confusing is to somehow have the declaration of a property change depending on the features used within that property.

Events use the event keyword regardless of whether they are auto-implemented or explicitly-implemented. If auto-implemented property syntax had been designed along with the original event syntax it's quite possible that either, or both, would be different.

About the necassity of indexer like properties: maaaaany use cases I can think of! Just see some of the GetXXX(string key) methods in the framework itself, that could all be rewritten as properties. But again, this would need major CLR changes to work throughout .NET and so might just be step in the far future.

No CLR changes needed. VB.NET has supported them since it's first release. It was an explicit design decision to not support them in C#. They create ambiguity in property resolution. The recommended design is to have a normal property that returns a type that defines an indexer. As for those existing methods, the reason that they're not properties is more likely that they do something to violate Microsoft's documented design patterns for properties and thus would make more sense being a method. For example, throwing an exception (beyond validation of arguments).

@lachbaer commented on Sat Feb 13 2016

@HaloFour

other language features such as anonymous types, iterators, async methods and auto-implemented properties

But all these features create identifiers that are not directly named in the language. When you write int fieldName; you expect the the name of the field fieldName, also in the MSIL and when using reflection. Even the special value keyword for the setter actually is no keyword, because the set_XXX method internally is created with a real paramenter called 'value'.

You suggest, that something like

public int Foo {
  int pScopedFieldName;
  get { return pScopedFieldName; }
  set { pScopedFieldName = value; }
}

would be (internally) rewritten to

private  int <pScopedFieldName>k__BackingField;
public int Foo {
  get { return <pScopedFieldName>k__BackingField; }
  set { <pScopedFieldName>k__BackingField= value; }
}

Can you name anything in C# where the real written down identifier name is rewritten? I don't think so. That's why I called it a hack. Again, when I myself as programmer write down fieldName I want it to be fieldName and not some freaking internal name. For anonymous types, internal management methods for await and the backing field of an auto-property the programmer does not explicitly declare a field name. You see what I mean?

For the scoping and name overwriting in other scopes the only (currently) realistic implementation is to have it as syntactic sugar, meaning

public int Foo {
  int _pScopedFieldName;
  get { return _pScopedFieldName; }
  set { _pScopedFieldName = value; }
}

is just translated to

private int _pScopedFieldName;
public int Foo {
  get { return _pScopedFieldName; }
  set { _pScopedFieldName = value; }
}

All other semantics stay the same as they are today, meaning that the field names must be unique throughout the class, because they are acutally just class fields. The only real (big) benefit is that the compliler does not allow access to these fields outside of the corresponding property and so avoiding accidential direct use of the field passing over its setter or getter.

Today there seems to be absolutely no implementation in the compiler of "block scoped methods", so that the declared fields are only visible in and bound to the property block, so another property (within the same class) could have another bound field block with the same names. Maybe the implementation of "local methods" that is scheduled for C# 7 introduces something like it, id'tk?

So the approach of first using only class scoped unique names in the property is very valid for now. If the compiler team introduces the 'block bound scope' for anything that has a block in C#, things could change - later. It won't break then existing code, because then existing code anyway take care of the uniqueness of the p-scoped field names.

About property keyword: I think that it is a good idea to introduce this keyword to C#, and I think that it should already have been there since the beginning. Just seperating a method from a property by adding or ommiting two paratheses is just too less for a language who's goal is clarity. But actually this would be another topic in the first place and not related to this issue.

About indexer like properties: that's another discussion. Let it go for now.

@HaloFour commented on Sat Feb 13 2016

Reflecting private fields is an inherently fragile operation. If anything hiding said field through compiler-provided "obfuscation" would only be an added benefit. Wrapping the fields in some nested type would be even worse since there would be no corresponding field that matches the scoped field declared directly on the type.

The C# language has zero enforcement of unique identifiers between sibling scopes. It would be inconsistent with the rest of the language to require it exclusively in this context. Even local functions provides the exact same identifier scoping rules as the rest of the language, complete with enclosing the parent scope.

The ship had sailed on property syntax design. Having multiple forms of syntax with wildly different semantics would add nothing but confusion to the language.

The team has already weighed in on their opinion of named indexes properties. The pattern to implement it now is considered sufficient, and it's not a design patterns they want to encourage.

https://roslyn.codeplex.com/discussions/542484

@lachbaer commented on Sat Feb 13 2016

First of all, I don't encourage to create a nested class for p-scoped fields! Not that we misunderstand each other. :-)

There are big differences between getters, setters and local functions!

Local functions have access to their outer variables of the parent functions block. Well, there IS an outer variable scope. BUT, the inner (local) function IS NOT accessible from anywhere else in the code.

Getter and setter are JUST OPTICALLY designed within the same scope. They ARE sibling functions of the containing class. There is NO INDEPENDENT outer contextual (variable) block, like with a method. The contextual block IS the CLASS SCOPE with its class fields.

And the latter is the problem in my eyes! Local functions can ref to the field block of their parent method. (Sibling) property methods can not, because there is no such "local variable context".

Remember VB6? Property Get ... and Property Let are 2 seperate methods in the same class. It's the same in C#, but it just not looks so... The enclosing brace { is just "syntactic sugar".

I hope, you now see the real scoping problem....?!

@lachbaer commented on Sat Feb 13 2016

Example, acutally the definition for a property since C# 1 is in reality:

public class AClass {
    int _field;
    public int myProperty get { return _field; }    // internally converted to get_myProperty();
    public int myProperty set { _field=value; }    // internally converted to set_myProperty(int value);
}

That is how it is until today. You see that there is no real block scope! And as there is no block scope, there is no enclosing local scope for variables.

@HaloFour commented on Sat Feb 13 2016

@lachbaer

Property accessories methods are not just sibling methods like they are in VB6. The property has official meta data unto itself. However, even that is implementation details irrelevant to this discussion. The CLR has no concept of child scopes within a method either, that's a feature completely faked by C#.

The syntax puts both methods under a single block and child blocks always inherent the scope of their parent. Changing that in one place and that place only makes no sense. It is much more important for the language to remain consistent with itself than to split hairs regarding implementation details of IL.

We're not going to agree on this subject.

@HaloFour commented on Sat Feb 13 2016

Sorry, your invented syntax doesn't compile. I wonder why?

@lachbaer commented on Sat Feb 13 2016

@HaloFour
Even in VB6 properties have (TLB/COM) metadata ;-) I just wanted to point out that in VB6 these two methods are actually defined as sibling methods (in VB.NET they're not). In C# and VB.NET they are in an enclosing block, but internally still sibling methods in the class.

I will have a look at how this variable block scope is realized in the compiler and come back on this after. But it may take some time.

Let's go a step back to the essentials of this topic. Do we all agree on the following proposals?

  1. It's a good idea to have property-scoped fields
  2. Those fields are private to the property, i.e. can only be access by the corresponding accessor methods (get, set, add, remove)
  3. The field declaration may have an access modifier of private, because private is implied anyway
  4. The type of the fields may be any visible types within the namespace and context of the surrounding class
  5. The type of a field could be var, what inferes the type of the property itself
  6. The field definition can contain initializers
  7. This feature should primarily be used to define a backing field, that cannot accidentially be accessed anywhere else in the class, so primarily defining only 1 field. Other goals with many fields can be acomplished, but is strongly discouraged.

Example:

public class AClass {
  public int Foo {
    private int _field1 = 0;
    int _field2;  // same as _field1, auto-initialized with 0
    var _field3;  // type 'int' infered from property
    get {...}
    set {...}
  }

  private void AMethod() {
    this._field1 = 12; // error: not in scope!
  }
}

Open issue: reusing of field names. Depends on internal constraints of the current compiler implementation and CLR. To be discussed when some light is shed on it.

@bondsbw commented on Sat Feb 13 2016

@lachbaer When I proposed this issue, language scope was the primary consideration. Weakening the typical behaviors of C# scopes seems contradictory to the goals of this proposal.

@lachbaer commented on Sat Feb 13 2016

@bondsbw Good you pointed your goal out again. So you definitely want to reuse field names in other properties of the same class? Do you agree to the other points above?

@bondsbw commented on Sat Feb 13 2016

@lachbaer

  1. Yes
  2. Yes
  3. No, I'd prefer not to allow private at all. Just like you cannot declare local function variables as private. private implies something that is accessible anywhere within the class, and that isn't the case here.
  4. I think this is right. To clarify, all types visible by class members should also be visible for property-scoped members.
  5. Inference should only occur from an initializer. I do not support inference from the property type itself. (The declaration of _field3 in your example would be invalid.)
  6. Yes (and var _field3 = 15; would be valid)
  7. Yes that multiple should be allowed. I don't necessarily agree that multiple should be discouraged, though I'll leave that to C# developers at large to decide whether that is problematic.

Otherwise your example seems correct.

@lachbaer commented on Sat Feb 13 2016

for 3. valid argument. for 5. Type inference with var is only valid in the methods itself ("Beginning in Visual C# 3.0, variables that are declared at method scope can have an implicit type var."; [https://msdn.microsoft.com/en-us/library/bb383973.aspx]() ) we are outside the actual method scope, so var is not allowed here. Therefore it can be reused for property type inference. As the optional initializer must be of the same type as the property anyhow it still looks like a method scoped var with type inference. If the initializer is left out, the inference should be clear to everyone none the less.

@bondsbw commented on Sat Feb 13 2016

@lachbaer That is somewhat inconsistent. If there were only one property-scoped field, I could see the utility (it could be seen as the equivalent of explicitly defining the name of the backing field, which is always the same type as the property). But even then I'd prefer a different keyword or syntax just to make it look different, say something like this:

public string ErrorMessage {
    using _errorMessage;
    get { return _errorMessage ?? "All systems nominal."; }
    set { _errorMessage = value; }
}

But honestly I don't feel that you get a whole lot of utility out of this, compared with the current proposal, so I would suggest either doing one or the other but not both.

I just feel that var in this case would be confusing (typically its type is determined to the right, not above). And doing so may cause consistency issues if C# ever does allow for implicit var member field declarations.

@lachbaer commented on Sat Feb 13 2016

@HaloFour

Sorry, your invented syntax doesn't compile. I wonder why?

It's pseudocode, if you haven't noticed... But that is what is really going on under the hood!!!!

@HaloFour commented on Sat Feb 13 2016

@lachbaer

It's pseudocode, if you haven't noticed... But that is what is really going on under the hood!!!!

Indeed, but it's not C#, so it doesn't have any bearing on a discussion regarding C# syntax. If C# had kept the members separate then you'd have an excellent point.

Also, that's only half of what's going on under the hood. You'd notice that if you were to define the accessor methods by themselves that other CLR languages would not recognize them as properties. Without the property member itself declared the methods would never be recognized as a property.

And as mentioned, what the CLR and IL do under the hood aren't really that relevant. C# takes numerous liberties in how it may implement certain features which don't exist in IL, and IL offers many features that C# doesn't expose at all. Locals are a good example as to IL they are nothing but a flat indexed table of types. Even the names don't survive beyond the debugger symbol metadata. The IL assembly language does offer some concept of block scoping but it requires the developer to manually assign the index slots otherwise they are assigned in lexical order, and that information is lost during assembly.

Anywho, I think we've pretty much exhausted our arguments for our opinions on the subject. I'd say it's up to the language team to decide.

@CyrusNajmabadi commented on Sat Feb 13 2016

So just to weigh in on what i've been thinking about for my implementation.

First, this is just state that's available to this single property that the value is scoped to. In that regard, it's similar to a 'local'. Just a 'local' that is available to all the accessors. As such, this is how i've been designing things from teh language level:

  1. You can have any number of property locals declared with the property.
  2. The property locals can come before/after/interspersed with the actual property accessors.
  3. The property locals are scoped to that property alone. Other members of the class/struct cannot interact with them.
  4. Subclasses can't interact with them (including overrides of the property).
  5. 'var' can be used for these property locals.
  6. Indexers should also be able to use these sorts of property locals.

Now (while this is out of the scope of the actual language change), here's how i would expect to emit them.

  1. These would simply be fields within the same class/struct that the property/indexer is contained within.
  2. The fields would be private.
  3. The fields would be named in such a way that they could never collide with an actual class member. likely something wonky <>_propname_localname

Open questions:

  1. Allow attributes on these locals? It wouldn't be hard to support, and there could be value in putting attributes on the final fields emitted. This does leak through though the implementation of how we deal with property locals.
  2. Allow these for events?
  3. How are we going to handle the syntax changes here in our API. It's non trivial to modify the syntactic constructs here in a non-breaking fashion.
  4. We've added 'local functions' to C#7. Should we allow "property local functions" as well?
  5. Should we allow other 'local' constructs? We could have local methods/types/etc. within a property. Technically it would all be possible.

For 3 and 4 i'm undecided. I can see the value in having local helper functions/methods that your property uses and which you don't want anything else to see. However, once you start encapsulating state+functionality+etc. then i start thinking you should use an actual class/struct for this. You can do so in a zero overhead manner by using a struct for this. Then, if your property has a property scoped local of that type, then you ensure only this property sees that state, and any procedural functionality you want can be provided through the struct.

@lachbaer commented on Sat Feb 13 2016

@CyrusNajmabadi could you give me a hint where in the roslyn source code you map the local names to the <>_ member fields? I'd like to have a look on how the local variables are scoped a.s.o., but cannot really find a starting point.

@lachbaer commented on Sat Feb 13 2016

To the open questions:

  1. What kind of attributes do you think of? As the actual field is a (hidden) class scoped field, wouldn't it be logical to accept all attributes for that kind, including serialization?
  2. Yes, but as an event does not need a backing field like a property no one will probably use the feature ever ;-) But if, then it's there. Shouldn't be any overhead in the code, more overhead to disallow it maybe?
  3. ...
  4. Like with events, I think it does not really matter. What eases the compiler and API code?
  5. Definitely NO! Getters and setters are only little helper functions. Everything that might be too close to the data model of the class should be outsourced to a class scoped method for interacting with that class. Otherwise you will get spaghetti code quickly, because it is hard to search for the right place. As you said a struct or another class would be the more suitable place, something like struct MyPropertyLogic. That again could be a nested privated class in class scope. Just imagine it being a nested class in an embedded property of an outer class... urgh... ;-)

@CyrusNajmabadi commented on Sat Feb 13 2016

I'd like to have a look on how the local variables are scoped a.s.o

I'm not sure what you mean. None of this code has been written yet. I'm still thinking about how the feature would be defined before starting on the actual implementation.

What kind of attributes do you think of?

Whatever the user might want. Maybe they use a serialization framework that uses attributes to specify which fields should/shouldn't be serialized. They might want these fields included/excluded for example.

Like with events, I think it does not really matter

I think it does. in c# 7 we will tell people "within a method body you can have local variables and local functions". Then saying "within a property you can only have local variables" and not local functions seems... off to me. Especially as a local function is effective just an invokable local variable (though much more efficiently implemented).

@HaloFour commented on Sat Feb 13 2016

  1. Definitely. NonSerializedAttribute is a biggie. Still waiting for field-targeting attributes to be supported on auto-implemented properties, by the way. #7004
  2. Not sure how much sense this really makes. Sure, an event with custom accessors does generally have a backing field of a delegate, but if that field is not available to other members within the type then it could never be invoked. I can't think of any real use case where you'd both want custom accessors with some kind of backing state but wouldn't need to be able to reference that state from somewhere else in the type. Events have never been quite on par syntactically or behaviorally with properties.
  3. Wouldn't this be purely additive? How would it break the existing API?
  4. Scoped in a way that they could be used by both getter and setter accessor, rather than being local to either? See below.
  5. Are there a lot of use cases for wanting to have types or methods scoped within the property? The big use case for the scoped fields is to prevent accidental access to the backing field, but helper types and methods aren't nearly as likely to be accidental targets.

@CyrusNajmabadi commented on Sat Feb 13 2016

Wouldn't this be purely additive? How would it break the existing API?

The current AccessDeclarationListSyntax looks like this:

OpenBrace
SyntaxList<AccessorDeclarationSyntax> Accessors 
CloseBrace

Having locals allowed anywhere between accessors makes this complicated. The easiest way to do this would likely only to allow locals before/after the accessor list. But that's a case where the implementation is leading the language, as opposed to this being the actual right thing for the language.

Slightly harder would be to have a new type of AccessorDeclaration. Like a "LocalDeclarationAccessorDeclarationSyntax". However, this would be quite ugly as AccessorDeclarationSyntax has these members declared on it: 

modifiers
attributes
keyword
body
semicolon

Keyword/body would especially make no sense here in the syntactic API.

Another option would be to introduce a new BaseAccessorDeclarationSyntax. Then expose two properties 'AllAccessors' and 'Accessors'. BaseAccessorDeclarationSyntax would have almost nothing on it (maybe just modifiers and attributes). The latter would then just return the old style accessors. However, this is unpleasant too as existing code that attempts to walk the tree won't even see the new members.

No great options as far as i can see.

Are there a lot of use cases

Sure... you have helper functionality that you only use with the property. And so you want to scope it just to that. Perhaps validation or other house keeping.

That said, it's probably less essential that this be provided as there is a suitable workaround i mentioned earlier.

@lachbaer commented on Sat Feb 13 2016

@CyrusNajmabadi With "it does not really matter" I just meant, a getter/setter is no "ordinary" method, at least it does not really look like one. It is just "code in a braced block". Methods start with a method declaration before the block begins. You know what I mean. So in my eyes it would be no restriction to not allow it, because an accessor is --- logically, not technically --- not a method.

But when I have quickly implemented the expression body syntax for accessors, I saw that internally they are just handeled like ordinary methods and parsing is hand over to the ordinary method parsing routines. So everything that is (and will be) possible in a method should also be possible in an accessor with any further ado...?!

But, I don't think it is a good coding style to put loooong algorithms in s/getters anyway. Whoever does this must be punished by - 1 week abstinence of coffee or so ;-) So at least we are talking about "clean" coding and "better" coding here and features that help us with improving everyday problems. The need for p-scoped fields comes very probably from the need of getting rid of 1 or 2 class scoped private fields for every self-implemented property, where the collegues were always tempted to access the fields directly instead via the s/getters. That's why I wrote "it doesn't really matter" wheather all features must be available to s/getter methods. Hope this "very few" words make my stand clear :-)

But I still wonder how you will implement the rewrite of the p-scoped fields. The CLR indexed slots of locals are vanished as soon as the method (in this case the s/getter) ends, aren't they?

@CyrusNajmabadi Couldn't it be helpful to use a compiler generated struct instead of rewriting local identifier names all over the accessor place? Even in the latter case, you end up with kind'a ugly compiler generated field names. Here's what I mean as suggestion:

public class MyClass {
    private struct <k>__MyProp_MyClass {
        private int _field1;
        private int _field2;
        public int get_MyProp() { return _field1 + _field2; }
        public void set_MyProp(int value) { _field1 = value; _field2 = value; }
    }
    <k>__MyProp_MyClass <MyProp>k__backingStruct;
    public int MyProp {
        get { return <MyProp>k__backingStruct.get_MyProp(); }
        set { <MyProp>k__backingStruct.set_MyProp(value); }
    }
}

So, the whole self coded part goes in the struct methods, no real identifier name rewriting must take place. Of course this is only generated if the compiler sees anything more than an ordinary get and set within the property declaration. I think this approach could be in accordance with the way the compiler already does things, like async and so.

@lachbaer commented on Sat Feb 13 2016

@HaloFour seems at least we agree on some things in the list :-) (y)

@lachbaer commented on Sat Feb 13 2016

@CyrusNajmabadi

Slightly harder would be to have a new type of AccessorDeclaration. Like a "LocalDeclarationAccessorDeclarationSyntax". However, this would be quite ugly as AccessorDeclarationSyntax has these members declared on it

That's exactly why I suggested some way above the use of a let (or var) body:

public int Foo {
    let {   // or 'var' keyword if you like it more
        int a;
        int b;
    }
    get { ... }
    set { ... }
}

In that let body you can loose of all the attribute methods that are already available in the compiler code today. But others here think that it would be too inconsistent, because they 'see' the fields going out of scope after the let block. What do you think from an implementors point of view?

@CyrusNajmabadi commented on Sat Feb 13 2016

That's exactly why I suggested some way above the use of a let (or var) body:

I'm loathe to make language choices based on the issues i'm having with the syntactic API. :)

@CyrusNajmabadi commented on Sat Feb 13 2016

So everything that is (and will be) possible in a method should also be possible in an accessor with any further ado...?!

I'm not talking about what's possible inside the accessor. I'm talking about what new entities we allow in the accessor list.

This feature, to me, is about adding support for 'locals' within the accessor list. If i'm going to allow local delarations, should that only be local variables, or should local functions also be allowed?

But, I don't think it is a good coding style to put loooong algorithms in s/getters anyway

It's not necessarily about a long algorithm. It's about having to repeat yourself for common code you might be using (and which may be using the local). Now, again, i'm not sure this is that important. So i'll likely leave it out and allow the LDM to decide if it shoudl be in.

But I still wonder how you will implement the rewrite of the p-scoped fields. The CLR indexed slots of locals are vanished as soon as the method (in this case the s/getter) ends, aren't they?

I think we're not on the same page. This wouldn't effect actual locals within the property getter/setter. This would only affect these new "property scoped locals". In this case, as they'll just be fields under the covers, and reference to them in code will simply be to the field just like we handle fields today.

i.e. if you have this code:

class C {
    int field;
    string Prop {
        get { ... do something with 'field' ... }
    }
}

This can now be rewritten to:

class C {
    string Prop {
        int field;
        get { ... do something with 'field' ... }
    }
}

All that will happen is that 'field' will actually get an emitted name that is unspeakable. But all the normal field references in the body of hte property will remain normal field references, emitted the same way as we do any field reference.

@lachbaer commented on Sat Feb 13 2016

@CyrusNajmabadi Yeah, I already understand the latter, but wouldn't it give you a hard time to rewrite the mentioned field names with the compiler generated synthesized field names? You need to keep track of a lookup-table and have to cross check every mentioned identifier in the block wheather it must be 'rewritten' to reference the synthesized field. And next, you must not do so in normal methods, meaning that you have to check that too if you reuse parser methods.

I'm talking about what new entities we allow in the accessor list.

Sorry, didn't get it. I was talking about new c# 7 local methods. Concerning this, I'll stand with my point that accessors (and properties) should only be lightweight, otherwise logic should be sourced out to other places. So, no, don't allow it :-)

@CyrusNajmabadi commented on Sat Feb 13 2016

but wouldn't it give you a hard time to rewrite the mentioned field names with the compiler generated synthesized field names

I think you're misunderstanding how the compiler works. The compiler will just "bind" the references in the getter/setter to this field symbol. The field symbol will be emitted with a special name. But the emitting for the field references shoudl happen naturally and transparently.

@lachbaer commented on Sat Feb 13 2016

Maybe I misunderstand something? You see the declaration of the p-scoped field, synthesize a c-scoped field and link the clear identifier name with the reference to the c-scoped field. The name of that synth field is never used again. Then when the parser sees an identifier that is not local it climbs the stairs up in the identifier<->reference chain? Sounds easy :-)

@CyrusNajmabadi commented on Sat Feb 13 2016

This is not how our compiler works :)

The parser is only response for producing concrete syntax trees. It doesn't do any semantic analysis at all.

all that needs to happen once we produce the field symbols for this is to update our binding code to allow matches to those fields from within these properties (and not from anywhere else). When we emit we can pick any name we want for this field. Because the compiler will have already bound the references properly it will just emit the references properly, regardless of whatever name we emitted for it.

@bondsbw commented on Sat Feb 13 2016

@CyrusNajmabadi

Having locals allowed anywhere between accessors makes this complicated. The easiest way to do this would likely only to allow locals before/after the accessor list.

I was thinking the fields would be required to be specified before accessors anyway. Looks better IMO. But I'm not very picky here.

We've added 'local functions' to C#7. Should we allow "property local functions" as well? Should we allow other 'local' constructs? We could have local methods/types/etc. within a property. Technically it would all be possible.

I agree with @HaloFour, these deviate from the goal of this proposal and seem unnecessary and messy. But if someone presents a good case for it, I'm not totally against the idea.

@lachbaer commented on Sun Feb 14 2016

@CyrusNajmabadi would the following changes break the API in a way that is not acceptable?

<!-- this Node hase been added, it looks exactly like "FieldDeclarationSyntax" -->
<Node Name="PropertyLocalFieldDeclarationSyntax" Base="BaseFieldDeclarationSyntax">
    <Kind Name="PropertyLocalFieldDeclaration"/>
    <Field Name="AttributeLists" Type="SyntaxList&lt;AttributeListSyntax&gt;" Override="true">
      <PropertyComment>
        <summary>Gets the attribute declaration list.</summary>
      </PropertyComment>
    </Field>
    <Field Name="Modifiers" Type="SyntaxList&lt;SyntaxToken&gt;" Override="true">
      <PropertyComment>
        <summary>Gets the modifier list.</summary>
      </PropertyComment>
    </Field>
    <Field Name="Declaration" Type="VariableDeclarationSyntax" Override="true"/>
    <Field Name="SemicolonToken" Type="SyntaxToken" Override="true">
      <Kind Name="SemicolonToken"/>
    </Field>
  </Node>

  <Node Name="AccessorListSyntax" Base="CSharpSyntaxNode">
    <Kind Name="AccessorList"/>
    <Field Name="OpenBraceToken" Type="SyntaxToken">
      <Kind Name="OpenBraceToken"/>
    </Field>
    <!-- next line has been added -->
    <Field Name="LocalFields" Type="SyntaxList&lt;PropertyLocalFieldDeclarationSyntax&gt;" Optional="true" />
    <Field Name="Accessors" Type="SyntaxList&lt;AccessorDeclarationSyntax&gt;"/>
    <Field Name="CloseBraceToken" Type="SyntaxToken">
      <Kind Name="CloseBraceToken"/>
    </Field>
  </Node>

At least property fields are some kind of class fields, and if semantically modifiers aren't allowed the compiler can check that later. I'm somewhat not comfortable to use the LocalDeclarationStatementSyntax, because though they should shine to look like lokals, when are they initialized? They are not really in the method but at class level.

It does not seem to me to easily have the var keyword implemented for property type inference. Of course it could be done, but is it worth the overhead? @CyrusNajmabadi you have probably written a lot of code already, what do you think? Would it be in favour of writing (or auto-generating) easy to maintain code?

@CyrusNajmabadi commented on Sun Feb 14 2016

That would not break the API and is likely something similar to what i will do. However, as mentioned before, from the language perspective it's unfortunate that this would mean that fields could not be interspersed with accessors. But it's something i'm considering.

It does not seem to me to easily have the var keyword implemented for property type inference

I don't see why 'var' would be hard to have at all.

@lachbaer commented on Sun Feb 14 2016

I think that it would actually be a benefit to have the fields at the beginning! Because you only need to define fields that you need in both, getter and setter. Otherwise it would be somewhat useless and lead to harder to oversee code. I'm already afraid of coders who put 30 fields in an accessor and the get and set keyword nicely hidden somewhere in between ROFL - We are doing C# here, note the '#' ;-)

Properties in my eyes shall only be some lightweight constructs for all kinds of scenarios. P-scoped fields are very nice for this to hide small portions of code from the rest of its surroundings, and so embedding the 'little, but not so little' things. The context keywords shall directly be behind the field declaration.

For heavy procedures one should consider using an extra class or struct or writing dedicated methods instead of abusing accessor methods.

In class and interface definitions on the other hand it makes many sense to intersperse fields with methods, e.g. for creating #regions. But keep the properties simple, please.

Many words, short sense: make field declarations before accessor keywords mandatory, please.

@CyrusNajmabadi commented on Sun Feb 14 2016

I think that it would actually be a benefit to have the fields at the beginning!

I disagree. It's not uncommon to have people who put methods first, and then fields last. i.e. expose your functionality, and leave your state as the least important stuff at the end.

That said, perhaps it would be acceptable to just allow defining these locals above/below the accessors and not in between. Unlike other members you only have at most two accessors. So all you're missing out on is the ability to define locals between the two. I can get teh argument that it's good enough to be able to define them only above/below the accessors.

@HaloFour commented on Sun Feb 14 2016

@CyrusNajmabadi

For the sake of argument I might say that since these property-scoped fields have a look and feel more like locals that perhaps local declaration rules should apply in that an identifier cannot be used before it is declared. Given such it would only make sense to have said fields appear before or between the property accessor methods. Fields declared between the accessors could only be used from whichever the second accessor happens to be, which may have limited use-cases:

public string Name {
    string name;
    get { return name; }
    int initialized = 0;
    set {
        if (Interlocked.CompareExchange(ref flag, 1) == 1) {
            throw new InvalidOperationException("Already initialized!");
        }
        name = value;
    }
}

@bondsbw commented on Sun Feb 14 2016

The inner declarations do seem confusing. I don't have much of an opinion on declaring fields after accessors.

@bondsbw commented on Sun Feb 14 2016

@CyrusNajmabadi, have you thought about name collisions?

Some possibilities:

A. Property-scoped fields may have the same name as a class member, but NOT the same name as any local accessor variable. this. prefix is required to reference the class member, and no prefix references the property-scoped field.

B. Property-scoped fields may have the same name as a local accessor variable, but NOT the same name as class members. this. prefix is required to reference the property-scoped field, and no prefix references the local variable.

C. Property-scoped fields may have the same name as class members AND local accessor variables. After the local accessor variable is declared, the property-scoped field cannot be referenced. Traditional rules apply for this. prefix.

D. Property-scoped fields may NOT have the same name as any class members or local accessor variables.

I lean toward D, mainly because I think name collisions cause more problems than they solve.

@CyrusNajmabadi commented on Sun Feb 14 2016

For the sake of argument I might say that since these property-scoped fields have a look and feel more like locals that perhaps local declaration rules should apply in that an identifier cannot be used before it is declared. Given such it would only make sense to have said fields appear before

I really like that argument. Seems acceptable to then limit property scoped locals to the top of a property. It also removes the problems making the syntactic API changes. Nice!

@CyrusNajmabadi commented on Sun Feb 14 2016

I'm leaning toward 'A'.

I think of these as 'locals'. As such, like normal method locals, they can have the same name as a class field. But like method locals, they cannot collide with other locals in lower scopes.

Other things follow from this. Property scoped locals cannot be referenced with this.. this. is for access members of the class, and these are not class members.

@bondsbw commented on Sun Feb 14 2016

A was my second choice. I don't like B or C for the reason you mentioned, that these are specifically different from class members. Even if you argue that they somewhat are class members, the fact that they are not even unique within the class makes this. seem strange.

@lachbaer commented on Mon Feb 15 2016

I really like that argument. Seems acceptable to then limit property scoped locals to the top of a property. It also removes the problems making the syntactic API changes. Nice!

I had that idea in my mind as well already, but I feel a bit uncomfortable with them "thinking" of locals.

Locals are instanciated when the method is called. They must not be read uninitialzed. Now, when are they initialized first? Upon the first call of get or set? What is the initial value? Besides, what attributes are allowed and applicable? Field or variable attributes?

When I write s/getters, I - even before I cared about this issue - always thought of them as class methods, just with a bit of syntactic sugar to embed the logic (doesn't mean others must think that, too ;-)

Now, when they are going to look like locals, meaning also that the optical appearance would imply for you that they are locals, but then again they cannot be left unassigned and so an implicit initialisation always takes place before the .ctor is called, things really get too strange and fuzzy in my eyes. For any reason, the C# designers did think formerly that it is a good reason to leave Dim Static (not Dim Shared!) in VB and not bring it to C#, therefore being it a must to declare variables that keep their value throught the lifetime of an instance as a class field.

That's why I plead to let them, optically, be also field declarations, meaning also that access modifiers are valid in the first place, but then semantically restricted to private. All attributes of class fields are valid, aso.

As a side constrain, they must be declared before the accessors, because if others read your code they await the get and set keyword lying directly beside each other (same row or column). And in my eyes it's anyway a bad idea to put 'less important variables' at the end of a method (even Pascal knew so ;-)

Properties are somewhat of a special syntax in C# anyhow, because set implies a break; of the flow of that 'property method' (in quotation marks). So imo it is no problem to restrict the fields to

And ask the design team to rethink their doubts for Dim Static in C# ;-) Then you can put flag variables in the setter directly...

@lachbaer commented on Mon Feb 15 2016

@CyrusNajmabadi Again I want to point out that in my eyes it is no good idea to have too many possibilities in the property declaration, just short code. Maybe the above mentioned proposal is a better approach to complex scenarios.

@lachbaer commented on Tue Feb 16 2016

8364 goes back to defining a put keyword for semi-auto-property setters, that has a signature of T put(T value, ref T field);, i.e. they expect a return value to assign to the backing field.

Would it be an idea to allow the modifier keyword field (there already exists a reserved contextual keyword field) for one p-scoped field, to allow for using the put accessor in regular properties also?

public string VerboseName {
    field string _theName;
    get => _theName ?? "(noname)";  // #850
    put => char.ToUpper(value[0]) + value.Substring(1);
}

In the last example both, getter and setter ("slash putter") are used meaningful, so it is no (semi-)auto-property. The described syntax makes sense in this realistic use-case.

(PS: of course it can be done with the existing syntax, but as we are discussing p-scoped fields here, this is a legal proposal to respect some side issues before @CyrusNajmabadi starts coding :-)

@lachbaer commented on Tue Feb 16 2016

(Addendum: up above at [https://github.com/dotnet/roslyn/issues/850#issuecomment-183517965] I suggested the use of a property keyword. I just found a reserved property keyword in the Roslyn code already - seems that at Microsoft somebody must have thought about it as well ;-)

@HaloFour commented on Tue Feb 16 2016

@lachbaer property has been a contextual keyword since C# 1.0 for targeting attributes.

@lachbaer commented on Tue Feb 16 2016

@HaloFour Ah, I see. Never used that target before (only return, and assembly oc). Thanks!

@lachbaer commented on Sat Feb 20 2016

What do you think about following syntax for quickly defining a backing field?

public string FirstName {
    get _firstName = "";    // _firstName is 'private string _firstName = "";'
    set { _firstName = value ?? "";  }
 }

Scope of the field is property scoped, of course. Declaration of the field name could be either after the getter or setter. [This post is also in #8364]

@bondsbw commented on Sat Feb 20 2016

@lachbaer That syntax doesn't seem intuitive, and if my interpretation is correct it would limit property scoping to a single field.

I'd prefer

public string FirstName {
    var _firstName = "";
    get => _firstName;
    set => _firstName = value ?? "";
}

or, without something like #7881,

public string FirstName {
    var _firstName = "";
    get { return _firstName; }
    set { _firstName = value ?? ""; }
}

@lachbaer commented on Sat Feb 20 2016

As you can see you have 3 times written '_firstName'. I want to find a way to avoid that for lightweight properties, as in cases above.

@paulomorgado commented on Sat Feb 20 2016

@lachbaer, what if already have a _firstName defined?

@bondsbw commented on Sat Feb 20 2016

@lachbaer I understand that you're looking to make it more concise, which is a goal I prefer as well. But I don't prioritize conciseness over familiarity and consistency with the rest of the language.

If there is only going to be one field allowed, then it should probably just be a keyword. Then you could possibly do something like

public string FirstName { get; set => field = value ?? ""; } = "";

where get; performs like the auto-property get;, returning the value of field. The term set; would be shorthand for set => field = value;, in case you need custom getter logic. Combining those would give you today's auto-property initializer syntax:

public string FirstName { get; set; } = "";

Another shorthand possibility would be like what you mentioned in #8364, where the setter can be written to return an expression which sets the backing field:

public string FirstName { get; set => value ?? ""; } = "";

That just does the same as the first example, but honestly I'm not sure how much real-world use that has other than the null coalescing in this example. Property change notifications, which are a big reason people write setters, need to occur after the setting of the backing field since synchronous event handlers would otherwise read the wrong (old) value of the property.

Anyway, the use of a single unnamed backing field goes beyond the scope of this issue, and perhaps should be discussed elsewhere. It isn't necessarily incompatible with property-scoped fields, and I can see the value when it is applicable.

@lachbaer commented on Sun Feb 21 2016

@paulomorgado No problem. If this proposal comes through the name will be property scoped, otherwise class scoped.

@bondsbw The following I have alread proposed in #8364

public string FirstName { get; set => field = value ?? ""; } = "";

Where field is like value a compiler supplied parameter. Just the initialization cannot be done (custom setters cannot be initialized anyway). I only put the get _firstName = ""; in this proposal, because it might impact this implementation. For all other discussion about "short porperty notation", or "semi-auto-properties", as I call them, please go to #8364 (where I head over now to share some thoughts ;-)

@lachbaer commented on Wed Jul 06 2016

An alternative might be #12361. One block indention more, but more flexible.

@lachbaer commented on Sat Jul 09 2016

In terms of #12361 a property would look like

local uint _age;
public uint Age {
  get {
    local _age;
    return _age;
  }
  set {
    local _age;
    _age = value;
  }
}

Not very elegant. So in terms of this proposal #850 it could be made:

local uint _age;
public uint Age {
  local _age;
  get => _age: // #7881 syntax
  set => _age = value;
}

It is just one line of code more, but more consistent if contemplating on #12361 (wich goes a bit farther).

@alrz commented on Wed Sep 07 2016

Assuming that property blocks maintain their own scopes I think the following will be very common,

string P1
{
  string field;
  get => field;
  set => Set(ref field, value);
}
string P2
{
  string field;
  get => field;
  set => Set(ref field, value);
}

Which certainly is not dry. This is the main use case discussed in the OP where we just need a single field per property.

I think something similar to #7614 or #8364 is a more elegant way of doing this.

string P1
{
  get; 
  set => Set(ref field, value);
}

field will be a contextual keyword that refers to the corresponding field. Since currently get; is not permitted with a setter, there wouldn't be any backward compatibility issue, i.e. field is a keyword in the setter body only when you have get;.

Having said that, I think this would be superior to property-scoped fields because properties rarely require more than one backing field.

EDIT: However, it will come in handy when you don't have a setter.

public object Property => ComputeValue();

public replace object Property
{
  object field;
  get => field ?? (field = original);
}

@bondsbw commented on Mon Sep 05 2016

@alrz I don't think it has to be an either-or scenario. I mentioned the field keyword in the OP, but I do like the idea of being able to condense get => field; to simply get;, I think that would be useful.

@pebezo commented on Fri Sep 23 2016

I love the field keyword. 90+% of the time when I'm introducing a private field is to work around this -- and it always feels dirty. I feel it should be auto-generated (like value) and for more complicated cases we can always fallback to the current "private field" declaration(s). I like @bondsbw proposal the best. A few examples where I'd find it useful:

public string Phone { get => field.Formatted(); set => value.OnlyNumbers(); }
public string Lazy { get => field ?? (field = ReadValue()); }
public string Lazy => field ?? (field = ReadValue());
public int Counter { get; set => field + value; }
public string Log { get { Foo(field); return field; } set { Bar(value); return field = value; }
public string IfDifferent { get; set { if (field != value) { Foo(); } return field = value; }

@lachbaer commented on Sat Dec 17 2016

There are some issues with an ever existing field keyword.

  1. Should the compiler always create a backing field, for every property? => No
  2. It could break old code if someone used field as an identifier
  3. Checking all this during analysing time, if there should be a field or not is too heavy.

Though, I absolutely like the idea of not having a seperate backing field.

For the sake of this topic (#850) we should continue the discussion about field on topic #8364.

For this proposal, I think the 'cleanest' solution really is this:

class C {
    public string Prop {
        // property local declarations must come first, like locals in methods (no 'hoisting')
        int _field;    // no access modifiers allowed
        get => _field.ToString();
        set => Int32.TryParse(value, out _field);
    }
}

This would allow for many, many real world scenarios without too much overhead.

Internally this could be nicely converted to:

class C {
    private int <Prop>k__Field__field; // <{PropertyName}>k__Field_{identifier}
    public string Prop {
        get {
                ref int _field = ref this.<Prop>k__Field__field;
                {
                    return _field.ToString();
                }
        }
        set {
                ref int _field = ref this.<Prop>k__Field__field;
                {
                    Int32.TryParse(value, out _field);
                }
        }
    }
}

@paulomorgado commented on Sun Dec 18 2016

Why the ref?

What's wrong with just:

class C
{
    private int <Prop>k__Field__field;
    public string Prop
    {
        get
        {
            return <Prop>k__Field__field.ToString();
        }
        set
        {
            int.TryParse(value, out <Prop>k__Field__field);
        }
    }
}

?

@lachbaer commented on Mon Dec 19 2016

@paulomorgado The field identifier must be available to the user code. It's definition is just a SynthesizedLocal (with name) that does appear in the user code. I think that it will be lowered by the compiler to what you wrote down.

@paulomorgado commented on Mon Dec 19 2016

@lachbaer Other than explicitly declaring the backing field and using it on the getter and setter, what's the difference to auto-properties?

Auto-properties directly access the backing field without the need for a local ref variable.

@lachbaer commented on Mon Dec 19 2016

@paulomorgado I think your questions rather concern #8364 Semi-auto properties. And as you can see there, they are "semi" 😉

I think we should leave this issue/proposal with the intend of introducing property local fields. They don't conflict with #8364.

@lachbaer commented on Sun Apr 02 2017

@CyrusNajmabadi Have you already started on this feature? I would like to commence with development of my proposal of semi-auto-properties, that introduces a field virtual local for the accessors:

string Name {
    ref string field = ref <Name>k__backingField; // as virtual local
    get => field;
    set => field = value;
}

I think that it would be best to have an additional PropertyScopeBinder where this could be placed?

bondsbw commented 7 years ago

This is #133.

jcouv commented 7 years ago

Thanks @bondsbw Closing as duplicate.