dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.88k stars 4.01k forks source link

Proposal: 'readonly' for Locals and Parameters #115

Closed stephentoub closed 7 years ago

stephentoub commented 9 years ago

(Note: this proposal was briefly discussed in #98, the C# design notes for Jan 21, 2015. It has not been updated based on the discussion that's already occurred on that thread.)

Background

Today, the ‘readonly’ keyword can be applied to fields; this has the effect of ensuring that a field can only be written to during construction (static construction in the case of a static field, or instance construction in the case of an instance field), which helps developers avoid mistakes by accidentally overwriting state which should not be modified. Optimizations are also possible in a compiler based on the knowledge that the field is immutable after construction.

Here’s an example of a class defined with a readonly field. The ‘m_birthDay’ field is explicitly declared by the developer to be readonly, which means any attempt to set it after construction will cause a compile-time error. The ‘FirstName’ and ‘LastName’ properties, which are defined with getter-only auto-props (introduced in C# 6), also result in readonly fields generated implicitly by the compiler, since there’s no need for a field to be writable if there’s no way for code to set it.

public class Person
{
    readonly DateTimeOffset m_birthDay; // readonly field, assigned in constructor

    public Person(string firstName, string lastName, DateTimeOffset birthDay)
    {
        FirstName = firstName;
        LastName = lastName;
        m_birthDay = birthDay;
    }

    public string FirstName { get; }  // getter-only auto-prop, backed by readonly field
    public string LastName { get; }

    public TimeSpan Age => DateTime.UtcNow – BirthDay;
    public string FullName => "\{FirstName} \{LastName}";

    public DateTime BirthDay
    {
        get => m_birthDay;
        set => m_birthDay = value; // Error: can’t assign to readonly field outside of ctor
    }
}

Problem

Fields aren’t the only places developers want to ensure that values aren’t mutated. In particular, it’s common to create a local variable to store temporary state, and accidentally updating that temporary state can result in erroneous calculations and other such bugs, especially when such "locals" are captured in lambdas.

Solution: readonly locals

Locals should be annotatable as ‘readonly’ as well, with the compiler ensuring that they’re only set at the time of declaration (certain locals in C# are already implicitly readonly, such as the iteration variable in a ‘foreach’ loop or the used variable in a ‘using’ block, but currently a developer has no ability to mark other locals as ‘readonly’).

readonly long maxBytesToDelete = (stream.LimitBytes - stream.MaxBytes) / 10;
...
maxBytesToDelete = 0; // Error: can’t assign to readonly locals outside of declaration

This is particularly valuable when working with lambdas and closures. When an anonymous method or lambda accesses local state from the enclosing scope, that state is captured into a closure by the compiler, which is represented by a “display class.” Each “local” that’s captured is a field in this class, yet because the compiler is generating this field on your behalf, you have no opportunity to annotate it as ‘readonly’ in order to prevent the lambda from erroneously writing to the “local” (in quotes because it’s really not a local, at least not in the resulting MSIL). With 'readonly' locals, the compiler can prevent the lambda from writing to local, which is particularly valuable in scenarios involving multithreading where an erroneous write could result in a dangerous but rare and hard-to-find concurrency bug.

readonly long index = …;
Parallel.ForEach(data, item => {
    T element = item[index];
    index = 0; // Error: can’t assign to readonly locals outside of declaration
});

Solution: readonly parameters

As a special form of local, parameters should also be annotatable as 'readonly'. This would have no effect on what the caller of the method is able to pass to the parameter (just as there’s no constraint on what values may be stored into a ‘readonly’ field), but as with any ‘readonly’ local, the compiler would prohibit code from writing to the parameter after declaration, which means the body of the method is prohibited from writing to the parameter.

public void Update(readonly int index = 0) // Default values are ok though not required
{
    …
    index = 0; // Error: can’t assign to readonly parameters
    …    
}

This support for ‘readonly’ parameters would be particularly helpful for a very specific usage of parameters: passing structs by reference. When a reference type is passed to a method, the state that’s actually passed into the method is a copy of the reference to the object (the reference is passed by value), effectively a pointer-sized piece of data. In contrast, when a struct is passed into a method, a copy of the struct is passed in (the struct’s state is passed by value). If the struct is small, such as is an Int32, this is perfectly fine and there’s little better that could be done by the developer. However, when the struct is large (for example, the System.Windows.Media.Media3D.Matrix3D struct contains 16 doubles, making it 128 bytes in size), it can become quite expensive to continually pass around copies of the data. In these cases, developers often resort to passing structs by ref, passing a pointer to the original data rather than making a copy. This avoids the performance overhead, but it now inadvertenly enables the called method to update the original struct’s value:

public void Use(ref Matrix3D matrix)
{
    …
    matrix.M31 = 0; // Oops! We wanted performance, not more bugs.
    …
}

By enabling marking parameters ‘readonly’, the struct could be passed by reference but still prevent the callee from mutating the original value:

public void Use(readonly ref Matrix3D matrix)
{
    …
    matrix.M31 = 0; // Error: can’t assign to readonly parameters
    …
}

This would also enable readonly struct fields to be passed by ref (to a readonly ref parameter), whereas in C# today passing a readonly struct field by ref is never allowed. As with ‘readonly’ on fields, ‘readonly’ as it applies to locals and parameters would be shallow rather than deep, meaning that it would prohibit writing to the readonly field, but it wouldn't prevent mutation of any state contained in objects referenced from the ‘readonly’ value.

Additional Considerations

In many situations, in particular when "var" is used to declare a local with type inference, the developer often would like 'readonly' behavior for that local. This is possible if 'readonly' is allowed on locals, e.g.

readonly int foo = ...;
readonly var bar = ...;

but it makes the desired immutability harder to express than mutability. To combat that, as shorthand for 'readonly var', we could also introduce a new 'let' or 'val' syntax, with the same meaning as 'readonly var':

val foo = ...;
val bar = ...;
ryancerium commented 9 years ago

I'd like to allow the readonly modifier on classes to allow immutability by default, and methods also to correlate with C++ const functions.

readonly class Point
{
  public int x; // Implicit readonly
  public int y; // Implicit readonly
  Point(int x, int y)
  {
    this.x = x;
    this.y = y;
  }
}

Point origin = new Point(0, 0);
origin.x = 1; // Error:  can't reassign a readonly class member
drewnoakes commented 9 years ago

readonly always felt a little bit of a short sell for instances of types with mutable internals. The readonly-ness is much more valuable when transitive, as happens with (well behaved) C++ const values.

class C
{
  private int _i;
  public int Read() readonly { return _i; } // method has 'readonly' modifier
  public void Write() { _i++; }
}

readonly C c = new C();
c.Read();
c.Write(); // compile error: method is not readonly

The readonly modifier restricts the method's ability to modify its own members. Any members are also treated as readonly too, and only their readonly methods may be invoked. The entire graph of references from this point outwards becomes immutable.


As functional programming paradigms become more commonplace, function purity becomes an comforting property to have, and an appealing property to enforce. Today we have [Pure], but there is no guarantee made by the compiler that this attribute is used factually. A strict purity constraint would extend the above readonly guarantee to the method's arguments any any static values/references. Such a function could have no observable side effects and would be truly idempotent. A pure method modifier could exist that extends this immutability not only to this but to all parameters.

void PureFunction(C c) pure // method has the 'pure' modifier
{
  c.Read();
  c.Write(); // compile error: a pure function's parameters are implicitly readonly
}

Framework code could optimise for or even require pure functions. The compiler could automatically hoist loop invariant code.

Porges commented 9 years ago

Re parameters, I don't like the way that readonly on reference parameters pollutes the (in-source) signature with information that is irrelevant to the caller.

For structs, it would be nice to have those semantics - I've often wished for in as an addition to ref and out, as readonly ref is not very symmetrical :grin: It wasn't mentioned in the proposal, but the call site should not require annotation with in/ref.

ryancerium commented 9 years ago

@drewnoakes I completely agree, but backwards compatibility means we're screwed.

readonly var list = new ArrayList<int>();
readonly var size = list.size(); // Error, calling non-readonly method on readonly reference.
fubar-coder commented 9 years ago

@stephentoub The following example is ambigous:

public void Use(readonly ref Matrix3D matrix)
{
    …
    matrix.M31 = 0; // Error: can’t assign to readonly parameters
    …
}

This would also enable readonly struct fields to be passed by ref (to a readonly ref parameter), whereas in C# today passing a readonly struct field by ref is never allowed. As with ‘readonly’ on fields, ‘readonly’ as it applies to locals and parameters would be shallow rather than deep, meaning that it would prohibit writing to the readonly field, but it wouldn't prevent mutation of any state contained in objects referenced from the ‘readonly’ value.

The first thing that I thought, when I saw readonly ref was, that ref was used to avoid a copy and readonly was given, to avoid that the Use method changes it. Then I realized, that readonly was used to declare the underling structure as const.

My proposal to resolve this ambiguity is:

public void Use(readonly ref const Matrix3D matrix)
{
    …
    matrix = new Matrix3D(); // Error: can't assign to readonly reference
    matrix.M31 = 0; // Error: can’t change immutable value
    …
}

The variable is readonly and the Matrix3D value is immutable. It should also be possible to use the following syntax:

public void Use(ref const Matrix3D matrix)
{
    …
    matrix = new Matrix3D(); // OK
    matrix.M31 = 0; // Error: can’t change immutable value
    …
}

Calling methods on const objects

Calling methods on const objects should only be allowed when those methods don't make changes to the underlying object.

Examples

The following shouldn't work:

class TestObject {
    public int i;
    public void SetValueOfI(int v) {
        i = v;
    }
}

public void Use(ref readonly TestObject o)
{
    …
    o.SetValueOfI(0); // Error: can’t call non-const method on const object
    …
}

Instead we have to be able to make methods and properties (setter/getter) readonly too:

class TestObject {
    public int i;
    public int AddWithI(int v) const {
        return i + v;
    }
}

public void Test()
{
    var o = new TestObject { i = 1 };
    Use(ref o);
}

public void Use(ref readonly TestObject o)
{
    …
    Assert.AreEqual(124, o.AddWithI(123)); // OK: Can call AddWithI, because it doesn't change the underlying object
    …
}

I personally prefer const over readonly, because IMHO readonly means, that the variable can only be read, but this doesn't necessarily apply to the object it points to (similar to C's T const * const) and - to me - immutable variables and immutable values are different things.

drewnoakes commented 9 years ago

@ryancerium, I suppose this feature would require new metadata in the type system, therefore a new CLR and at the same time a new BCL. ArrayList<T> and friends would be annotated accordingly.

drewnoakes commented 9 years ago

@ryancerium can you further explore why the readonly modifier should apply to the type rather than the instance? The latter seems more flexible. You could still have:

readonly Point origin = new Point(0, 0);
origin.x = 1; // compile error: can't modify a readonly instance's fields
ryancerium commented 9 years ago

@drewnoakes readonly isn't transitive for fields, or for arrays for that matter. It was a mistake committed long ago to not have transitive readonly like C++ const, but it's where we are. Don't get me wrong, I much prefer it, but I think that ship has sailed.

I want to co-opt readonly to make it easier to make immutable data types. Right now I have to put readonly on all the data members, and that's annoying as hell :-)

Here's a different Point class that has mutable x and y values.

class Point
{
  public int x, y;
}

class C
{
  readonly Point origin = new Point(0, 0);
  public void M(readonly Point p)
  {
    origin.x = 100; // Totally fine in C# as-is today.

    p.x = 100; // Error per your suggestion, seems weird when I can modify origin

    readonly Point d = new Point(p.x - origin.x, p.y - origin.y);
    d.y = 100; // Error per your suggestion, seems weird when I can modify origin
    // d = new Point(-1, -1); // Illegal today, reassigning the reference
  }
}
louthy commented 9 years ago

Agreed on the desire for readonly classes, it would be very much appreciated.

I think the syntax for local readonly values should take a leaf out of the F# book and use let. The reason for that is I think val looks too close to var and would make it slightly less easy to scan code quickly. And I think trying to avoid a readonly prefix would be wise to reduce the amount of effort required to 'do the right thing' in choosing the immutable option when coding. As somebody who likes to make as much of my code as pure and as immutable as possible, I would much prefer not to have to litter my code with readonly.

    val foo = 123;
    var bar = 123;

Compared to:

    let foo = 123;
    var bar = 123;

I think the second version is much more 'scannable'.

stephentoub commented 9 years ago

The devil is always in the details on these kinds of proposals around immutability, const, etc. (i.e. when you start working through the intricacies, there are a variety of interesting gotchas and corner cases that need to be handled and increase complexity or at least acknowledged that there are holes.) I have several more proposals to post in this area, I just didn't get a chance to do so yesterday; I'll ensure they're linked to from here when I put them up, hopefully later today.

RedwoodForest commented 9 years ago

And I think trying to avoid a readonly prefix would be wise to reduce the amount of effort required to 'do the right' thing in choosing the immutable option when coding.

As somebody who also programs in Java which has final for readonly parameters and locals as well as fields, I always use it (when applicable) for fields but I often omit it from parameters and locals. I think let would be a good choice, and I would definitely use it.

ryancerium commented 9 years ago

The devil is always in the details on these kinds of proposals around immutability

@stephentoub Ain't that the truth. Is there a (very large) matrix anywhere that combines the list of types and methods and object locations (parameter/field/local/static)? Or do the language designers just keep them in the back of their head at all times?

struct S{}
struct<T> where T : struct GVS{}
struct<T> where T : class GRS{}
class C{}
class<T> where T : struct GVC{}
class<T> where T: class GRC{}
dynamic D{}

static class
{
  int a;
  S

Ah forget it, I'm getting tired already and realizing that the list is longer than War and Peace.

MgSam commented 9 years ago

I like this proposal, especially around using let to infer a readonly local. This makes the need to actually type the word readonly in the context of a method very rare, which is a good thing.

Richiban commented 9 years ago

I fully agree that let is the preferred syntax here, at least for local readonly variables / values. I believe that programmers should be encouraged into programming with immutability in mind, and having a nice, short keyword will facilitate this :).

Of course, let would work with type inference; you could simply swap var for let in most programs and you'd be fine. If you want to explicitly type the variable then you can much the same as in linq query expressions. So you should be able to write let john = "John" or let string john = "John" the same way in Linq you can write from name in names and from string name in names.

This also has the added benefit of matching both F# and Swift for syntax.

axel-habermaier commented 9 years ago

@Richiban: let john : string = "John" would be the correct F# syntax. let string john = "John" would declare a method called string, taking a parameter john of generic type, returning the constant "John".

However, I also agree that let should be preferred over val. The syntactic difference between val and var is just too small and easy to miss.

Richiban commented 9 years ago

@axel-habermaier Thanks, I was aware of the correct F# syntax. Perhaps I wasn't clear - let string john = "John" is what I want the C# syntax to be.

gafter commented 9 years ago

The "let" keyword would not read well for parameters.

BrannonKing commented 9 years ago

Is it possible for the compiler to determine that a struct is not modified in a method? If so, the struct could then be passed by ref to that method as a compile-time optimization. Obviously a readonly keyword would be a good clue that a struct should be passed by reference into that method. I don't see that you need both the readonly and ref keywords together.

louthy commented 9 years ago

@gafter

The "let" keyword would not read well for parameters.

How do you mean?

HaloFour commented 9 years ago

My two cents?

Support readonly as a modifier keyword for both parameters and local variables. Parameters with this modifier cannot be modified by the method nor can they be passed to methods as ref or out parameters. Local variables with this modifier must have an initializer and also cannot be modified by the method nor passed to other methods as ref or out parameters.

The behavior of this modifier will be exactly like the behavior of the readonly modifier when applied to fields. If the value of the variable is a reference type then any state carried by the instance may be modified through whatever members are exposed by that type, including public fields. If the value of the variable is a struct or value type then members which may mutate the value cannot override the value stored in the variable.

public class Box<T> {
    public T Value;
}

public struct Incrementor {
    private int value;

    public Incrementor(int initialValue) {
        this.value = initialValue;
    }

    public int Increment() {
        this.value += 1;
        return this.value;
    }
}

public void Foo(Incrementor i1, readonly Incrementor i2, readonly Box<string> boxed) {
    Debug.Assert(i1.Increment() == 1);
    Debug.Assert(i1.Increment() == 2);
    Debug.Assert(i1.Increment() == 3);

    Debug.Assert(i2.Increment() == 1);
    Debug.Assert(i2.Increment() == 1);
    Debug.Assert(i2.Increment() == 1);

    readonly Incrementor i3 = new Incrementor(0);

    Debug.Assert(i3.Increment() == 1);
    Debug.Assert(i3.Increment() == 1);
    Debug.Assert(i3.Increment() == 1);

    i3 = new Incrementor(5); // compiler error, i3 is readonly

    boxed.Value = "foo";
    Debug.Assert(boxed.Value == "foo");
    boxed.Value = "bar";
    Debug.Assert(boxed.Value == "bar");
    boxed = new Box<string>(); // compiler error, boxed is readonly

    readonly int number = int.Parse("4");
    int.TryParse("5", out number); // compiler error, number is readonly
}

Foo(new Incrementor(0), new Incrementor(0), new Box<string>() { Value = "baz" });

Additionally, as a counterpart to var also support the use of the existing let keyword as a method to declare implicitly-typed readonly variables. No type is permitted to be specified. This is syntactically similar to how let is already used in LINQ queries.

var i = 2;
let j = 3;

i = 4;
j = 5; // compiler error, j is readonly

Apart from the existing similar usage of let in C# it is also used by Apple Swift to define readonly variables, although that language does optionally permit explicitly specifying the type.

Richiban commented 9 years ago

I could get down with this syntax - if it's technically the addition of the readonly keyword but locals have the keyword let as shorthand for readonly var then you get the best of both worlds as far as I'm concerned.

The only thing I'm concerned about is the fact that requiring an extra keyword on method parameters will mean that nobody bothers. They should be writing:

public int Add(readonly int x, readonly int y) {...} but who's going to do that? Readonly parameters should be the default, but unfortunately that would be a breaking change and therefore has a close-to-zero chance of happening :(.

HaloFour commented 9 years ago

@Richiban

Readonly parameters should be the default, but unfortunately that would be a breaking change and therefore has a close-to-zero chance of happening :(.

Given that all parameters are by-val by default does that really matter? Let them screw with their version of x and y all they want, it won't affect the caller.

Richiban commented 9 years ago

@HaloFour No, you're right, the caller will be fine. I'm thinking more of reasoning the contents of a method. When variables are immutable by default it's harder to write spaghetti code. I've often seen code like this in existing projects:

public void DoSomething(Customer customer)
{
     ...
     if (someCondition)
    {
        customer = new Customer();
    }
    ...
}

This method is very hard to reason about (at least in the second half) because you don't know whether customer is still the object that was passed into the method or not. That's what I mean by the fact that I would prefer all parameters to be readonly. Reusing the customer parameter name as if it were a local variable is sheer laziness on the part of the programmer in this case, but if the language allows it and makes it easy then unfotunately that's what people will do.

mikedn commented 9 years ago

I've seen a lot of poorly written code but I've yet to see parameters modified in the middle of a method. Besides, there are valid cases to modify parameters:

public void DoSomething(Customer customer)
{
    customer = customer ?? new Customer();
    ...

You really don't want to introduce a new variable in this case because that means that someone my end up accidentally using the customer parameter instead of using the new variable.

MgSam commented 9 years ago

@Richiban If the method author wants to mess with the parameters and they were immutable by default, they would likely just add a mutate keyword (or whatever this hypothetical keyword would be) to allow themselves to do so. I don't think making it the default would accomplish anything.

I imagine in practice very few methods will have parameters actually designated as readonly. I think it only adds value on long methods and/or those with complex logic. And even then, its a weak guarantee as you can still write to properties or indexers of a readonly parameter.

Given they have such limited use, I'm not even sure readonly parameters are worth implementing at all... (Though I'm still a big fan of readonly locals).

Richiban commented 9 years ago

@MgSam Perhaps you're right. I'm very much in the F# way of thinking, and in F# you can't declare mutable parameters at all. To me it doesn't really make any sense why you would want them any way.

However, while I'm all for the ability to add the readonly keyword to method parameters (because "why not?") I think that in practice it will be rarely used since it's a) extra typing, b) extra noise to read c) of dubious value and d) will probably just lead to code style arguments amongst developers. Look at the final keyword in Java. People should really use it (for reasons I think we all agree with) but the extra keyword makes a surprisingly large negative impact on readability and thus very few people use it. That's why I think it's so important that we have the keyword let in C# for locals at least, because it's nice and short (val would be my second choice).

