C#: The compiler should be consistent about issuing error CS1612 when modifying readonly rvalue struct

[slepmog]

Version Used: VS 16.6.2

Abstract The C# compiler appears to be inconsistent about issuing CS1612 Cannot modify the return value of 'expression' because it is not a variable.

When the value being modified is a readonly struct, the compiler allows the "mutation" (apparently, on the basis that it cannot occur anyway, because the struct is readonly), but only if the struct was returned from a function. If the struct was returned from a property getter, the compiler issues CS1612, even though the getter-returned value is no more of an rvalue than a function-returned value.

Steps to Reproduce: Have the following code:

static void Main()
{
    var f = new S1Factory();

    // "Mutating" an rvalue struct returned from a function.
    // The compiler allows that, because no mutation can actually occur
    // due to the struct being readonly, so no reason to raise error CS1612.
    f.GetNewS1AsFunction().CausesSideEffectsWhenWrittenTo = 42;

    // Same, but this time the struct is returned from a property getter.
    // It is just as much of a readonly rvalue as above, but this time the compiler
    // raises CS1612. It shouldn't. Or, it should raise it for the above too.
    f.GetNewS1AsProperty.CausesSideEffectsWhenWrittenTo = 42;
}

readonly struct S1
{
    // Write-only property. Because the struct is readonly,
    // the property only exists for its side effects
    public readonly int CausesSideEffectsWhenWrittenTo {
        set {
            // Cause side effects. The struct cannot be mutated though
            // because it's readonly.
        }
    }
}

readonly struct S1Factory
{
    public readonly S1 GetNewS1AsProperty => new S1();
    public readonly S1 GetNewS1AsFunction() => new S1();
}

Expected Behavior: Both assignment lines in the Main() compile. Or, both don't compile - I am honestly not sure which one is correct.

Actual Behavior: The line with f.GetNewS1AsFunction() compiles, the line with f.GetNewS1AsProperty does not, which is inconsistent and confusing.

Note that if you remove the readonly modifier from struct S1 and property CausesSideEffectsWhenWrittenTo, then the compiler will correctly issue CS1612 on both assignment lines, because now that the struct is no more readonly, a call to the write-only property is allowed to mutate the struct, and because it's an rvalue, the error is correctly raised.

Original StackOverflow question: https://stackoverflow.com/q/62444352/

[carlreinke]

I'd prefer that both compile. One example use case is implementing fake indexed properties:

(Edited to show more alternatives.)

public sealed class Example
{
    private readonly int[] _indexedPropertyValues = new int[1];

    public IndexedPropertyValues IndexedProperty => new IndexedPropertyValues(this);

    public IndexedPropertyValues IndexedProperty2() => new IndexedPropertyValues(this);

    public ref int IndexedProperty3(int index) => ref _indexedPropertyValues[index];

    public IndexedProperty4Values IndexedProperty4 => new IndexedProperty4Values(this);

    public readonly struct IndexedPropertyValues
    {
        private readonly Example _this;

        internal IndexedPropertyValues(Example @this)
        {
            _this = @this;
        }

        public int this[int index]
        {
            get => _this._indexedPropertyValues[index];
            set => _this._indexedPropertyValues[index] = value;
        }
    }

    public readonly struct IndexedProperty4Values
    {
        private readonly Example _this;

        internal IndexedProperty4Values(Example @this)
        {
            _this = @this;
        }

        public ref int this[int index] => ref _this._indexedPropertyValues[index];
    }
}

public static class Program
{
    public static void Main(string[] args)
    {
        var example = new Example();

        example.IndexedProperty[0] = 1;  // CS1612

        ((Example.IndexedPropertyValues)example.IndexedProperty)[0] = 1;  // Okay (but ugly)

        example.IndexedProperty2()[0] = 1;  // Okay (but ugly)

        example.IndexedProperty3(0) = 1;  // Okay (but ugly and can't validate value)

        example.IndexedProperty4[0] = 1;  // Okay (but can't validate value)
    }
}

[CyrusNajmabadi]

Wouldn't it make more sense to write your property as:

        public ref int this[int index] => ref _this._indexedPropertyValues[index];

?

[carlreinke]

~That only works if you only have one.~ (Edit: Sorry, misread your comment.) ~Also~ if the indexed property is backed by a growable array, you might not want to hand out refs. You also wouldn't be able to do any validation in the setter, since there wouldn't be one.

[CyrusNajmabadi]

