Docs: Explain when NRT *may* change runtime behavior

Created on 22 Jul 2020  Â·  9Comments  Â·  Source: dotnet/docs

In Entity Framework, enabling nullable reference types feature without code modifications will change the behavior.

Without NRT: any reference type property maps to be optional in the database, unless the RequiredAttribute is explicitly added.

With NRT: any reference type property maps to be required in the database, unless it's marked with nullable annotation.

The article gives an impression that enabling NRT will never change the runtime behavior.

I'm not sure how entity framework knows under the hoods if the property is marked with nullable annotation or not, (maybe reflection?). But anyways, the explanation about this point shouldn't be EF-specific, but to whatever technique that is used to determine the nullability at runtime, and EF may be just taken as an example.

Although this is not the common case, and is actually rare, but I think worth noting.


Document Details

⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

Area - C# Guide P2 Pri2 csharp-null-safettech doc-enhancement dotnet-csharprod

Most helpful comment

I think @ajcvickers comment sums it up nicely.

In short the compiler doesn't change any runtime behavior here for NRT. That said it does change the metadata around the code that is compiled. This encoding is documented and supported by the compiler team. That means Frameworks which utilize reflection can rely on this to enhance their feature experience as they choose.

There is even a proposal to just expose this naturally on the Reflection APIs. That hasn't been done just cause hasn't risen to the priority list. There is nothing fundamentally wrong with it though (besides making reflection more C# centric).

Note that not all of our metadata encoding is supported in this manner. For example you could use metadata to expose closure types but that is explicitly not supported. We can and will break that encoding as we see fit. But in the case of nullability it is a supported encoding.

All 9 comments

I believe the current wording is more correct, and it make me uncomfortable to have the private reflection behavior of one library lead to high-level guidance change for a much larger feature in an unrelated component. The compiler did not introduce behavior changes for Nullable Reference Types, but as with any change, the use of private reflection can produce runtime behavior that differs from the assurances made by the compiler.

It may be worth filing a bug on dotnet/efcore that the library is deviating from the expectations of the language.

/cc @dotnet/roslyn-compiler

@sharwell I think efcore team wouldn't want to introduce a big breaking change like that for them.

What about being specific in the docs and state it in the way you said:

The compiler did not introduce behavior changes for Nullable Reference Types, but as with any change, the use of private reflection can produce runtime behavior that differs from the assurances made by the compiler.

adding @JeremyLikness

I think it does make sense to add a note similar to @sharwell 's comment above, and specifically mentioning EF. I'll make the following proposal:

  1. The change should go in the language reference article on NRT, rather than this tutorial.
  2. Add the following note following the first paragraph in Nullable references and static analysis:
    > The compiler doesn't introduce behavior changes, but other libraries may use reflection to produce different runtime behavior for nullable and non-nullable reference types. Notably, Entity Framework Core reads nullable attributes. It interprets a nullable reference as an optional value, and a non-nullable reference as a required value.

@BillWagner it LGTM, but I would like @ajcvickers to make the final approval for this

This was discussed at length last year. Both EF Core and MVC have different behaviors by-design based on lengthy discussions with the Roslyn team that this was appropriate. I think it's inappropriate to characterize this as "deviating from the expectations of the language." This use of reflection to obtain information about types and behave differently is a fully expected behavior of using NRTs with .NET. It should not just be a side-note.

@ajcvickers

I agree, and it's a question of where details should go.

In the language reference, emphasize that there are no new types generated for nullable reference types: A System.String and a System.String? are the same runtime type. The compiler adds metadata for the nullable context, and to distinguish a non-nullable reference from a nullable one.

I reviewed the language reference and the tutorial referenced in this issue. Neither state that other tools don't use reflection to modify behavior. I'd expect EF Core docs and MVC docs would cover those behaviors. I can add links to where that behavior is covered and call it out more strongly. Do you have a recommendation?

I think @ajcvickers comment sums it up nicely.

In short the compiler doesn't change any runtime behavior here for NRT. That said it does change the metadata around the code that is compiled. This encoding is documented and supported by the compiler team. That means Frameworks which utilize reflection can rely on this to enhance their feature experience as they choose.

There is even a proposal to just expose this naturally on the Reflection APIs. That hasn't been done just cause hasn't risen to the priority list. There is nothing fundamentally wrong with it though (besides making reflection more C# centric).

Note that not all of our metadata encoding is supported in this manner. For example you could use metadata to expose closure types but that is explicitly not supported. We can and will break that encoding as we see fit. But in the case of nullability it is a supported encoding.

Adding @roji here, since he has worked on the NRT docs for EF Core. @roji Do you think the EF docs cover this sufficiently? If not, please file a docs issue for it.

@ajcvickers I pretty much agree with the above. At the C# language level, NRTs don't change runtime behavior, but that doesn't mean they never impact library behavior. The EF docs definitely seem to describe all this sufficiently (here and here). The only possible issue (as the OP says) is with this C# language page, where it may be worth adding a sentence on this C# language page to state that library behavior may still be impacted (not sure it's actually needed though).

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ygoe picture ygoe  Â·  3Comments

skylerberg picture skylerberg  Â·  3Comments

sebagomez picture sebagomez  Â·  3Comments

ike86 picture ike86  Â·  3Comments

LJ9999 picture LJ9999  Â·  3Comments