HaloFour commented 9 years ago

Of course the major reason to use final is the language requiring parameters/variables be final in order to reference them in an anonymous class. No such concerns in C#. It would exclusively be for developers to ensure that they don't accidentally overwrite the existing value. On Feb 6, 2015 10:39 AM, "Richard Gibson" notifications@github.com wrote:

@MgSam https://github.com/MgSam Perhaps you're right. I'm very much in the F# way of thinking, and in F# you can't declare mutable parameters at all. To me it doesn't really make any sense why you would want them any way.

However, while I'm all for the ability to add the readonly keyword to method parameters (because "why not?") I think that in practice it will be rarely used since it's a) extra typing, b) extra noise to read c) of dubious value and d) will probably just lead to code style arguments amongst developers. Look at the final keyword in Java. People should really use it (for reasons I think we all agree with) but the extra keyword makes a surprisingly large negative impact on readability and thus very few people use it. That's why I think it's so important that we have the keyword let in C# for locals at least, because it's nice and short (val would be my second choice).

— Reply to this email directly or view it on GitHub https://github.com/dotnet/roslyn/issues/115#issuecomment-73255483.

MrJul commented 9 years ago

I agree with @Richiban: readonly will in practice be applicable to most parameters, but won't be used because it's cumbersome to annotate every single parameter with it. It should be a default, but it's too late for that. Maybe as a project-wide option? VB had language options for years, C# not so much (except for arithmetic overflow checks).