In that case, using a method seems sensible here to me.

[carlreinke]

Today, yeah, you either give up the indexer syntax and use a method or you give up being allocation-free and make the fake indexed property return a reference type or you give up the setter on the indexer and return a ref.

Given that the problem CS1612 is meant to protect against (mutating the copy of the struct returned by the property) can't happen when the returned struct is readonly, it seems reasonable for the compiler to not produce the error.

[carlreinke]

There was another alternative that I missed: return a ref from the property.

public sealed class Example
{
    private readonly int[] _indexedPropertyValues = new int[1];

    private readonly IndexedPropertyValues _indexedProperty;

    public Example()
    {
        _indexedProperty = new IndexedPropertyValues(this);
    }

    public ref readonly IndexedPropertyValues IndexedProperty => ref _indexedProperty;

    public readonly struct IndexedPropertyValues
    {
        private readonly Example _this;

        internal IndexedPropertyValues(Example @this)
        {
            _this = @this;
        }

        public int this[int index]
        {
            get => _this._indexedPropertyValues[index];
            set => _this._indexedPropertyValues[index] = value;
        }
    }
}

public static class Program
{
    public static void Main()
    {
        var example = new Example();

        example.IndexedProperty[0] = 1;
    }
}

This seems to me like the least-bad way to work around CS1612 for the fake-indexed-property use case.

[MrBrixican]

I'd also like to see both cases compile. The struct still remains immutable for all intents and purposes, so there is nothing gained by prohibiting this functionality. This enables a prime use case in easily revocable, immutable, garbage free handles for an object pooling management system, specifically in relation to high performance applications like games.

[CyrusNajmabadi]

@MrBrixican can you use methods in that case?

[MrBrixican]

@CyrusNajmabadi Ah, you are correct. I meant to say more easily enables. Properties are far more convenient wrappers around the typical getter/setter method combo (especially for +=, -=, etc) and significantly reduce the api surface a consumer would need to understand. Of course, methods serve the role well enough in the interim, but this issue should still be addressed seeing as properties are foundational to the C# language.

[Chicken-Bones]

I'd like to bump this one in the hopes of getting it out of backlog. I publish an API for a large community of users and this issue creates friction.

Simplified:

public readonly struct Tile
{
    private readonly uint _index;
    public byte TileColor { get => ...; set => ...; }
    ...
}

public readonly struct Tilemap
{
    public Tile this[int x, int y] => ...
}

// Cannot write
Tiles[x, y].TileColor = 0;

// Have to write
Tile t = Tiles[x, y];
t.TileColor = 0;

In other places we have been able to use ref returns with get only properties, but here we need an actual property.

[CyrusNajmabadi]

@Chicken-Bones In this case, using a method seems sensible. The above code really makes very little sense (both before and after). Tile being readonly really makes it enormously confusing that writing into a property has any meaning. This truly feels like an abuse of structs, indexers, and properties, as far as .Net and C# want them to be considered.

If you want to express that you can mutate, expose a method that can do such a thing.

[MrBrixican]

While I appreciate that a workaround is being offered, this issue was created with the basis that something that should be working, readonly struct pass-through properties that do not violate the immutable contract, does not work. Whether a design pattern is "abuse" or not, is not relevant. C# is a tool, and it is up to developers themselves to not misuse it.

Properties are a strength of C# and are designed with mutation in mind, hence setters existing. The sentiment that only methods should be used for mutation is something I'd expect more from a JVM or Javascript discussion, not dotnet, haha.

[CyrusNajmabadi]

Whether a design pattern is "abuse" or not, is not relevant.

It is very relevant. We've designed the language, and compiler rules, around the patterns we think are appropriate and which we think are not. That something could possibly be done still has to fall to the question of should it be done.

C# is a tool, and it is up to developers themselves to not misuse it.

This is not the philosophy of the language. We very much put up guards and prevent things if we feel that overall it would be a bad thing to allow.

I'd expect more from a JVM or Javascript discussion, not dotnet, haha.

C# has been designed this way since the beginning. Similar to other languages, we have a strong focus on trying to make good decisions for the ecosystem as a whole, and that often means preventing patterns we think would be problematic.

[Chicken-Bones]

In our case, the struct being readonly is a performance detail for the jit and compiler, and the properties accessing an external (structure of arrays) data store is an implementation detail not relevant to the user.

Users are not unfamiliar with structs being references to actual data (such as Memory)

