Allow invoking readonly members on structs in object initializers

[TahirAhmadov]

Imagine you have a lazy-instantiated collection, and a "proxy collection" to wrap around it:

using System;
using System.Collections;
using System.Collections.Generic;

var foo = new Foo
{
  Strings = { "abc" },
  Numbers = { 123 } // currently, this is CS1918
};
foo.Numbers.Add(456);
Console.WriteLine(string.Join(", ", foo.Strings));
// output: abc, 123, 456

class Foo
{
  public IList<string> Strings => this._list ??= [];
  public Wrapper Numbers => new Wrapper(ref this._list);

  List<string>? _list;
}
public readonly ref struct Wrapper : IEnumerable // currently, this is CS8343, but this will soon be possible
{
  public Wrapper(ref List<string>? list)
  {
    this._list = ref list;
  }
  public readonly void Add(int i)
  {
    (this._list ??= []).Add(i.ToString());
  }

  readonly ref List<string>? _list;

  //public IEnumerator<int> GetEnumerator() => throw new NotSupportedException();
  IEnumerator IEnumerable.GetEnumerator() => throw new NotSupportedException();
}

The gist of it is, the CS1918 shouldn't be generated, because the Wrapper.Add is readonly, therefore it's safe to invoke it as part of collection initializer. CS1918 should continue to be generated in all other existing cases.

[jaredpar]

This seems like a case for init on methods not changing the meaning of readonly. The use case you want here is essentially exactly what init provides today.

[TahirAhmadov]

I'm not sure what init has to do with this, can you please clarify?

[jaredpar]

The core behavior you want to achieve is to allow mutation of the object during the initializer but for it to be read only afterwards. This is exactly what init property accessors set out to solve. They are accessible in the ctor, other init members and object initializers.

The idea of init doesn't have to apply to only property setters. It's could also be extended to apply to method as well. This was considered when we did init accessors but decided to wait until we had more reason for need.

This seems like the most likely route to get this behavior.

[jaredpar]

Hmm ... missed a bit that you're using a ref field here. That changes it a bit.

[TahirAhmadov]

@jaredpar The objective here is to twofold: 1) allow making a struct typed property as a proxy for something else, like in this particular scenario; 2) bring the struct manipulation rules in object initializers more inline with rules outside of object initializers. I don't think it has anything to do with init keyword.

[timcassell]

This code doesn't make sense to me. Numbers property is a getter for a ref struct. How would you even be able to write that to a class? It's illegal even without CS1918.

It seems that you would want to have a collection expression work by adding elements to an existing collection, rather than creating a new one. I don't think that even makes sense with the assignment operator, and maybe needs something like a backward pipe operator <|?

[TahirAhmadov]

Warnings are breaking changes.

Outside of init blocks - yes, it's a breaking change, but inside init blocks, it's currently an error, so it's a relaxation of rules.
Like I said, I'm not going to argue for the breaking change outside of init blocks anymore; there is a separate discussion for that and there is no point to keep going in circles.

We want to get customers attention, we want them to consider their code and take action.

Same idea here - mark those members readonly and the warning goes away.

The use of readonly is an assertion that it's not (but it's not a guarantee).

Can you elaborate why it's not a guarantee?

The absence of readonly is not a guarantee or even affirmative statement that the modification is lost.

• For fields, those set values are guaranteed to be lost.
• For properties, they are not guaranteed, but are much more likely than not to be lost in most code (unless there is evidence there are a large number of structs with full body properties which make changes outside of the struct?)
• For methods, those are outside of init blocks, and this discussion is about relaxing/removing CS1918, not tightening the general rule.

PS. The first two bullet points above about fields and props are arguments for why inside init block, (not in general!) we should go from error CS1918 to warning for non-readonly members (fields and non-readonly properties, and non-readonly Add method for collections). But again, I'll be fine with full removal of CS1918, as well.

[TahirAhmadov]

To summarize:

• The general rule discussion is Make it an error to invoke non-readonly methods on a readonly field #8355; I'm not going to debate that here anymore.
• The init block discussion is here, the removal of CS1918. I'm happy with either
  • full removal of CS1918, (potentially with a MS-shipped analyzer for those who want it) or
  • with partial relaxation with warnings etc.

[jaredpar]

Same idea here - mark those members readonly and the warning goes away.

Customers do not own all the code they consume. Telling them just mark it readonly means a substantial amount of customers cannot follow your advice.

Can you elaborate why it's not a guarantee?

There are a number of ways that a readonly member can mutate this.

For fields, those set values are guaranteed to be lost.

Incorrect. When the fields are reference types, struct backed by reference type, using mutable statics, etc ... then the mutations persist.

For properties, they are not guaranteed, but are much more likely than not to be lost in most code

Properties are just method calls by another name. They are no different in terms of capabilities here.

For methods, those are outside of init blocks, and this discussion is about relaxing/removing CS1918, not tightening the general rule.

You insist on bringing readonly into this discussion. That means you have to discuss the behavior of method calls outside of collection initilaizers because that is exactly how the behavior is specified.

[TahirAhmadov]

Customers do not own all the code they consume. Telling them just mark it readonly means a substantial amount of customers cannot follow your advice.

For init blocks, previously it was an error altogether, so I'm not sure why it's such a big deal that they have to now deal with a warning.

There are a number of ways that a readonly member can mutate this.

Sure, but those are "hackish". I'm not saying it can be made airtight, nor should we try.

Incorrect. When the fields are reference types, struct backed by reference type, using mutable statics, etc ... then the mutations persist.

I meant setting those fields, not mutating referenced objects. The latter should be allowed, ditto for refs for structs.

Properties are just method calls by another name. They are no different in terms of capabilities here.

I know that 🤣 However, just common sense tells us that 99% of property setters modify the struct itself, either it's an auto property, or it sets a field, etc. That's why i said - if there are many properties with outside-of-struct side effects, I would like to see evidence of that - I just don't think that's the case. And again, currently all this is an error anyway, so....

You insist on bringing readonly into this discussion. That means you have to discuss the behavior of method calls outside of collection initilaizers because that is exactly how the behavior is specified.

Nope - I mention readonly for property setters and Add methods in the context of init block collection item adders, not regular method calls outside of init blocks. Again, I'm not debating the general rule outside of init blocks. I'm not. 🤣

[TahirAhmadov]

Once more - if you think full removal of CS1918 and completely bringing the init blocks behavior inline with the general rule outside of init blocks is the way to go, great. I'll take that and be a happy guy. 👍

[timcassell]

It's a different error, but it seems very closely related to #9174. @jnm2

[Edit] Ah, I see it was called out in #9175.

[jnm2]

Nice to find that it's not purely theoretical! Thanks for the crosslink making me aware of this thread.