Look at the previously mentioned Java's final, or VB's ByVal keyword (which isn't inserted by default anymore since VS2010 SP1 because it was too verbose).

@MgSam

I think it only adds value on long methods and/or those with complex logic

If you're smart enough to understand that your method is too complex and might need a readonly parameter, you're also probably smart enough to refactor it :) Corollary: you won't find readonly in code where it's really needed.

That said, I'm all for readonly locals, with a simple short keyword (my vote goes to let).

aluanhaddad commented 9 years ago

I am very much in favor of adding readonly local declarations. However, I would like to suggest that the keyword which is chosen to introduce a readonly, type inferred local be val and not let. The reason is that I feel it makes the method body read better and provides a nice symmetry with the existing var keyword.

To illustrate this, let us consider the let keyword (pun intended), which reads extremely well in the context of a query because it is used as a verb. Consider:

evenSquares = 
    from n in numbers        // from is a preposition
    let nSquared = n * n     // let is a verb
    where n % 2 == 0         // where is a preposition
    select nSquared;          // select is a verb

This reads wonderfully and is very symmetric.

Now let us consider the use of the var keyword, which can be thought of as a noun or noun modifier. Consider:

int ComputeSomething (IEnumerable<int> numbers) // C# 6.0
{ 
    var result = 0;          // var is a noun or noun modifier
    var offset = 25;         // this is not going to be modified 
    foreach (var n in numbers) // var is a noun or noun modifier
    {
        if (n % 2 == 0)
        {
            result += offset;
        }
        result += n;
    }
    return result; // return is a verb
}