[CyrusNajmabadi]

In our case, the struct being readonly is a performance detail for the jit and compiler,

'readonly' means that non-readonly methods will make copies. If you don't make your struct readonly, then you have no problem, as the compiler will glad mutate something non-readonly.

'readonly' isn't for performance. It's to actually ensure that it can't mutate.

and the properties accessing an external (structure of arrays) data store is an implementation detail

But it has to not be an impl detail. Because the only way this could work would be if the property doesn't mutate the value itself. And a setter which isn't actually mutating the thing you're calling on is the anti-pattern, and is much more cleanly and clearly expressed as a method call.

[Chicken-Bones]

Correct readonly prevents defensive copies being created which would serve us no purpose.

Originally, Tile was a class, and Tiles[x, y] always returned a reference to the same class. We had two issues with this approach, but performance and memory overhead was a primary concern. In an update ~2 years ago we changed Tile to a struct containing a single _index and moved the data into arrays. This eliminated object overheads, and improved cache coherency. readonly struct Tile is effectively a validated reference, or a smart pointer, etc.

Users understand that Tile being a struct does not mean the effects of their setters are discarded. They can continue to write code as if Tile was a class. Moving them to GetColor() SetColor(Color color) method pairs provides no additional readability or clarity.

I understand that seeing c.Color = value where c is a local copy of a struct raises some questions, but I believe that seeing c.SetColor(value) raises the exact same questions, and that these questions are quickly addressed by documentation.

I understand that we need to consider some subset of possible ways to write C# to be 'good/proper/intended', so I respect the time you're spending making this decision.

In my experience, one of the strengths of C# is that it provides tools to obtain better performance and implement closer to the metal, while retaining the safety of a managed language. I've always seen struct as primarily performance and native interop motivated, and recent focus on ref fields indicates that this is an opinion shared by the language team.

[CyrusNajmabadi]

but I believe that seeing c.SetColor(value) raises the exact same questions

It raises no questions for me. As methods are in the space of doing anything desired, whereas properties very much are designed such that they can be thought of as fields, just with computation being allowed on them.

Can you just expose this as a ref?

[carlreinke]

And a setter which isn't actually mutating the thing you're calling on is the anti-pattern

Implementing a mutable view using a readonly struct is not an anti-pattern. Span<T> is a readonly struct, and yet mutating something other than the struct itself is the reason that it exists!

[CyrusNajmabadi]

@carlreinke What mutable properties does Span have that you get CS1612 with?

[carlreinke]

@CyrusNajmabadi It seems you entirely missed the point.

[CyrusNajmabadi]

@carlreinke You seemed to be pointing at Span as an example of why something should be allowed. But Span itself doesn't seem to have any of the properties (no pun intended) that make it a suitable analogy for what's going on here.

[carlreinke]

I am pointing at Span<T> as an example of a mutable view implemented using a readonly struct, which is what you claimed is an anti-pattern. The only difference is that Span<T> uses a ref indexer (which is an indexed property) whereas CS1612 prevents using a ~non-indexed~ non-ref property for the same effect.

[CyrusNajmabadi]

I am pointing at Span as an example of a mutable view implemented using a readonly struct, which is what you claimed is an anti-pattern.

That's not what i claimed. What i said was:

And a setter which isn't actually mutating the thing you're calling on is the anti-pattern

and

As methods are in the space of doing anything desired, whereas properties very much are designed such that they can be thought of as fields, just with computation being allowed on them.

'Span' doesn't change things for me as 'Span' isn't doing anything off wrt statements.

[CyrusNajmabadi]

whereas CS1612 prevents using a non-indexed property for the same effect.

Correct. As i asked before: Can you just expose this as a ref?

That's what Span does here. If you want to be more like Span that's totally reasonable to me. An explicit ref access is def a way to indicate that you behave in this fashion.

[Chicken-Bones]

but I believe that seeing c.SetColor(value) raises the exact same questions

It raises no questions for me. As methods are in the space of doing anything desired, whereas properties very much are designed such that they can be thought of as fields, just with computation being allowed on them.

Can you just expose this as a ref?

We have done so for other members, but some of them require a property to perform bitpacking. Whether a property getter returns a ref or whether the property has a set method is only identifiable by interrogating the underlying member, not from reading code in plain text. The use cases are often similar, though it remains that neither refs nor properties are first class language citizens yet.

