Discussion: Init Properties and modreq

[HaloFour]

Problem

Over the weekend I decided to play with putting modreqs onto accessor method signatures in properties to see what the impact would be on Visual Studio. I assumed that the tooling support would be lacking and was correct. Object Browser doesn't understand the signature and incorrectly omits the type of the setter accessor method:

However, as I explored further I found that the language also didn't interact with the property as expected. Worse, the latest releases of the three major .NET languages, each handled the property slightly differently.

1. C# considers the property to be unsupported. You can call the getter accessor method directly, but you can't use property syntax.
2. VB.NET understands the property and considers it to be read only.
3. F# understands the property and ignores the modreq, allowing the property to be set.

This is problematic for multiple reasons, not the least of which is the inconsistency of the behavior.

In C#, existing compilers would not recognize the property. You can workaround this by calling the getter accessor directly, obj.get_Property(), however C# forbids doing this for recognized properties. This means that if you consume a C# 9.0 assembly from C# 8.0 and interop with these properties that updating to C# 9.0 would be a source breaking change.

In F# the situation is worse in that the feature does not prevent arbitrary mutation of the property.

Methodology:

As for how I tested this, I created a simple C# class library with the following code:

public static class IsInitOnly { }

public class TestClass {
    public string Property { get; set; }
}

Then I used ildasm/ilasm to manually modify the IL to add the modreq to the return value of the setter accessor method:

.method public hidebysig specialname instance void modreq(IsInitOnly)
          set_Property(string 'value') cil managed { ... }

Then I had to update the signature of the setter accessor method in the property metadata:

.property instance string Property()
{
  .get instance string TestModReqSetter.TestClass::get_Property()
  .set instance void modreq(IsInitOnly) TestModReqSetter.TestClass::set_Property(string)
}

I also tried applying the modreq to the parameter of the setter accessor method and the behavior was the same.

Possible Solution?

Property metadata in the CLR allows for a third accessor slot called other. To my knowledge none of the major .NET languages use this slot. I propose that instead of emitting a modreq-decorated set accessor that the C# compiler instead emit a new "init" accessor with that modreq modifier and assign that to this third slot:

// note the name change
.method public hidebysig specialname instance void modreq(IsInitOnly)
          init_Property(string 'value') cil managed { ... }

.property instance string Property()
{
  .get instance string TestModReqSetter.TestClass::get_Property()
  .other instance void modreq(IsInitOnly) TestModReqSetter.TestClass::init_Property(string)
}

I have confirmed that C#, VB.NET and F# all interpret this property as you'd hope/expect. All three languages allow you to read from the property as a property and none of the languages allow writing to the property. If we find a language that uses the other slot it may be worth testing with that language to see how it would behave with the modreq applied to the signature.

From a tooling perspective, VS does show the init_Property method in the same manner that it displayed the setter accessor above.

Advantages:

• Forward compatible with the existing major .NET languages and runtimes.
• Not a major design change to the currently proposed design.

Disadvantages:

• Switching from set to init or back remains a binary breaking change, but that was already the case.
• Uses that last property metadata slot preventing it from being used for other features in the future.

[333fred]

@jaredpar making sure you see this.

[jaredpar]

Yep. @HaloFour and I talked about it in the original PR and I pinged @jcouv about it.

[HaloFour]

@alrz mentioned on Gitter that if the team was interested in the other accessor approach, keeping the set_ prefix naming convention and not using the modreq would make it so that switching between init and set is not a binary breaking change.

My only concern with doing that is that is that without the modreq it's not clear to other languages which may already make use of the other accessor that they need to be compatible with init-only properties.

[jcouv]

Thanks @HaloFour for raising this and the detailed investigation!

Maybe I'm missing something, but I'm not quite seeing the impact as you describe it: "This means that if you consume a C# 9.0 assembly from C# 8.0 and interop with these properties that updating to C# 9.0 would be a source breaking change."

Let's walk through a few scenarios:

1. a library author uses C# 9 on .NET 5 and targets an older TFM: they won't be able to have an init-only property in the library (IsInitOnly type used as modreq doesn't exist in older TFMs), so there is no problem of consumption from an older compiler.
2. a library author uses C# 9 on .NET and targets the netcoreapp5 TFM and uses init-only property: this library cannot be consumed from an older compiler (it's an unsupported scenario to use an older compiler against .NET 5). The plan is to have the compilers (C#, VB and F#) that ship with .NET 5 to be able to consume init-only properties (ie. they can handle the modreq).

[YairHalberstadt]

@jcouv
The spec did say you can provide your own instance of IsInitOnly and this would be preferred to the one in the core library.

[HaloFour]

@jcouv

So the plan is that the compiler will require a runtime check in order to use the init-only behavior? As @YairHalberstadt mentioned the current proposal seems to indicate otherwise, and the meeting notes suggested that the modreq approach was considered due to broad compatibility with runtimes.

[jcouv]

The spec did say you can provide your own instance of IsInitOnly and this would be preferred to the one in the core library.

@YairHalberstadt Good point. That does create exposure of init-only properties to older compilers.

@jaredpar Should we adjust that to solve the problem, or is that too much of a restriction? We could also consider an approach based on Obsolete with magic string.
As we discussed in API review, I'm concerned that using a different slot would have much more of a negative impact in the ecosystem :-(

[HaloFour]

@jcouv

As we discussed in API review, I'm concerned that using a different slot would have much more of a negative impact in the ecosystem :-(

Out of curiosity what kind of negative impact do you foresee?

[KalleOlaviNiemitalo]

Uses that last property metadata slot preventing it from being used for other features in the future.

Can't you have multiple other accessors for the same property? For events, informative text in ECMA-335 6th ed. II.22.13 says each event can have "zero or one raise_ row, as well as zero or more other rows in the MethodSemantics table", and the reflection API has EventInfo.GetOtherMethods returning an array.

I have been sort of assuming that any language using other would encode the custom semantics with a prefix in the name of the accessor method, like your init_, and set the specialname flag.

[khm1600]

II.17 of ECMA-335 says

A property can contain any number of methods in its body.

and only one getter and one setter can be specified for one property, which seems to imply a property can contain multiple other accessors.

[HaloFour]

@KalleOlaviNiemitalo @khm1600

You're right, you can have multiple other accessors for a property.

[alrz]

I wonder if reflection still works to set .other init-only properties (for ef and deserializers?)

[jcouv]

Out of curiosity what kind of negative impact do you foresee?

This is more of an unknown unknown rather than a specific known impact. One of the areas we were worried about is reflection. Other tooling around IL could also be problematic.

[HaloFour]

@jcouv

Sounded like reflection was a concern when it came to modreq in general since the reflection APIs don't expose it. That might be more of a reason to use the other slot since existing frameworks wouldn't be able to tell an "init" from a normal "setter". And even if modreq was added to the reflection API existing frameworks would have to be changed to be able to tell the difference. Same would be true of a magic ObsoleteAttribute.

Are these same tools expected to be able to deal with modreq? I guess this is the problem with trying to use an incredibly niche feature of the runtime.

[YairHalberstadt]

@HaloFour
The majority case where reflection sets properties is in Serialization, DI, or object mappers.
All those cases count as part of an initialization phase, so you'd want them to be able to set initonly properties.

Now it's true that in same cases people use reflection to set things they can't via language rules, but you can also use reflection to set private properties, so why should initonly properties be any different.

[HaloFour]

@YairHalberstadt

Seems dangerous to design the feature so that just "works" out of the box with every existing framework that uses reflection for some purpose rather than requiring explicit opt-in. It's presumptuous that all such cases would align with init-only. I'd also suggest that there's a pretty big difference between the pathological case of trying to work around language (and runtime) protection via reflection vs. lacking the ability to correctly identify the intentions of the language.

[YairHalberstadt]

@HaloFour