This reads very nicely. Consider:

int ComputeSomething (IEnumerable<int> numbers) // C# where let declares a readonly local.
{ 
    var result = 0;          // var is a noun or noun modifier
    let offset = 25;          // let is a verb
    foreach (let n in numbers) // let is what?
    {
        if (n % 2 == 0)
        {
            result += offset;
        }
        result += n;
    }
    return result; // return is a verb
}

This does not have the nice symmetry of the previous but the ability to express immutability is wonderful.

Now consider:

int ComputeSomething (IEnumerable<int> numbers) // C# where val declares a readonly local.
{ 
    var result = 0;           // var is a noun or noun modifier
    val offset = 25;          // val is a noun or noun modifier 
    foreach (val n in numbers) // val is a noun or noun modifier
    {
        if (n % 2 == 0)
        {
            result += offset;
        }
        result += n;
    }
    return result; // return is a verb
}

This the best by far.

HaloFour commented 9 years ago

I don't think that the fact that let is a verb is that big of a deal. I think that you can read it more like binding the expression to a name in which case I think a verb is appropriate.

The two reasons that I don't care for val is that it is yet another keyword (C# 7.0 feels like a keyword explosion thus far) and that it looks so similar to var that when scanning through code you might not immediately notice the difference.

I like let because it's already a keyword in C# that behaves in a nearly identical way and because it's not without precedent. Apple Swift uses var and let in virtually the same manner as this proposal. EcmaScript 6.0 has both keywords as well although their use is different. F# has val and let where val is an unintialized declaration and let is a binding, regardless of mutability.