I understand your stance on properties behaving as fields rather than get/set methods. I think that allowing setters at all in readonly structs contradicts this, but perhaps it was too late to change.

I believe further agreement could only be obtained by consulting more perspectives, as C# caters to a diverse set of users with different expectations and use cases.

[carlreinke]

@CyrusNajmabadi Okay, fair enough, with Span<T> there is a ref along the way, but the API user doesn't see the ref in their code. Assuming you can build an API that using ref that's identical in usage to the API you wanted to build, what's the value that CS1612 is providing here? All it did was require you have something to ref that you otherwise might not have needed.

[CyrusNajmabadi]

All it did was require you have something to ref that you otherwise might not have needed.

To me... that is the value :)

It strongly pushes one down the path of only exposing in this fashion through either a method or a ref-property. Normal props are blocked off.

[dmitry-azaraev]

Generally ref-struct never should be used, unless you really have reasons and want to. When I'm meet this issue in 2020 - i'm owned in hands two readonly structs, which are copyable and valid all their lifetime, more over they always constructed on the fly, so they never directly stored anywhere.

There is impossible to put any sensible argue because language is just a tool, and it should prevent only technically invalid programs. It MUST not prevent to write valid programs. C# with CS1612 doing exactly this, e.g. prevent to write valid programs.

[MrBrixican]

I think we need to review some statements in this issue so far:

As methods are in the space of doing anything desired, whereas properties very much are designed such that they can be thought of as fields, just with computation being allowed on them.

This is not in line with the official C# language spec (§15.7.1):

However, unlike fields, properties do not denote storage locations. Instead, properties have accessors that specify the statements to be executed when their values are read or written. Properties thus provide a mechanism for associating actions with the reading and writing of an object’s characteristics;

Clearly, actions are very much included in the scope of property setters. Many dotnet classes make use of these "side-effects" for events/notifications. Look no further than some of the most used classes in enterprise settings: DataTable, DataRowCollection, and DataRow (source).

It has also been mentioned that a property setter that does not directly modify the item you're calling it on is an anti-pattern:

And a setter which isn't actually mutating the thing you're calling on is the anti-pattern, and is much more cleanly and clearly expressed as a method call.

On top of already contradicting §15.7.1 regarding no standardized storage location, this also contradicts with more Microsoft code:

• FileStream.Position setter redirects to FileStreamStrategy.Position (source)
• Console uses ConsolePal in setters and getters (source)

All of these were found after only a few minutes of searching, there are sure to be countless more contradictions.

---

All that is to say, code correctness is so subjective, it is not even worth discussing in this context. Let's please stay on topic, this issue is not about good code, anti-patterns, or best practices. This is about an inconsistency in the way readonly struct property setters are treated by the compiler:

internal class Program
{
    ReadonlyStruct PropertyReadonlyStruct { get; set; }
    ReadonlyStruct FieldReadonlyStruct;
    ReadonlyStruct GetReadonlyStruct() => PropertyReadonlyStruct;
    ReadonlyStruct this[int index] => PropertyReadonlyStruct;

    void Main()
    {
        FieldReadonlyStruct.Data = 0;     // Case 1: no issue
        PropertyReadonlyStruct.Data = 0;  // Case 2: CS1612
        GetReadonlyStruct().Data = 0;     // Case 3: no issue
        this[0].Data = 0;                 // Case 4: CS1612
    }

    public readonly struct ReadonlyStruct
    {
        public int Data { get => 0; set { /* no-op */ } } // Note no mutation of ReadonlyStruct instance
    }
}

Cases 1 and 3 currently work, and given that breaking changes are generally avoided they will continue to work in future. It only makes logical sense then, to maintain consistency, cases 2 and 4 should also work.

Regardless of my own bias, I think it's pretty obvious that removing this error in these cases will at the very least reduce confusion among developers which we've seen for the last 4 years.

However, I'm not on the boat of "fix it right now" and understand that the implementation to address this issue may be far more complex than many of us imagine. I just think it would be neat if this issue could be given a little more consideration. :)

[CyrusNajmabadi]

On top of already contradicting §15.7.1 regarding no standardized storage location, this also contradicts with more Microsoft code:

These do not contradict what i was saying. These are not readonly.

Properties thus provide a mechanism for associating actions with the reading and writing of an object’s characteristics;

This also doesn't contradict what i was saying. Associating actions with the ... writing of an object's characteristics. Writing to the characteristics of a readonly rvalue is something more likely than not to be a problem, so it is blocked (like with many other things blocked for similar reasons).

