Added: LDM Notes for October 27th, 2021

[333fred]

https://github.com/dotnet/csharplang/blob/main/meetings/2021/LDM-2021-10-27.md

Agenda

1. UTF-8 String Literals
2. Readonly modifiers for primary constructors

[jnm2]

We could also take primary constructors in a more radically different direction, where we'd adopt a more F#-like approach and consider these parameters captures, not fields.

This seems like a letdown. I'd still be repeating the same thing in two places instead of three. I've been looking forward to the day when this repetition would be gone. Today, much of the time that I open a .cs file, the first thing I see is similar to this:

class Foo
{
    private readonly A a;
    private readonly B b;
    private readonly C c;
    private readonly D d;

    public Foo(
        A a, // Normally these are not especially short names
        B b,
        C c,
        D d)
    {
        this.a = a;
        this.b = b;
        this.c = c;
        this.d = d;

        // Often further initialization, which is a different conversation
    }
}

What I was hoping for:

class Foo(
    A a,
    B b,
    C c,
    D d)
{
    // ...
}

However, scoping the feature down to just primary constructor parameters was slightly less controversial. Most members of the LDM are not opposed to the concept in general, but we remain conflicted as to whether we need to have such a feature immediately. While we did get a good deal of immediate feedback, some LDM members are unsure whether this feedback will persist after users actually get their hands on the feature and give it a try.

If readonly can't be the default, then putting readonly in front of each parameter is still vastly nicer for me than usually having to say A a, B b, etc a second time as field declarations.

It would be really interesting to see if I decide that I no longer care whether implicitly declared fields are marked readonly. From experiencing flip-flopping between let and const for locals in TypeScript, I already know I'm not interested in the ability to declare (non-primary) parameters and locals as readonly. The scope is not that big. Maybe this can extend to fields too. It could take a while for me to find out.

What could hurt is no longer noticing if I accidentally assign to a field instead of a local in one of the methods in the class, or extra time reviewing code having to keep in mind that each field is mutable. IDE/ReSharper have nagged to make things readonly right away for so long that I've lost all feeling, so I'm not sure how often these things would bite.

[jnm2]

Is mutability a good default? Here it looks like the field is being mutated, but the constructor parameter is being mutated. And which value of a is assigned to this.a?

class Foo(int a)
{
    private int b = a++;
    
    public int B => a++;
}

This could be a lot more subtle with structs.

class Foo(SomeStruct s)
{
    private int x = s.SomeMutatingMethod();
    private int y = s.SomeOtherMutatingMethod();
}

[qrli]

readonly is often a sensible default for locals and (in particular) parameters, but in practice LDM members do not find accidental local/parameter mutation to be a real source of bugs.

For single thread code, that's probably true. But for multi-threading code, readonly helps and give much more confidence. Well, I admit modern frameworks did a good job avoiding explicit multi-threading code in most daily work. So let's forget readonly local & parameters for now, but focus on readonly fields.

Unlike readonly local & parameter, C# already has readonly fields, for long. Refactor tools have been suggesting and auto-adding readonly to fields, for long. Now, if we were to have primary constructor without readonly fields, it feels like losing some feature.

[CyrusNajmabadi]

We shouldn't discuss "likely" as the purpose of these designs and discussions is to decide what we actually will ship. However, what we're discussing now is if it's necessary in the initial impl that we preview to support any sort of concept of 'readonly' or if that could, say, come in a future preview.

[jnm2]

@DavidArno Yes. If declaring the field manually, then I expect the syntax would need to be int x = x because this is how records work when you declare properties manually.

[qrli]

what we're discussing now is if it's necessary in the initial impl that we preview to support any sort of concept of 'readonly' or if that could, say, come in a future preview.

My understanding is that: excluding it from initial release effectively means the "readonly by default" option has been vetoed.

[CyrusNajmabadi]

It has not been vetoed at all :-)

[DavidArno]

@CyrusNajmabadi,

There will be some people who want the fields created from a primary constructor to be read-only and there will be others that want them to be writable. In both cases, the half that "luck out" will want a way of overriding that default behaviour to get the behaviour they want.

Now you might decide in the initial release to make the default behaviour the only way of doing things, eg by not supporting private readonly int x = x; notation. If you do go that way, then realistically your only option is to make them writable as those that want read-only can still use the feature. Sure, it will feel rushed as it'll be missing a key piece of functionality. but we can always work around that missing functionality by using attributes and an analyzer that checks for mutation. This approach gets the feature partially done quickly, allows it to be patched by the community in the short term and dangles the carrot of doing it well at a later date in front of our noses.

Or you might decide to allow the default behaviour to be overwritten. Being able to do:

public class C(int x, int y, int z)
{
    private readonly int _y = y;
    public Z { get; set; } = z;
}

allows developers complete control over what happens with those primary constructor parameters from the start. There's a small chance that this will close off future development of the feature though, which has to be taken into account.

[HaloFour]

but in practice LDM members do not find accidental local/parameter mutation to be a real source of bugs.

Until you capture them in a lambda expression in which case they become shared state where I have had to deal with bugs that result from accidental mutation. The team was forced to adopt a breaking change in the behavior of foreach specifically to address this class of bugs in one case.

We could also take primary constructors in a more radically different direction, where we'd adopt a more F#-like approach and consider these parameters captures, not fields.

I'm not 100% sure I grok this statement. If this results in the parameter not being accessible anywhere except field initializers then you would have succeeded in partially mitigating the mutation issue (although not entirely) but at the expense of making primary constructors pretty much entirely useless.

Quite a few languages have modifiers to indicate that parameters and/or locals should be readonly, including C++. I don't see why this is remotely controversial. There is value in declaring intent and having it enforced by the compiler even if it doesn't otherwise affect the behavior of the code.

[alrz]

"I fought for that when I was young and idealistic"

I can only guess what this is about. Have we given up the runtime flag to opt into having all System.String instances just become UTF-8 under the hood?

[333fred]

Unrelated, sorry 🙂

[orthoxerox]

We could also take primary constructors in a more radically different direction, where we'd adopt a more F#-like approach and consider these parameters captures, not fields.

The big difference is that F# record members are immutable (readonly in C# terms) by default.

[DavidArno]

... we took a look at another longstanding C# proposal, allowing readonly on parameters and locals. Several LDM members are concerned about this as a general feature, mainly because of the "attractive nuisance" issue

I'm not familiar with the term, "attractive nuisance". But a search of the term suggests it's the concept of something being attractive to children that can cause them harm. By that definition, things like mutable by default and classes being unsealed by default are examples of "attractive nuisance". But the above quote seems to suggest some of the LDM see "pit of success" features such as readonly locals as an example of "attractive nuisance", so I'm genuinely confused by this line. Have I misunderstood the term?

[CyrusNajmabadi]

I would actually prefer a diagnostic which I could set as compilation warning, which would flag all mutations for me. Then I could investigate such places in code

This is already possible today. Just write an analyzer that checks this for your own code.

[DavidArno]

Methods can already be marked readonly in other contexts. It does not mean the list of parameters is readonly. So it would be very inconsistent to use the same syntax to mean something else now.

Good point.

[CyrusNajmabadi]

It might be possible to do something like:

void Add(readonly: int x, int y, int z)

though.

[qrli]

Or, enable to opt-in readonly-by-default at class level:

public readonly_by_default class Foo
{
  private Bar bar; // readonly by default
  private mutable int counter; // opt-in mutable
  public Foo(Bar bar) // parameter is readonly by default
  {
    this.bar = bar;
  }
  public void DoSomething(int a, int b, ref int c) // parameters are readonly by default, except ref/out
  { ... }
}

[smoothdeveloper]

I also find readonly modifier being the most logical, and like @CyrusNajmabadi proposal; @CyrusNajmabadi, in your example, is it only x which is readonly or the feature applies to the whole signature?

I am curious if we should expect "no change in generated IL" in code which would still compile the same after the annotation is added or if I'm already missing something there.

• should the compiler error if readonly is applied to ref or out parameters?
• should the compiler warn if readonly is applied to in parameter?

[Richiban]

Important note: Please don't take the following too seriously.

I find the whole readonly parameters and locals issue just maddening.

It's like the LDM is a car manufacturer and the users are asking for the next version of their cars to come with turn signals:

Users: Can we please have turn signals on the car? That'd be great, thanks.

LDM: Why do you want turn signals on the car? They don't actually enable the car to do anything.

Users: I know, but they're really great for declaring your intent, and other road users really appreciate that. It makes it much easier for others to understand what you're doing.

LDM: The LDM doesn't see the value in adding turn signals. They don't actually prevent accidents.

Users: Well, hang on! There are definitely some senarios where they would. Plus, you can see that pretty much every other brand of car comes with turn signals so they must be a good idea.

LDM: We're not sure about this. The C# car is a turn signals-off-by-default vehicle, therefore we don't think we should even put turn signals on the car.

Users: I'm not sure I understand that argument. Again, almost every other brand of car designed in the last ten years comes with turn signals. It's starting to get frustrating that C# doesn't.

[Some time passes]

LDM: Okay, we've decided not to put turn signals on the car because we can't decide what colour they should be.