That said, I don't feel all that strongly either way,

gafter commented 9 years ago

@HaloFour @louthy let works well for local declarations, but it doesn't work so well in other contexts such as parameters where the initializing expression isn't given at the declaration:

    public static void Main(let string[] args) // let does not read well here
    {
    }

@louthy I don't think there is a readability issue with var vs val. To the reader of the program they mean the same thing: declare a local variable. The only difference is to the person who modifies the code, as val will place restrictions on what you can do.

HaloFour commented 9 years ago

I agree. let would only be a counterpoint to var for inferred variables. This would also be true if yhry decide to use val. Parameters would just reuse the existing keyword readonly, as could explicitly declared variables. On Feb 11, 2015 12:49 PM, "Neal Gafter" notifications@github.com wrote:

@HaloFour https://github.com/HaloFour @louthy https://github.com/louthy let works well for local declarations, but it doesn't work so well in other contexts such as parameters where the initializing expression isn't given at the declaration:

public static void Main(let string[] args) // let does not read well here
{
}

@louthy https://github.com/louthy I don't think there is a readability issue with var vs val. To the reader of the program they mean the same thing: declare a local variable. The only difference is to the person who modifies the code, as val will place restrictions on what you can do.

— Reply to this email directly or view it on GitHub https://github.com/dotnet/roslyn/issues/115#issuecomment-73927956.