Transitioning to an API where the "more likely than not" doesn't hold (like ref-props, or just methods) sidesteps this, and makes the api less problematic.

All that is to say, code correctness is so subjective, it is not even worth discussing in this context

It is very worth discussing. Stating that you're unwilling to consider a core concern that is one of the reasons we have this behavior in the first place isn't beneficial to forward progress here.

Let's please stay on topic, this issue is not about good code, anti-patterns, or best practices.

Yes. It very much is. The idea here is to disallow a pattern that seems likely suspect. As with most disallowed patterns, this does mean some potential cases may get caught up in the net. But given that there appear to be a multitude of different API shapes that would work just fine here and be much clearer, that seems to be an acceptable thing.

[CyrusNajmabadi]

Cases 1 and 3 currently work,

Correct. The rules here model the sorts of api patterns that would be viable and appropriate in these cases. It's very intentional that hte cases that do not work have that behavior, and steering APIs away from working this way was intentional.

[CyrusNajmabadi]

There is impossible to put any sensible argue because language is just a tool, and it should prevent only technically invalid programs. It MUST not prevent to write valid programs. C# with CS1612 doing exactly this, e.g. prevent to write valid programs.

That's not how language design works for C# (or virtually any other language for that matter). And the language has many rules that are about blocking things that we technically could allow, but do not feel are good to allow.

For example, we do not allow 'collection initializers' on types that do not implement IEnumerable. We also don't allow you to write code like the following:

a + b; // Only assignment, call, increment, decrement, await, and new object expressions can be used as a statement

Yes, we could relax all these rules and allow this to be compiled, for the rare user that wants to do something like this. But that's not our philosophy. We will often block by default if we feel like this just isn't a good pattern. And if someone wants this to be legal, the onus is on the person wanting that to make a very strong argument that will convince us on hte lang design team to make this legal. :)

[MrBrixican]

These do not contradict what i was saying. These are not readonly.

However, it does contradict. §16 Structs gives no specifics on what a property is, thus I'm going to assume the intention is to fall back to §15.7.1 as per §16.3.

Interestingly, §16 does have a few addendums regarding struct properties, though:

§16.4.12.6 describes, in Microsoft's own words, that a getter or setter is, in fact, treated as a method call.

A value resulting from a method invocation e1.M(e2, ...) or property invocation e.P has safe-context of the smallest of the following contexts:

• caller-context.
• The safe-context of all argument expressions (including the receiver).

A property invocation (either get or set) is treated as a method invocation of the underlying method by the above rules.

§16.2.2 describes that readonly instance properties can't have a setter declaration. Either this rule is not being enforced since setters that respect immutability are allowed, or more likely, non-auto-property setters in such cases are not considered instance properties, but instead just convenient wrappers around a method body as suggested above:

A readonly struct has the following constraints:

• Each of its instance fields shall also be declared readonly.
• None of its instance properties shall have a set_accessor_declaration (§15.7.3).
• It shall not declare any field-like events (§15.8.2).

From the above we can conclude the following:

• Non-auto-property setters have no enforced or implied implementation regarding storage location
• Struct property invocations are synonymous with method invocations
• readonly struct non-auto-properties are just convenient wrappers around their method bodies

Thus, the distinction between methods and properties in this context is not aligned with language spec, and ultimately irrelevant.

---

As an aside, let's take a look at the CS1612 message:

Cannot modify the return value of 'expression' because it is not a variable

In my previous examples, was the struct ever "modified"? No. Byte for byte regardless of what you set the property to, the struct is exactly the same, as enforced by readonly. The error does not apply.

If there is a desire to enforce patterns, please create a new honest error with the following:

Cannot call set accessor on readonly struct property 'property' when returned from accessor, in order to prevent a perceived anti-pattern, not due to infeasibility. Please assign the struct to a local variable or field to ignore this guard entirely.

I offer this to you under MIT so feel free to use it. :)

---

As others have noted, this change is not something that affects the average developer, as struct implementations are rarely explicitly defined in the majority of C# enterprise code, readonly even less so. This is for advanced, niche, high performance use cases where it can be assumed the developer a) knows what they are doing and b) wants to offer a less off-putting/clunky user experience to api consumers.

In a language that allows operator overloading (particularly implicit operator) and unsafe code, there has obviously been a spirit of giving the developer the tools they need to do what they want with minimal barriers. The infantilization of the developer that is being pushed seems very contrary to what has made C# popular in the first place.