The alternative is to severely delay adoption of initonly whilst we wait for every Serializer/DI framework to update.
XML Serializer, and many of the older DI Frameworks have not been updated for years. Effectively we would be leaving anyone who uses these legacy libraries locked out of working with this feature forever.
Yes some places will accidentally use reflection to do something they shouldn't. Reflection explicitly bypasses a lot of checks, and is considered dangerous for that precise reason.

[YairHalberstadt]

XMLSerializer in particular is interesting here. Since it doesn't support constructor injection, there is no way to use it to deserialize immutable objects. My current approach is to obsolete the setter. I was hoping init would provide a better way to do this, but that will only work if reflection works out the box with init.

[HaloFour]

@YairHalberstadt

I don't see having existing frameworks "accidentally" sorta support "init" through not being able to differentiate them as being one of the explicit motivations of the current design. The team has explicitly mentioned, more than once, about having to reach out to major languages and major frameworks to ensure that they will be able to support the new conventions and metadata. I believe that all such tooling should be updated explicitly to support it, or not support it at all. Yes, that will slow adoption, but that adoption should be intentional. If you're concerned about XmlSerializer being supported you should probably take that up with the CoreFX team.

Honestly, if you're seeing the current design as "good" because it allows for existing libraries to accidentally follow it, that makes me think that the current design needs to be seriously rethought.

Reflection explicitly bypasses a lot of checks, and is considered dangerous for that precise reason.

There's a big difference between using reflection to bypass checks and accidentally bypassing checks because they don't exist. A massive number of frameworks employ reflection and the vast majority of them aren't trying to behave pathologically.

[YairHalberstadt]

I think this is a matter of cost to benefit.
I can immediately think of hundreds of frameworks that will immediately benefit by supporting init without any extra work If each framework would take 20 developer hours to support init only properties, that's thousands of developer hours saved (probably a significant underestimate), and a huge speed boost to adoption.
What is the most popular nuget package you can find which this will adversely affect because they'll set something they shouldn't?

[HaloFour]

@YairHalberstadt

I think this is a matter of cost to benefit.

The LDM has never called this out. I'd say it's more of a happy coincidence than an intended goal.

Every single one of those same libraries are also potentially in violation of this feature. How many hours are necessary to audit where those "init" accessors are "accidentally" interpreted as setters incorrectly? Likely more than 20 a pop. I'd rather explicit support and slower adoption than accidental support any day.

[SingleAccretion]

explicit motivations of the current design.

That's the feeling I got while listening to the discussions here.

[HaloFour]

@SingleAccretion

Seems pretty mixed to me. @jaredpar does advocate for it (and this kind of stuff belongs in meeting notes and in the proposals) but it sounds like many other members are questioning it.

Either way, the problem with sticking with the current design is that set+modreq is broken because the majority of existing languages don't know how to deal with it.

Is the team sure that this approach of "accidentally" working will actually work with existing deserializers? How many of them emit dynamic assemblies instead of using reflection? IIRC that's what XmlSerializer does. Will that scenario work out of the box or will those frameworks simply explode at runtime?

[YairHalberstadt]

Will that scenario work out of the box or will those frameworks simply explode at runtime?

That's a good point @HaloFour
Seeing as you've got this modreq situation set up, would you be able to test it?

[HaloFour]

I can see if I can play with it later. It sounds like this problem has already reared itself with mocking frameworks trying to mock virtual methods accepting in parameters because the modreq is not automatically included in the generated method.

dotnet/runtime#25958

[jaredpar]

I think this is a matter of cost to benefit.

This is indeed a cost benefit trade off and after discussion we decided to favor init working in existing scenarios over the cost of having every single on of those scenarios change to support init properties.

What is the most popular nuget package you can find which this will adversely affect because they'll set something they shouldn't?

The other aspect to consider here is how far do you want to go with enforcement? The item to keep in mind here is that the runtime provides zero enforcement of readonly instance fields. That means reflection, even public reflection, can stomp all over readonly today and there is no penalty for doing so.

One aspect we considered in LDM when discussing these type of packages is "what is the value in holding init accessors to a higher standard than readonly fields? The answer we came to is there isn't a lot of value in doing this.