gafter commented 9 years ago

@HaloFour so you suggest three keywords for the same concept? :/

Richiban commented 9 years ago

Not three - 'readonly' is the keyword proposed, and 'readonly var' can be shortened to 'let'.

On Wed, Feb 11, 2015 at 6:11 PM, Neal Gafter notifications@github.com wrote:

@HaloFour https://github.com/HaloFour so you suggest three keywords for the same concept? :/

— Reply to this email directly or view it on GitHub https://github.com/dotnet/roslyn/issues/115#issuecomment-73932216.

HaloFour commented 9 years ago

@gafter Not at all. I would reuse the existing keywords readonly and let. The readonly keyword could be applied to parameters or to formally declared variables:

public void Foo(readonly int x) {
    x = 5; // compiler error
    readonly int y = 10; // initializer required otherwise compiler error
    y = 15; // compiler error
}

The let contextual keyword would be used for inferred variables:

readonly int x = 10;
// equivalent to
let y = 10;  // again, initializer required otherwise compiler error
y = 15; // compiler error

I vote for let because C# already has the keyword with nearly identical syntax and behavior within LINQ projections, because it looks nothing like var and would not be mistaken as such when scanning through code, and because other languages already offer it as a precedent. In my opinion if you consider let to be a binding to an expression rather than a variable declaration it doesn't really matter that it is a verb.

I'm neither here nor there regarding readonly var. It's the tiniest bit more verbose and seems unnecessary if let or val shorthand is provided.

So really, my "proposal" is about identical to the original proposal in this thread, other than putting in my vote for let (over val) and expanding a little on my preference that these variables behave very similarly to readonly fields.

aluanhaddad commented 9 years ago

