Unexpected inconsistency in records
codeblog.jonskeet.uk
codeblog.jonskeet.uk
The first is to use a `With` method and rely on "optional" parameters in some sense. When you write `with { x = 3 }` you're basically writing a `.With(x: 3)` call, and `With` presumably calls the constructor with the appropriate values. The problem here is that optional parameters are also kind of fake. The .NET IL format doesn't have a notion of optional parameters -- the C# compiler just fills in the parameters when lowering the call. So that means that adding a new field to a record would require adding a new parameter. But adding a new parameter means that you've broken binary backwards compatibility. One of the goals of records was to make these kinds of "simple" data updates possible, instead of the current situation with classes where they can be very challenging.
The second option is a `With` method for every field. A single `with { }` call turns into N `WithX(3).WithY(5)` for each field being set. The problem with that is that it is a lot of dead assignments that need to be unwound by the JIT. We didn't see that happening reliably, which was pretty concerning because it would also result in a lot of allocation garbage.
So basically, this was a narrow decision that fit into the space we had. If I had the chance, I would completely rework dotnet/C# initialization for a reboot of the language.
One thing I proposed, but was not accepted, was to make records much more simple across the board. By forbidding a lot of the complex constructs, the footguns are also avoided. But that was seen as too limiting. Reading between the lines, I bet Jon wouldn’t have liked this either, as some of the fancy things he’s doing may not have been possible.
My biggest sadness reading this is that what MS have done is to outsource the issue to all C# devs. We will all hit this problem at some point (I have a couple of times) and I suspect we will all lose hours of time trying to work out WTF is going on. It may not quite be the Billion Dollar Mistake, but it's an ongoing cost to us all.
A possible approach I mentioned elsewhere in the thread is this (for the generation of the `with`):
var n2 = n1.<Clone>$();
n2.Value = 3; // 'with' field setters
n2.<OnPostCloneInitialise>(); // run the initialisers
public virtual void <OnPostCloneInitialise>()
{
base.<OnPostCloneInitialise>();
Even = (Value & 1) == 0;
}
That would just force the new object to initialise after the cloning without any additional IL or modifications.
Let's be clear: breaking dozens of tools because of a change to the IL format also outsources an issue to all C# devs. The .NET IL format has been basically unchanged since .NET 2.0 and huge numbers of people take very hard dependencies on the exact things they do and do not expect. I don't expect we would have been able to make significant changes due to the breaking change impact.
> A possible approach I mentioned
This would likely be even harder to understand. For better or worse, the .NET design is that external initializers happen _after_ the constructor runs. That's been true all the way back to when the initializer syntax was first introduced in C# 3. Making regular initializers and `with` initializers have inverted order strikes me as being way worse.
If I could go back in time, I think the main change to C# I would make would be to enforce that the constructor always runs after all external initialization.
What am I missing?
Internal initializers are run as part of the constructor, before any user code. External initializers are run after the constructor, on the constructed object.
For instance:
class C
{
public int P = 5;
}
var c = new C { P = 3 };
In your example:
var n2 = n1.<Clone>$();
n2.Value = 3; // 'with' field setters
n2.<OnPostCloneInitialise>(); // run the initialisers
If changing the schema isn't reasonable, use a copy constructor instead.
It isn't, that's why there's a blog article documenting how unexpected it is.
> that depend on mutable fields
The fields are not mutable. The `with` expression creates a whole new record, clones the fields, and then sets the field you're changing (the field is read-only, so this is the compiler going 'behind the scenes' to update the new record before it sets the reference). The reason for all this is performance: the new structure is allocated on the heap, a memcopy happens (old structure copied onto the new), and then the `with` changes are applied. It's just at this point the 'init time' properties aren't run on the new object.
In the language the fields are immutable. So, the argument is that a whole new record initialised with the fields of the old record (with some changes) should run the 'init time' properties so that they get set too, otherwise the data-structure can become inconsistent/poorly-defined.
> use a copy constructor instead
It's probably worth reading the article:
"Note that because Value is set after the cloning operation, we couldn’t write a copy constructor to do the right thing here anyway."
The behaviour is expected for the language design. Whether developers using the language expect it is a separate matter.
The with operator clearly allows someone to break encapsulation and as such should only be used in cases where you aren't expecting encapsulation of the underlying record.
> It's probably worth reading the article:
> "Note that because Value is set after the cloning operation, we couldn’t write a copy constructor to do the right thing here anyway."
It's probably worth reading the entire article, as that quote is followed by: "(At least, not in any sort of straightforward way – I’ll mention a convoluted approach later.)", which presumably was what was being referred to there.
In general, there's a whole ton of gotchyas around encapsulation of precomputed values. That's just life outside of a purely functional programming context.
So, Microsoft meant it. Ok...
> Whether developers using the language expect it is a separate matter.
Really? Perhaps read the 'Principle of Least Astonishment' [1] to see why this is a problem. If I create a new object I would expect the 'init time' properties to be initialised.
> It's probably worth reading the entire article, as that quote is followed by: "(At least, not in any sort of straightforward way – I’ll mention a convoluted approach later.)", which presumably was what was being referred to there.
It's probably worth continuing to read the article. Because the attempt to deal with it required manual writing of Lazy properties:
private readonly Lazy<ComputedMembers> computed =
new(() => new(Value), LazyThreadSafetyMode.ExecutionAndPublication);
> In general, there's a whole ton of gotchyas around encapsulation of precomputed values. That's just life outside of a purely functional programming context.
Great insight. Let's not run the 'init time' properties for a newly initialised object, just in case it works as expected. This 'feature' can't even be manually resolved by doing post-`with` updates (because often the properties are init/read-only). It makes the whole init-property feature brittle as fuck.
[1] https://en.wikipedia.org/wiki/Principle_of_least_astonishmen...
The Principle of Least Astonishment definitely applies. It's a deliberate design choice that unfortunately violates the principle.
> Great insight. Let's not run the 'init time' properties for a newly initialised object, just in case it works as expected. This 'feature' can't even be manually resolved by doing post-`with` updates (because often the properties are init/read-only). It makes the whole init-property feature brittle as fuck.
? I'm not sure I follow what you are going with here, but yeah, in general you'd have to carefully limit all uses of "with" for objects with precomputed values to inside the encapsulation of said objects. Alternatively, as you mentioned, you could just not have precomputed properties.
What you’re describing is incidental complexity. It is not a good thing. We can’t constrain the complexity with language constraints, we have to rely on the programmer following some guidance and never ever making a mistake. In large teams and large code-bases that's a problem. And it's an offloaded problem from Microsoft to every software development team on the planet that uses C#.
The incidental complexity for the average C# developer has increased, whereas a better direction of travel would be toward correctness and declarative features. I would prefer it if the csharplang team worked toward that.
Yes.
This muddies the water between just setting a field vs executing a function that does work and then sets the field.
If I write a record with an explicit setFoo(foo: Foo) I wouldn't expect a clone & subsequent direct field assignment to execute the setFoo(foo: Foo) code.
If I create a new object I would expect the 'init time' properties to be initialised. Regardless of how it was initialised. The current approach just leads to inconsistent data structures, with significant issues for debugging how a data structure got into an inconsistent state. Modern language features should not be trying to save nanoseconds like they did in the past. Or, should at least default to 'correct' with performance opts outs.
using System;
// same behavior with
// public sealed class Inner
public struct Inner
{
public int Value { get; set; }
}
// same behavior with
// public sealed record class Outer(Inner Inner)
public record struct Outer(Inner Inner)
{
public bool Even { get; } = (Inner.Value & 1) == 0;
}
class Program
{
public static void Main(string[] args)
{
var inner = new Inner { Value = 42 };
var outer = new Outer(inner);
inner.Value = 43;
Console.WriteLine("{0} is {1}", inner.Value, outer.Even);
}
}
Records were a relatively recent feature addition, computers are significantly more powerful, and our programs are significantly more complex. And so it’s hella frustrating that MS didn’t opt for program correctness over performance. They even missed the opportunity to close the uninitialised struct issue with record structs (they could have picked different semantics).
I find their choices utterly insane when we’re in a world where bad data could mean a data breach and/or massive fine. The opportunity to make records robust and bulletproof was completely missed.
I was hoping that we’d get to a set of data-types in C# that were robust and correct: product-types and sum-types. With proper pattern matching (exhaustiveness checking).
Then either a Roslyn analyser or a C# ‘modern mode’ which only used the more modern ‘correct’ features so that we could actually go away from the compromises of the past.
Unfortunately many modern features of C# are being built with these ugly artefacts seeping in. It’s the opposite of declarative programming, so it just keeps increasing incidental complexity: it exports the complexity to the users, something a type-safe compiler should be reducing.
For me, this is where lies the design flaw, trying to support both inheritance and be immutability at the same time.
I've seen people use records for value based equality and to use for things like dictionary keys. Immutability in c# just doesn't exist, any attempt to achieve it is flawed from the start
You could even imagine a compiler generated virtual method: `OnCloneReinitialiseFields()`, or something, that just re-ran the init-property setters (post clone operation).
Is there some other inheritance issue that is problematic here? Immutability isn't a concern, it's purely about what happens after cloning an object, whether the fields are immutable or not doesn't change the behaviour of the `with` operation.
Shouldn't SomeCalculatedValue be "This is another some value *calculated*" when using with ?
Edit: Actually that is the problem, that it isn't "recalculating" due to the way that the initialization of read-only properties works in a C# record.
Sometimes the csharplang team make some utterly insane decisions. This is the kind of thing that would happen back-in-the-day™ in the name of performance, but in the modern world just adds to the endless list of quirks (like uninitialised structs). I suspect there are elements of the design team that are still stuck in C#1.0/Java mode and so holes like this don't even seem that bad in their minds. But it literally leads to inconsistent data structures which is where bugs live (and potential security issues in the worst cases).
[0] https://meta.stackexchange.com/questions/9134/jon-skeet-fact...
I feel like what's happened here is that the author actually needed a system to cache their derived values, but didn't think to build that out explicitly.