Reflection can already invalidate object immutability and this is exceedingly ulikely to change. Hence by making init accessible to reflection we're not changing the semantics here, we're just fitting with the existing system. In that sense the trade off here is very reasonable from our view point.

[HaloFour]

@jaredpar

I'm not talking about finding a way to prevent pathological applications of reflection, I don't see why that analogy applies here or in the design considerations. Yes, you can break all of the rules with reflection, that doesn't mean that rules shouldn't exist or that guardrails shouldn't be erected for libraries that aren't trying to go out of the way to do the "wrong thing". Serializers like JSON.NET intentionally respect readonly fields.

To that end why not make it metadata that is completely erased at runtime? That way there is no binary compatibility break when flipping between set and init and all existing code will just support mutating it out of the box. Newer languages and newer frameworks can be made to respect it.

[jaredpar]

The design trade offs we're going for here is meant to mirror that of readonly fields. Specifically the following:

• Blocking reflection which mutates members (set of fields or invoke of property setter) is a non-goal. This has always been an avenue to violate immutability in .NET and this feature isn't any different in that respect.
• Enabling frameworks which want to correctly respect this behavior the ability to do so. Just as a serializer like JSON.NET can check to see if a field is readonly they can check to see if a property setter is init.

To that end why not make it metadata that is completely erased at runtime?

The metadata needs to be present in metadata so the language and frameworks can respect and engage with it.

That way there is no binary compatibility break when flipping between set and init and all existing code will just support mutating it out of the box.

One desirable behavior of using a modreq is that flipping init to set (or vice versa) in a virtual position is a binary breaking change as well as a source breaking change. That is how other features like using in work. It's not a fundamental requirement of the design though, just a benefit.

[SingleAccretion]

@jaredpar
A question I found interesting: how will init work with dynamic?

[jaredpar]

A question I found interesting: how will init work with dynamic?

My expectation is that we will change dynamic to not allow access to init accessors. It cannot modify readonly fields today hence that causes the behaviors to line up.

[HaloFour]

Some good news is that very basic use of JSON.NET, DataContractJsonSerializer and XmlSerializer seem to work with the modreq in place. I'm actually kind of surprised with XmlSerializer as I thought it always emitted an assembly, but maybe that's changed with .NET Core.

If the goal is to use set and modreq specifically for this "accidental" support in existing frameworks that brings us back to the original problem mentioned on this issue with existing versions of C# and F#.

[SingleAccretion]

I guess the "pro-init-is-set" view then (absent examples of broken code that is relevant) is that the cost of updating the tooling and the compilers is worth it compared to the cost of not having "out of the box" support for serialization. In all honesty, even for me, who would rather have a separate new accessor for theological reasons, this sounds like a very hard argument to beat.

[Joe4evr]

In all honesty, even for me, who would rather have a separate new accessor for theological reasons, this sounds like a very hard argument to beat.

And that's a real shame, IMO. I believe that it's much better in the long run for serializer frameworks to be forced in the position to explicitly opt-in to supporting init accessors, rather than having them work on a technicality. =/

[canton7]

@HaloFour Could you test compiled expressions as well? Though if the serializers work, that probably will too.

[davkean]

@jaredpar @jcouv I've read above and can't find the conclusion on why the moqreq on the return type of the setter as opposed to the parameter type?

Is that so that you can flip from init -> set and existing binaries that have a MemberRef to it continue to work? Edit: Nope, can't be that as return types are part of the signature.

I was picturing that compilers need to understand just the modreq(s) on the parameter type to call it, but not on the return type - but is the feeling here that compilers need to understand all the modreqs applied across the entire signature before calling it?

[jaredpar]

@davkean

I've read above and can't find the conclusion on why the moqreq on the return type of the setter as opposed to the parameter type?

The idea was essentially future proofing for the more general init members feature. In a world where init members existed we wanted to make the check for "is this an init member" simple for non-compilers. All methods have a return type, but not necessarily a parameter. Hence putting the modreq on the return type meant the check would be universal.

[agocke]