@HaloFour There is also a precedent for the val keyword in this context. Scala uses this syntax. Most Scala IDEs use syntax colorization to help the reader distinguish between var and val.

In F# let is used to bind readonly names to values inside function bodies, and let mutable is used to bind non readonly names to values inside function bodies.

Anyway, I do read let as binding a value to a name, which I think reads poorly in a context where var also binds a value to a name.

qrli commented 9 years ago

For parameters, no matter it is readonly or val, it is nothing related to the signiture of the method. It is more about the method implementation. Think about interface and virtual methods. So I think it may be better to be specified in method trait/contract.

aluanhaddad commented 9 years ago

@qrli I think that readonly should be used in method signatures, property signatures, and in any place where the type must be stated explicitly. This is consistent with its existing use in field declarations. My suggestion is that val be chosen as opposed to let as the keyword for declaring readonly implicitly typed local variables. The reason is that it reads better and is more symmetric.

qrli commented 9 years ago

@aluanhaddad If we are talking about "const" (as in C++) instead of "readonly", I will agree with you. But for "readonly", it is nothing about the external visible behavior of a method, but only for the convenience of a specific implementation. Enforcing it on a type system provides only trouble.

aluanhaddad commented 9 years ago

@qrli I think we agree.

paulomorgado commented 9 years ago