Users: I really don't mind what colour they are. Can we please just have them?

LDM: The LDM doesn't see the value in adding turn signals.

[Users' collective heads hit collective desks]

[HaloFour]

Because we can't have enough car analogies:

readonly is like seatbelts. Both are introduced much later. Both are explicitly opt-in and require additional effort on the part of the user. Both have little/no impact on operation. Both are rarely actually put to use, but in those cases when they are they can avoid catastrophic consequences. Both are worth it.

[bernd5]

Good analogy!

[TonyValenti]

I don't have insight into why a Utf8String type would be requested, but I personally think that the internal storage format of a string is an implementation detail that shouldn't be leaked.

When working with interop scenarios, I think I would prefer to see something like a [StringFormat(StringFormats.Utf8 | StringFormats.NullTerminated)] applied to returns or method parameters and have the conversion happen automatically and have a "ToBytes" extension method that would take similar parameters to the above attribute.

[jnm2]

How can you avoid the leaking? There are some things you can represent with a single char that have no equivalent in UTF8, like half of a surrogate pair, and there are UTF8 characters that require more than two bytes to represent and can't fit in a char.

Also, string indexing/slicing is no longer random access if there is conversion going on transparently.

[CyrusNajmabadi]

but I personally think that the internal storage format of a string is an implementation detail that shouldn't be leaked.

This would have been a nice idea to follow in .net 1. However, we're in the position caused by taking the opposite route. With strings it's very much not an internal detail. The nature of it being a contiguous sequence of 16 bit characters shows all over the place. And there is a ton of code that depends on this.

I'm a huge proponent about allowing the runtime to change the internal representation. However, it will be massively impacting if we ever do so given the dependencies people have on the current form.

[koszeggy]

I don't have insight into why a Utf8String type would be requested

I believe the main motivation here is the new way of JSON parsing by System.Text.Json, which is optimized for UTF8 strings, and has a lot of members with ReadOnlySpan<byte> parameters. Ironically, this new parser is blazingly fast when you parse a JSON content but only as long as you don't want to access anything by a string name, in which case it suddenly becomes slower even than Newtonsoft.Json.

I personally think that the internal storage format of a string is an implementation detail that shouldn't be leaked.

As @jnm2 also mentioned actually the UTF16 nature of string is also leaked. But I agree with you about that I wouldn't really like to pollute the language with a new 'alternative string' type. I would rather see an implicit conversion from a C# string literal to ReadOnlySpan<byte> and maybe #5295 can also help to use such strings as constants. Then it would be possible:

const ReadOnlySpan<byte> myUtf8String = "Some UTF8 string initialized from a C# string literal";

And then System.Text.Json could also work without introducing a bunch of new overloads with some new Utf8String type, so the already existing API could just work seamlessly.

📝 Note: Of course, if both string and ReadOnlySpan<byte> overloads exist, then using a string literal should pick the string overload so I support the idea of some u8 postfix for this case even though I don't prefer introducing some new Utf8String type.

[huoyaoyuan]

Currently, the runtime does not have a real Utf8String type, and instead uses byte[], Span<byte>, or ReadOnlySpan<byte> as the de-facto interchange type for UTF-8 data.

I'm interested why the runtime team doesn't accept it. The proposal and experimental implementation have existed for years.

[koszeggy]

I mentioned some reasons why I personally would not like it (confusion, unnecessary "pollution", the need of a lot of new overloads). What are your reasons to introduce such a type?

[huoyaoyuan]

A slight pain: ReadOnlySpan<byte> can't be used on heap, and ReadOnlyMemory<byte> or byte[] isn't as efficient/clear as string.
I don't use UTF8 too much though.

[koszeggy]

As for efficiency, I see it a bit differently: since we have spans, almost all public APIs have been refactored to work on [ReadOnly]Span<char> and even the string overloads are redirected to the span implementation. Such an API never needs to know whether there is a real string in the background. And spans are much more efficient/flexible as they can mean arrays, strings, some unmanaged memory or even stack allocated buffers. Introducing a Utf8String with a heap allocated array-like storage would just be a downgrade from spans, IMHO.

I risk to say that maybe we wouldn't even have string now if we had spans since C# 1.0. Completely new APIs often have only the span overloads that can be cast implicitly from strings anyway.

One painfully missing feature is the support of compile time constants (eg. in attributes, switch cases, parameter default values) but it would not come even for a Utf8String without some special handling. And hopefully #5295 will solve this issue.

A slight pain: ReadOnlySpan<byte> can't be used on heap

This is true, that's why we have the ReadOnlyMemory<byte> and ArraySegment<byte> types (or the plain old array) and you can pass all of these to span parameters easily.