If the goal is to use set and modreq specifically for this "accidental" support in existing frameworks that brings us back to the original problem mentioned on this issue with existing versions of C# and F#.

This doesn't seem to be much of an issue. There are a lot of good reasons why someone can't update their framework target, but not very many for why they can't update their compiler version.

Note: even if you provide your own IsExternalInit type, for an older compiler to consume it you would have had to target an older framework (< 5.0) with the latest language version, which will be unsupported.

[HaloFour]

@agocke

which will be unsupported.

And trivial. It should be apparent from the number of questions around C# 8.0 and older versions of .NET how many people want to be able to use the newer compilers with the older runtimes. You state this yourself right here:

There are a lot of good reasons why someone can't update their framework target, but not very many for why they can't update their compiler version.

You're giving them one.

Also, broad compatibility with runtimes was listed as the main reason this approach was chosen:

Given the path forward for other languages and compatibility with many runtimes, we think it's a better future as an IL pattern.

[agocke]

You're giving them one.

Incorrect. Using the latest compiler with langversion set to C# 7.3 is a perfectly supported scenario to target the desktop .NET Framework, for example. In fact, it's the recommended scenario. Using the latest Visual Studio is always recommended.

Similarly, using a .NET 5 compiler to target netcoreapp3.1 is perfectly supported, you'll simply need to install the 3.1 SDK and the dotnet tool will automatically use the latest compiler, not the compiler which originally shipped with the 3.1 SDK.

[HaloFour]

@agocke

Incorrect. Using the latest compiler with langversion set to C# 7.3 is a perfectly supported scenario to target the desktop .NET Framework, for example. In fact, it's the recommended scenario. Using the latest Visual Studio is always recommended.

So, then which is it? Is this feature supported only on the combination of .NET 5.0 and C# 9.0, or is it supported on older frameworks? Where does the broad runtime compatibility come into play? How will people using those older compilers know that they need to update to the latest version to understand these modreqs correctly? Especially F# users, where they have no idea that anything is broken until they upgrade to the newer compiler?

The messaging around the compiler/runtime situation in general has been unbelievably poor. If the goal of the team is for the two to evolve in lock-step, like Java, then fine. But even through this thread it's difficult to ascertain that this will be the case. You can't both claim broad runtime compatibility and then shrug at the fact that is broken because you don't actually care to support any existing runtime.

There's also the really weird situation that you want "accidental" support for these properties on existing serialization frameworks, zero of which will be targeting .NET 5.0.

[agocke]

So, then which is it? Is this feature supported only on the combination of .NET 5.0 and C# 9.0, or is it supported on older frameworks?

There are three components in play here: framework version, language version, and compiler version.

Certain frameworks are supported for certain runtimes. netcorapp3.1 is supported on the 3.1 runtime, net48 is supported on the desktop 4.8 runtime, etc.

Language versions have minimum framework requirements. C# 8 has a minimum framework requirement of netcoreapp3.0/netstandard2.1. C# 9 will have a minimum framework requirement of net5.0.

Compiler versions have maximum framework requirements. The compiler originally shipped with 3.1 supports all frameworks up to netcoreapp3.1 and language versions up to C# 8. The compiler shipping with 5.0 will support all frameworks and language versions up to C# 9.

So the difference here is that upgrading each of the framework or language version components may require upgrades of other components. But because the compiler supports all previous versions of related components (barring the rare breaking change), it's almost always good to upgrade to the new compiler.

[agocke]

As for the support matrix, the feature will only be supported on net 5.0 and C# 9.

You were explicitly asking about an unsupported scenario, where someone compiles for netcoreapp3.1 with C# 9, and then that library is consumed by someone else using netcoreapp3.1. If they are using the net5.0 compiler, the modreq will be recognized, and the get property will work, even in C# 8, even though this is an unsupported scenario. If they are using a netcoreapp3.1 compiler, the entire property will not be accessible.

Because of the above situation, where upgrading the compiler is almost always a safe operation, even unsupported scenarios will likely be functioning. The only thing which will not is consumers using old compilers and consuming libraries created by users producing unsupported binaries.