(sorry, but I haven't read all the comments with the required and deserved attention)

I do agree that having unmodifiable locals is very useful. For closures and code correctness.

But I think, like with fields, there are to use cases that could be leveraged: readonly and const locals.

A readonly local would be pretty much how @stephentoub specified.

A const local would be like a field const. const locals would be replaced by its value like the field equivalent.

As to syntax, I wonder if only the modifier could be used:

readonly x = GetX();
const y = 1;
var z = GetZ();    

It's a bit of a stretch, I know.

As for function parameters, taking the @stephentoub's use case of large structs, if I mark a by referenece parameter as readonly, I can't pass it along by reference without the assurance that it won't be modified. So the readonlyness must be surfaced to the caller. The suggestion of an in modifier seems like a good idea here. It will also give space for the combined use of in and readonly if such a use case surfaces.

And, wouldn't it be great if I could mimick the behavior of foreach and using?

var a = 1;
{
    readonly a; // _a_ is readonly in this scope.

    // use _a_ but don't use it.
}
a = 2; // _a_ is writable again.
whoisj commented 9 years ago

Let me start by espousing my love for this entire concept. The more we can place intent into the code, the less likely we are to create bugs, the easier it is to share/consume code, and the more verification we can push upstream to the compiler. All good things.

const'ness is always a delicate matter to design properly. The const keyword in C was great in its day, but the languages derived from C have evolved to the point where const isn't as flexible as it could be. C# has attempted to differentiate const'ness with a variety of options: const for compile time constants, readonly for immutable allocations, and the System.Collections.Generic.IReadOnly*<T> family. All good in their own right, but inconsistent as a whole.

This is a fantastic time to begin resolving the inconsistencies. For starters, in place of the IReadOnly*<T> famlily, why not enable readonly to be a modifier of the type definition of generics? example: List<readonly T> list = List<readonly T>();

So what are the complete set of use cases?

Immutable parameter values:

void Method(readonly object Obj)
{
    // ...
}

Immutable reference parameters:

void Method(readonly ref TimeSpan time)
{
    // ...
}

Immutable allocations:

private readonly object;
private readonly object[] m_objs;
private readonly List<object>

immutable arrays:

private readonly  char[] m_string = readonly { 'i', 'm', 'm', 'u', 't', 'i', 'b', 'l', 'e' };

Immutable generics:

public readonly List<readonly object> GetList { get { return ...; } }

Immutable local values:

for ( int i = 0; i < arr.Length; i++ )
{
    let x = arr[i]; // let means readonly
    ...
}

Finally, I think we should always allow references to be cast towards less mutability as a rule.

List<object> foo = new List<object>();
List<readonly object> bar = foo; // legal, less mutable
readonly List<object> baz = foo; // legal, less mutable
readonly List<object> biz = bar; // illegal, as T would be more mutable
HaloFour commented 9 years ago

As mentioned on #3202, the one issue with readonly ref when it comes to value types is that I don't think that there is any mechanism that would allow for the compiler to prevent calls to mutating methods. To the compiler they look like any other method defined on the value type.

static void Foo(readonly ref Point pt) {
    pt.Offset(2, 2);
}

static void Main() {
    var pt = new Point(2, 2);
    Foo(ref pt);
    // oops, pt is now {4, 4}
}
whoisj commented 9 years ago

Lastly, I'd bring up the idea of a writable keyword to allow partial undoing of readonly. There's plenty of times when I'd like to make a reference readonly but can't because current conventions are too strict.

Example, if for some reason (this is an actual use case for me currently) I need a fixed length dictionary where the keys of the dictionary are fixed (none should be removed or added) but users of the API are required to modify the values: today I need a custom type. With proper readonly and writable keyword usage I wouldn't.

public readonly Dictionary<readonly Control, writable Color> ControlColors { get { return ...; } }
HaloFour commented 9 years ago

@whoisj I don't see how your property definition defines a "read-only" dictionary. That seems to imply that the C# compiler would know a great deal more about the intent of the generic type arguments than it does. At best, the C# compiler could try to enforce that any parameter or return value of TKey would be a readonly Control, preventing use of property setters, but not much else. And since no such concept is baked into the CLR it couldn't be enforced at all.

whoisj commented 9 years ago

@HaloFour hmm... I suppose you have a point.

It would be great if there was a way to create an IReadOnlyDictionary<TKey, TValue> where the TValue was writable without the need to produce a new type completely. Thoughts?

drewnoakes commented 9 years ago

@ryancerium:

I want to co-opt readonly to make it easier to make immutable data types. Right now I have to put readonly on all the data members, and that's annoying as hell :-)

Have you changed your opinion on this after trying C# 6's truly readonly auto properties?

jonathanmarston commented 9 years ago

I think readonly on locals and parameters is a good thing.

I also really like the idea of being able to express that a method will not (and cannot) modify member fields and properties by applying the readonly keyword to methods.

On top of that, add a new modifier, immutable, that can also be applied to fields, locals, and parameters, that is different from readonly in that readonly specifies that the value of the field cannot be reassigned, while immutable specifies that the value (or referenced object) cannot be mutated.

The immutable keyword would add compiler checks that don't allow writing to fields, calling setters, or calling getters or methods not marked readonly (auto-properties would have implicit readonly getters). With the immutable keyword, you could effectively enforce immutability at the point where a type is used with a simple modifier, much like const in C++, but instead of the confusing const * const syntax, you could instead combine the readonly and immutable keywords. So "public readonly immutable MyType MyField" would be completely valid.

Essentially, to enforce the rules of a readonly method, the method body of a readonly method would only have access to an "immutable this" reference rather a normal, mutable "this".

This also gives the added benefit of possible runtime optimizations since you could safely pass value types to parameters marked readonly immutable by reference.

For example:

public class Person
{
    public string FirstName { get; set; } // Implicit readonly get
    public string LastName { get; set; }
    public void ChangeMe() { }
    public readonly string GetFullName { return FirstName + " " + LastName; }
}

public void SetPerson(Person person)
{
    person.FirstName = "John";
    person.LastName = "Doe";
}

public void WritePerson(immutable Person person)
{
    Console.WriteLine(person.FirstName); OK - calling a readonly getter is fine
    Console.WriteLine(person.GetFullName()); // OK - GetFullName is marked readonly
    person.FirstName = "Bill"; // Error - Can't call a setter on an immutable reference
    person.ChangeMe(): // Error - ChangeMe is not marked readonly
    SetPerson(person); // Error - can't pass an immutable reference to a method that takes a mutable reference
}

It gets a little more complicated when you have an immutable reference to an object that has a property of a complex type. To ensure immutability, the compiler would have to assume that all values obtained from an immutable reference, whether returned by field access, getters, or method calls are also immutable. There may be a need to override this for some methods (think factory methods) so a way to reverse the immutability may be in order (mutable would be an obvious keyword choice):

public class PersonFactory
{
    public mutable Person CreatePerson() { ... } // The person returned from this method would always be mutable, even if the reference to the PersonFactory is an immutable reference
}

Also, the immutable keyword would be a valid modifier for return types:

public immutable Person GetPerson(int id) { ... }
HaloFour commented 9 years ago

@binki This proposal only refers to the variables/parameters themselves, not to the references to which they point. If the parameter was of type readonly ref Point you'd still be able to mutate the struct via calling one of it's mutating methods like Offset.

You should probably look into #159, a proposal for immutable types.

binki commented 9 years ago

Ah, sorry. I was just looking at @jonathanmarston’s post which seems to be a proposal to change the definition of readonly to support const * semantics.

You should probably look into #159, a proposal for immutable types.

Hmm. It is hard to tell where my thoughts belong. That proposal seems, at least initially, to be more about building classes which are immutable than implementing const in C#. Bother, I don’t know if I have time to figure out even the right venue to place my idea :-/.

I do agree with the main tenant of this issue: that locals and parameters should be able to be declared readonly. This would be useful both when accepting structs as ref parameters and to control the mutability of fields in lambda capture implicitly-created objects. Even without these features, it can enable the programmer to protect him/herself from accidental mutation of variables that should only be set once. However, this issue is then also not a place to discuss changes to what readonly means. C# already has a well-accepted (if broken) meaning for readonly, and the changes that @jonathanmarston and I suggest should be considered separately.