[CyrusNajmabadi]

escribes, in Microsoft's own words, that a getter or setter is, in fact, treated as a method call.

Yes. For the purposes of safe-context determination. That's not related to this convo though.

[CyrusNajmabadi]

None of its instance properties shall have a set_accessor_declaration (§15.7.3).

It looks like that is stale. @BillWagner for thoughts on fixing that up.

[CyrusNajmabadi]

In my previous examples, was the struct ever "modified"? No.

And that's why this is an antipattern. The code certainly gives that appearance. If you're going to have a type that behaves in that way (where it doesn't seem like it will mutate, but the code indicates it will), just don't use a non-ref property for that. :)

Non-auto-property setters have no enforced or implied implementation regarding storage location

That's why i said that this wasn't a question of whether something could be allowed, but rather should it be allowed. This falls into the latter discussion. I completely understand the arguments of making this just about "could this be allowed", but they're falling flat for me because (like other examples provided in this thread) i think the additional guardrails are good things. Indeed, the examples given so far have reinforced that belief for me due to how wonky they actually appear.

I think this thread is lacking in real world examples that demonstrate why this change would be good. So far, all the examples have made me actually feel like we don't go far enough in terms of blocking certain things.

[BillWagner]

None of its instance properties shall have a set_accessor_declaration (§15.7.3).

It looks like that is stale. @BillWagner for thoughts on fixing that up.

The ECMA spec is out of date. The committee is working on updating it.

Right now, the ECMA committee is working on the spec for C# 8. We haven't merged any of the initial PRs for those features. That's why the C# 8 speclets are still published. init only setters were added in C# 9. The init-only setters spec details what will change for readonly structs.

I admit this isn't ideal. We're continuing to work to catch up. But a long hiatus, and a (mostly) volunteer committee means progress takes time.

[MrBrixican]

And that's why this is an antipattern. The code certainly gives that appearance. If you're going to have a type that behaves in that way (where it doesn't seem like it will mutate, but the code indicates it will), just don't use a non-ref property for that. :)

I think this thread is lacking in real world examples that demonstrate why this change would be good. So far, all the examples have made me actually feel like we don't go far enough in terms of blocking certain things.

@Chicken-Bones has already given a good example as to why ref properties would not work, bit packing. Where the binary representation of data is not 1 to 1 with how the user should have to interact with it.

I have given the example of non garbage collected immutable handles that set and get values through interop, where you would clearly not want the value to be stored in the struct or have the ability to set the values directly via ref in the first place. Why would you not want to interact with verbose C getters and setters through beautiful, concise C# properties? Can you honestly say, as an api consumer, you would prefer methods over properties? Here's a more concrete example in an audio library I've been tinkering with. As an example, moving a sound in 3d space would go from:

this.Music.Position += new Vector3(1f);

to

this.Music.SetPosition(this.Music.GetPosition() + new Vector3(1f));

If you really think that methods are a better experience, either you're joking or you may find this link useful.

I'm not sure if we've not been clear enough or perhaps we need the input of Microsoft developers with more diverse backgrounds in development.

[CyrusNajmabadi]

If you really think that methods are a better experience, either you're joking

I'm not joking.

[CyrusNajmabadi]

Can you honestly say, as an api consumer, you would prefer methods over properties?

Yes, when dealing with readonly types and non-locations rvalues. But, personally, i wouldn't make these readonly.

[Chicken-Bones]

Yes, when dealing with readonly types and non-locations rvalues. But, personally, i wouldn't make these readonly.

This depends on what the purpose of the readonly modifier on types is. As it stands, there are multiple purposes which aren't fully aligned, the result of which is friction here.

1. Preventing the modification of the values of fields in the type. Providing a guard rail for developers working on the type.
2. Signalling to consumers of the type, that none of the properties or methods on the type can modify the values of its fields.
3. Allowing the compiler to eliminate defensive copies.

Purposes 2 and 3 are both derived from the core functional specification, but they come at odds here, otherwise I would be okay to remove readonly modifier from my struct.

Regarding purpose 2, the convention:

• I expect relatively few consumers of struct types will check the definition to see the readonly modifier.
• Unlike ref it has no effect or restriction on the behaviour of user code (outside of this warning). It can only be used to help reason about the underlying implementation of the type. Personally, I like to know the implementation of my types, not just their interface, but that's because I'm a low level programmer, not one who 'programs against the interface'
• Property setters on readonly types appear to initially pose a conflict, as you've described. "How can a readonly type have a setter which appears to modify it? Properties are like fields, and the fields can't be assigned to." As property setters in readonly types are not forbidden outright, the user may come to one of two conclusions:
  • The property setter mutates some other data, which is referenced by but not literally held in the struct (in a similar manner to assigning to a ref field), presumably the same data returned by the paired getter.
  • The property setter is unsafe/erroneous
  • or they may just remain confused.

As you've said, it remains the burden of the proposal champions to make a compelling use-case for opening up of a restriction.

I believe this restriction only really affects developers who are trying to make 'pointer-like' structs and are also concerned with performance because their structs are 'dereferenced' repeatedly in hot loops. This is exactly the ECS (or structure of arrays) game development pattern which has risen in popularity in recent years. You can see other developers with the same scenario and motivation in https://github.com/dotnet/csharplang/discussions/2068

If the C# language team believes that allowing setters in readonly structs at all was a mistake, then the only argument that can be made is that they're useful, but perhaps that's not worth the cost of muddying the convention.

Additional notes:

• Even if the compiler didn't eliminate defensive copies, the readonly modifier is still helpful to type authors in ensuring they don't accidentally mutate the struct when maintaining or adding members. Developers of 'lightweight native resource handles' may also wish to use readonly for this reason.
  • Developers who have only exposed properties on their struct may wish to add the readonly modifier, not realising that it will affect consumers of their type in the rvalue case, as there are no other observable usability side-effects.
• If the 'extension everything' or 'roles' proposals are eventually implemented, there would be further motivation for revisiting this limitation. Perhaps extension setters would be forbidden on readonly types, but ECS or other 'handle' structs would benefit heavily from allowing plugin authors to define extension properties on such handles when they also associate component data with those entities.

[CyrusNajmabadi]

I believe this restriction only really affects developers who are trying to make 'pointer-like' structs and are also concerned with performance because their structs are 'dereferenced' repeatedly in hot loops.

IMO, if you have such a situation, writing methods seems entirely cromulent :)

The benefit here is that if you have the normal property form the code looks entirely suspect/broken. In the examples, since a struct is returned, normal assumption is that that would be a copy. And so, mutating that copy is more often than not just a straight bug. Akin to getting a tuple as an rvalue and writing into it.

Methods much more clearly express: do unbounded work, including on data outside of the value. Properties that "call through" to some other store are the much less common case, and it seems fine to restrict those to catch the common case of this being a bug. Methods, on the other hand, allow you to express that something special can be going on, and have hte API stay aligned with the intent and clarity of the impl.

[Chryssie]

Methods much more clearly express: do unbounded work, including on data outside of the value. When talking about the rvalue though, the method is done and the impl of that method doesn't have a say on whether or not mutating the rvalue is valid or not.

That is unless if mutating the struct would not require a copy. Which, to that, it's worth noting that calling a readonly setter on a readonly field compiles whereas calling a non-readonly setter on a readonly field gives "CS1648: Members of a readonly field 'Program.ReadOnlyField' cannot be modified (except in a constructor or a variable initializer)".

class Program
{
    void Main()
    {
        ReadOnlyField.ReadOnlyData = default; // No error
        Property.ReadOnlyData = default; // CS1612: Cannot modify the return value of 'Program.Property' because it is not a variable.
        Method().ReadOnlyData = default;

        ReadOnlyField.DataProperty = default; // CS1648: Members of a readonly field 'Program.ReadOnlyField' cannot be modified (except in a constructor or a variable initializer)
        Property.DataProperty = default; // CS1612: Cannot modify the return value of 'Program.Property' because it is not a variable.
        Method().ReadOnlyData = default; // No error
    }

    readonly StructWithAField ReadOnlyField;
    StructWithAField Property => default;
    StructWithAField Method() => default;

    struct StructWithAField
    {
        public int DataProperty
        {
            get => default;
            set { }
        }
        public readonly int ReadOnlyData
        {
            get => default;
            set { }
        }
    }
}

In that case the error is different, CS1648 as oppose to CS1612, however, I don't think that impacts whether or not the code itself looks broken.

[CyrusNajmabadi]

/shrug :) Which errors get reported aren't really that important to me (though consistency across the scenarios would certainly be nice). What matters to me is the semantics we want to block and what we want to allow.