[HaloFour]

@agocke

So the TL;DR version is that "init" properties are only expected to work (or "be supported") on .NET 5.0.

IMO, if the teams goal is to continue marching C# and .NET in lock-step, that needs to be both communicated and enforced a whole lot better. It's trivial to make C# 8.0 compile to older targets, and every feature except DIM can be made to work. This has been the case with (I believe) every feature that C# has shipped in the prior 15 years (between DIM and generics). Invariably a decent part of the community will continue to do this. With DIM the break makes sense, you're not going to backport a completely new runtime feature to older runtimes. But here the reason is simply because the team doesn't care about how the current compiler can deal with perfectly legal metadata or how you might decide to apply it in the future. Yet it's a goal that the feature works out of the box for assemblies compiled with unsupported compilers targeting unsupported runtimes. It's difficult for me to not take exception with the direction that the design has taken around this feature.

Anyway, it's clear we're not going to agree about this.

[HaloFour]

I will note that the set route with the magic Obsolete attribute would likely both fix the issue with older versions of the compilers as well as let the feature work "magickally" with existing serialization frameworks.

And I am very thankful that we can at least engage in this conversation. Whether we agree or not it's awesome to be able to debate language design with actual language designers. :)

[agocke]

I will note that the set route with the magic Obsolete attribute would likely both fix the issue with older versions of the compilers as well as let the feature work "magickally" with existing serialization frameworks.

To be clear, I agree with this and am not opposed if we take this path. I just wanted to emphasize that using a new compiler with old language versions/frameworks is a perfectly supported and recommended practice. And if we assume that people are doing this (if you upgrade to new versions of VS this will likely happen regardless), the scenarios that break are significantly smaller than it may seem at first.

[CyrusNajmabadi]

IMO, if the teams goal is to continue marching C# and .NET in lock-step, that needs to be both communicated and enforced a whole lot better.

Tagging @terrajobst . Immo, do we have good docs anywhere explaining in depth what @agocke outlined in: #3376 (comment)

?

Thanks!

[ericsampson]

@CyrusNajmabadi , bumping your question if this made it into the docs, given that there's a lot of C#9/.NET 5 doc work going on in prep for 🚢. Should we ping Bill Wagner or something?

[333fred]

@BillWagner, do we have docs anywhere on the supported matrix of language version/target framework?

[BillWagner]

We have this table which covers some of the discussion above. This table has some of the info as well. It doesn't quite cover all the scenarios @agocke mentioned above. Finally, this article covers the concepts behind framework dependencies, but not CLR dependencies.

[YairHalberstadt]

Closing as the ship has already sailed here

[FreeApophis]

We just ran into this scenario:

• C++/CLR solution which compiles also C++ code and uses old Compiler
• C# which uses .NET 5 and C#9 feature which produces NuGet's also for the above solution in a compatible way as .NET Standard 2.0 Target - no warnings no errors, until you use a Property from this library. It produces the very odd CS1545 which has no documentation at all why it could happen in C#.

https://docs.microsoft.com/en-us/dotnet/csharp/misc/cs1545
The only stackoverflow article was useless:
https://stackoverflow.com/questions/66054634/cs1545-when-moving-from-visual-studio-2019-back-to-2017

I finally found this User-Story after someone mentioned that the init-property uses modreq.

And that's the solution, isn't it lovely:

  <PropertyGroup>
    <DefineConstants Condition="'$(MSBuildVersion)'=='16.5.1' ">BAD_COMPILER;$(DefineConstants)</DefineConstants>
  </PropertyGroup>

#if BAD_COMPILER
            return result.get_IsRequired();
#else
            return result.IsRequired;
#endif

[jaredpar]

Sorry you're hitting that. That is unfortunately one of the potential problems the user of the NuGet library is signing up for when they down target C# features in that way. This is explicitly unsupported for reasons like this one. It can cause friction when used in older toolsets that don't understand the metadata encoding for new features. It's not just a discussion around modreq it's all of the encoding decisions we make with new features here.