Discussion on non-null reference types

[jcouv]

Discussion started off from Chuck's PR following recent LDM discussion, and continued with Neal.

The way we initially tried to formalize this was with two or maybe three states (? meaning nullable, ! meaning non-null, oblivious), two warning scenarios (dotting and assignment), and three contexts (C# 7.0, C# 8.0, C# 8.0 + non-null flag for additional warnings).

We started with two warnings:

• assigning a ? type to a ! type is a warning
• dotting into a ? type is a warning

I was not clear what warnings should be produced for C# 8.0, and what additional warnings should be produced when the non-null compilation flag is set.

As we tried to clarify, two scenarios jumped out:

[scenario 1]

var s = "";
s = null;
string q = s; // expect no warning 8.0 (compat)

If s is a ? and q is a !, then this code compiles in 7.0, but breaks in 8.0.

[scenario 2]

var s = c ? "" : null;
Print(s.Length); // expect no warning in 8.0 (compat)

Problems:

• If the conditional expression is given a ? type, then this code compiles in 7.0, but breaks in 8.0.
• If the conditional expression is given an oblivious type, then this code will never produce a much needed warning.

Those two scenarios, plus some others below, make us think that all new nullability warnings must be disabled for 8.0, and only enabled when you set the non-null warnings on.

Some other scenarios we discussed:

[scenario 3]

var s = ""; 
s = null; // Print is external API, declared with Print(string!)
Print(s.F()); // expect no warning in 8.0 (compat), but warning with non-null setting

[scenario 4]

var s = Foo();
if (s == null)
    FailFast(); // throws, but the compiler doesn't know that
Print(s.Length); // unfortunate warning, breaks compat

I think all the designs we discussed produce a false alert on scenario 4 (FailFast method throws, but the compiler doesn't know it).

We fell back onto having three states (?, !, oblivious) instead of just two (?, !).

[scenario 5]

string /*!*/ s = null; // expect no warning in 8.0 (compat), but warning on conversion with non-null setting

We'll want a warning here, so we need a third warning: 'conversion of null to !'.

[scenario 6]

string s /*!*/ = c ? "" : null; // expect no warning in 8.0 (compat), but warning on assignment with non-null setting

We'll want a warning here, so we extend the better type rule for conditional expression. Just like c ? 1 : null should be inferred to have type nullable type (int?), c ? "" : null should get the nullable annotation. The "" is a !, there is a 'null conversion to ?' on the null.

This leaves us with three warnings (only when non-null flag is set):

• warn when dotting on ?
• warn on 'null conversion to !'
• warn when assigning from ? (determined by flow analysis) to ! (from symbol)

The information from symbol tells you "what you can put into it". The information from flow analysis tells you "what you get out" (for local variables). For fields there is an open question.

Assign to (from symbol) \ from (from flow analysis)	!	?	oblivious
!		warn
?
oblivious

Options

Option 0 (Mads)

• string => !
• string? => ?
• string! => disallowed
• var (local) => inferred from RHS

This option strongly relies on the import setting per library (see details below). Somehow the warnings are not produced for types from opted-out libraries.

The upsides are that:

• there are only two syntax forms and two types (simpler).
• types mean the same thing everywhere in source (API and method bodies). (only opted-out libraries have a different behavior)

The downsides are that:

• it requires an out-of-band mechanism for library authors to coordinate with clients.
• migration process is all-or-nothing
• difficulty of analysis for types from opted-out libraries
• tooling issues for storing and managing opted-out references: command-line, IDE, project file, etc

Option 1

For both fields and locals:

• [URTANN] string => !, and [URTANN] applies to method bodies (URTANN = "UnadornedReferenceTypesAreNotNull" attribute)
• [URTANN(false)] string => oblivious
• string? => ?
• string => oblivious
• var (local) => ?
• string! => disallowed

The downsides are that:

• migration requires more code change: once you try to migrate an API, the body will requires a lot of ? sprinkled in
• you don't get the benefits of flow analysis until you make such code change
• the meaning of "string" depends on URTANN above

Option 2 (Neal)

Same as option 1, but with a difference for locals ([URTANN] does not apply to method bodies):

• [URTANN] string => !
• [URTANN(false)] string => oblivious
• string? field => ?
• string field => oblivious
• string? local => ?
• string local => kinda oblivious (flow analysis gives you the type at the use-site of the local)
• var (local) => ?
• string! => disallowed

The upsides are that:

• you get helpful warnings in method bodies.
• the migration story is incremental:
  1. turn on non-null flag: you should get very few warnings at this stage
  2. gradually add URTANN to APIs and fix clients (rarely needed)
  3. repeat and move URTANN up until top-level
  4. done

The downsides are that

• "string" doesn't mean the same in APIs and in methods (because URTANN does not apply inside method bodies).
• you cannot get some benefits of ! in method bodies.

Option 3

Same as option 2, but URTANN applies to method bodies.

URTANN dictates how "string" is interpreted in a method.

The upsides are that you get helpful warnings in method bodies, and "string" means the same in APIs and in methods.
The downside is that the migration becomes more difficult (you need to add ? on locals).

Option 4 (Julien)

Another way to keep "string" to have the same meaning between APIs and locals, is for APIs to align with the rules for locals.

• No URTANN
• string => oblivious (for field), and kinda oblivious for locals (we let flow analysis warn)
• string! => !
• string? => ?
• var (local) => ?

The upside of this option is that "string" has the same meaning everywhere (in field and in local).
The downside is that we introduce a syntax for ! and APIs will be littered with it (but not method bodies).

Importing libraries

There is also an orthogonal discussion on what to do with types from libraries. It can be bolted onto the various proposals.

There is a proposal to import a library with some opt-out, which means that every type is treated as oblivious.

	legacy lib	modern lib
opt-out	oblivious	oblivious
no opt-out	oblivious	! and ?

Open issues

Open issues we mentioned but didn't discuss in details:

• TryGet API

Map<int, string /*!*/> map = ...
map.TryGet(...);

• FirstOrDefault API
• default(T)
• overrides
• what flow analysis does for fields?

[jnm2]

all new nullability warnings must be disabled for 8.0, and only enabled when you set the non-null warnings on.

Given that whatever we get will be opt-in, I'd be happier with a two-state system and errors rather than warnings. And ten times happier with CLR enforcement of a type system guarantee of non-defaultness.

[sharwell]

So if the diagnostics related to this feature are reported at assignment and use time, we have the following cases:

Target \ Source	?	!	oblivious
?			info ¹
!	warn		info ²
oblivious	info ³		info ⁴

Each informational case is a diagnostic at "hidden" severity, which could be examined by tools or have the severity increased to gain knowledge about the amount of protection being offered by the use of null analysis.

¹ This case preserves safety, but addressing it could reduce the complexity of code.
² This is a "poisoning" case, where opt-in use of a non-null type is suppressed due to the use of an API which has not yet opted in.
³ Possible assignment of nullable value to a target that expects a non-null value.
⁴ Should only be reported when the source or target is non-local (poisoned value leaving the scope of analysis).

For flow analysis, we also update the type upon assignment:

Target \ Source	?	!	oblivious
?	?	!	?
!	!	!	oblivious
oblivious	?	!	oblivious

For flow analysis, we may also have to merge two values:

Value 1 \ Value 2	?	!	oblivious
?	?	?	?
!	?	!	oblivious
oblivious	?	oblivious	oblivious

Inputs to APIs which have not yet opted-in will be oblivious. Outputs from APIs which have not yet opted-in will be oblivious.

[sharwell]

💭 Breaking changes over time are likely (caused by introduction of new warnings as the flow analysis strength is increased). This could be mitigated by making the compiler only responsible for the preservation of type information in signatures. The flow analysis for warnings could then occur as a set of analyzers which are versioned independently.

[MadsTorgersen]

@jcouv:

Since you labeled option 0 "Mads", let me address how it would handle your six scenarios. It is in keeping with design decisions last week, (and I will get the notes out soon), but goes beyond what has yet been discussed in LDM.

The core of option 0 is that there's a "nullable reference types" feature that is on by default in C# 8.0, and does not cause breaking changes (except insofar as a referenced library uses ? and the previous compiler didn't recognize the metadata, so recognizing it causes new warnings. There's a proposed per-reference opt-out to deal with that). This feature leads to warnings when you dereference a nullable reference type, or convert it to a type that is not nullable, except if flow analysis determines that it won't be null in that particular place in the code.

Then there's another feature, which we can call "Only nullable reference types should be null", which does its best to ensure that you don't put null into reference types unless they are marked nullable. This feature is opt-in, because it will generate warnings on existing legal code. It is also somewhat unreliable, in that it doesn't come close to full protection: it has many known holes. Below, "opted-in" means you have that feature on by default.

Here's how "The Mads option" handles each scenario:

[scenario 1]

var s = ""; // infer 'string'
s = null; // warning only if opted-in
string q = s;

No problems here. When you opt-in, you start getting warnings about putting non into things that aren't nullable. Great!

[scenario 2]

var s = c ? "" : null; // infer 'string' if not opted-in
Print(s.Length); // therefore no warning if not opted-in

If you haven't opted in to null in not-nullable reference types being a bad thing, you shouldn't be concerned about this behavior. We infer string, as always, and we put null into it, as always.

The question is what should happen when you opt-in. I think this is an open design question, and we have two options:

1. Still infer 'string', and start warning on the conversion of the null to it.
2. Start inferring 'string?' instead. Warn on s.Length.

While 1 is simpler, in that it avoids changing type inference under a compiler switch, it also leads to quite unintuitive and unhelpful behavior. I am more and more convinced that we should do 2.

[scenario 3]

var s = ""; 
s = null; // Print is external API, declared with Print(string!)
Print(s.F()); 

This example is a little cryptic: What's the problem exactly? In the Mads option, there is no notion of ! in Print(string!). If it is defined as Print(string), however, then it is treated as if it was declared in your own source.

• If you are opted-in, then you get a warning on the second line (assignment of null)
• If F returns a nullable type T?, then you get a warning on the third line regardless of whether you are opted in or not (converting from nullable to not nullable)

[scenario 4]

var s = Foo();
if (s == null)
    FailFast(); // throws, but the compiler doesn't know that
Print(s.Length); // unfortunate warning, breaks compat

Agreed that none of the proposals address this on its own, though I don't see that it breaks compat: Somebody added a ? somewhere, so this does not break existing code. Specifically, the above only yields a warning if Foo returns a nullable reference type, so this is not a compat issue.

It's still a problem though. We'd need to think of adjoint features to help us when logic relevant to the flow analysis is factored out into helper methods. Here it's the fact that FailFast never returns. There's also IsNullOrEmpty type stuff. However, this whole thing is orthogonal to the design choices discussed, and doesn't impact the core decisions, so let's leave it aside for now.

[scenario 5]

string s = null; // warning if opted-in

This one's the textbook case for why we need an opt-in to warnings about assignment of null.

[scenario 6]

string s  = c ? "" : null; // warning when opted-in

Depending on the design decision in [scenario 2], this yields a warning for either one reason or the other, but only when opted-in:

• if we infer string for c ? "" : null, then that expression itself leads to a warning that null is assigned to the inferred string
• if we infer string? for c ? "" : null, then assigning that to string s leads to a warning.

I hope that helps clarify things, and how we can live with just two states: Nullable reference types and not nullable reference types.

[MadsTorgersen]

By the way, we haven't discussed URTANN in the design meeting yet, but I am a staunch opponent of it. It adds another thick layer of complication to the whole feature, and I don't think it adds much value.

For most APIs that haven't been annotated yet, an opted-in client should be ok with not passing null to them. Rarely does passing null to an API obtain something uniquely valuable, and when it does, there'll be an easy workaround (pass null!, using the dammit operator to shut the compiler up).

In the rare cases where this really does go against the API's pattern of usage, or where you simply can't deal with fixing your code up (even though you opted in), use the reference-level opt-out to silence warning from that library wholesale, until:

• you are ready to deal with it, or
• they add the proper ?s to their API or
• forever: it's no disaster, you just locally lose the benefits of the feature when using that particular API.

[sharwell]

@MadsTorgersen In Scenario 3, you mention the following:

If F returns a nullable type T?, then you get a warning on the third line regardless of whether you are opted in or not (converting from nullable to not nullable)

However, if F and Print are both defined in e.g. mscorlib, then this (warning without opt-in) could be a breaking change for existing code.

[HaloFour]

Re var, I commented about this recently.

TL;DR, var should be ? because:

1. You can safely assign ? to it and the compiler will warn if you attempt to dereference.
2. You can safely assign ! to it and the compiler will not warn if you attempt to deference due to flow analysis.
3. You can safely reassign either ? or ! to it and flow analysis will ensure the appropriate behavior.

var s = ReturnsNonNullable();
var l1 = s.Length; // no warning

s = ReturnsNullable(); // no warning
var l2 = s.Length; // warning

s = null; // no warning
var l3 = s.Length; // warning or maybe even error

Honestly, I might go so far to argue that the ? suffix is unnecessary except at the boundary. The compiler can determine how a local is assigned and used within the method so being explicit about it isn't entirely necessary. I could see wanting to have a way to ensure at assignment that a local will be not null, but I'd argue that the ! "dammit" operator would suffice there.

To that end I think I lean towards Option 2.

And sorry for being thick but did I miss where "URTANN" was defined? Or is that just a placeholder for the boundary metadata attribute?

[MadsTorgersen]

@sharwell, we aren't keen on doing the flow analysis as separate analyzers, because we think that their implementation and performance will benefit greatly from being integrated in the compilers. Also, probably most improvements of the flow analysis will remove warnings based on recognizing more details, or more specially annotated methods, or whatnot.

However, as a middle ground, if we find the need to add additional warnings, maybe they can be added behind a "warning waves" flag, along with other "breaking" warnings that we may choose to add to a given version of C#.

[sharwell]

However, as a middle ground, if we find the need to add additional warnings, maybe they can be added behind a "warning waves" flag, along with other "breaking" warnings that we may choose to add to a given version of C#.

This was my first thought, but then I realized we have a potential problem related to flow analysis. If a change in flow analysis rules results in code that was previously "oblivious" being treated as either ? or !, then new instances of existing warnings could show up down stream. This would mean the warning waves would need to be tied into the flow analysis inference code instead of the diagnostic reporting code, which could make long-term maintenance very difficult.

Example:

var value = LegacyType.GetValue();
if (value != null)      // This is a bug that non-null reference types feature could catch someday
  Environment.FailFast();

NewType.SendNonNullValue(value);

In the above code, the result of GetValue() is "oblivious". No warning is reported in the call to SendNonNullValue. If a future update to the flow analysis supported a never return type (#538), the inferred type of value at the call site becomes nullable instead of oblivious. The code would start reporting a warning under the original usage rules even though at first release no warning was reported.

Moving the analysis to analyzers decouples the maintenance of this complicated algorithm from the compiler's compat concerns, allowing much more rapid development of its capabilities. The time saved in maintenance issues will likely cover the investment cost of performance parity and then some.

[jcouv]

@MadsTorgersen Thanks for the clarifications on the "Mads option" :-)

It seems we still end up with three cases for symbols coming from a library:

1. string? => nullable
2. string (library was not opted-out) => non-nullable
3. string (opted-out) => non-nullable but avoids some warnings (I think this is what we've been calling "oblivious")

We can say that we only have two cases: string? and string, but for the latter there are still two sub-cases (did this come for an opted-out or a not-opted-out library?) for warning behavior.

If we're going to compute/track those bits of information for symbols from libraries (regardless of how we organize them into two or three states), then we should consider allowing source symbols to take advantage of such opt-out as well.

From prototype, Chuck found that migrating just one field in Roslyn (BoundExpression.Type) is hard enough that we'd likely want to stage the migration of our APIs. Whether this is done with some granular mechanism for opt-in like URTANN or some granular mechanism for opt-out instead, it seems likely useful for practical migrations.

For scenario 2 (var s = c ? "" : null;), if we're ok with the language making a different inference depending on warning setting (I thought that was verboten), then I like option 2 as well (inferring string?).
I find it counter-intuitive that we would ever infer string for this local, in a world where string? exists. On the other hand, if we inferred string? when not opted-in, then we break compat... :-(

[sharwell]

On the other hand, if we inferred string? when not opted-in, then we break compat... :-(

If the analysis which reports diagnostics lives as an analyzer, then the compiler type system can use the inferred type string? without it being a breaking change. "Opting in" involves adding the analyzers to the project. The approach decouples the compiler's understanding of the type system from the breaking changes associated with checking these references.

[jcouv]

@sharwell Your proposal seems orthogonal to that problem.

We could also shift the two warnings ("warnings when you dereference a nullable reference type, or convert it to a type that is not nullable, except if flow analysis determines that it won't be null in that particular place in the code") to be tied to opt-in switch.

Either way (shifting more warnings to be opt-in or shifting them to an analyzer), it means that you don't get some useful warnings by default even when you write code explicitly with new syntax string?, because we inferred string? on some other existing code which should remain compatible...

[sharwell]

Either way (shifting more warnings to be opt-in or shifting them to an analyzer), it means that you don't get some useful warnings by default even when you write code explicitly with new syntax string?, because we inferred string? on some other existing code which should remain compatible...

True, or at least possibly true.

However, I have serious doubts regarding "improvements within the bounds of back compat" being a strong motivator for adopting the new feature. I believe people will care substantially more about 1) overall coverage¹ and 2) the migration story for large and established, but still actively developed code bases. If we disable warnings until opting in, I doubt anyone would bat an eye.

¹ Think code coverage tooling, except looking for instances of "oblivious" types in the CFG

[gulshan]

I think there would be a break either in Roslyn or in APIs. And if the former is not an option, latter is the way to go. My opinion is based on option 4 but taking things a bit further-

• Merge current oblivious and nullable ? types.
• Make a non null references a separate type as struct NonNull<T> where T : class. ! would be a syntactic sugar for this type.
• ! type will have implicit operator conversion to oblivious/nullable types!
• A helper extension methods convert oblivious/nullable types to ! after performing a null checking. It throws if null is found.
• New dotnet standards with ! type support with version update in 3rd part as 1.0 -> 1.0.1. APIs will also be of ! type where appropriate.

[DavidArno]

@HaloFour,

TL;DR, var should be ? because...

To copy @MadsTorgersen's phrase, I am a staunch opponent of this. To my mind, Mads' ideas on all six scenarios make total sense to me: they are exactly as I'd imagine the (non)nullable ref types feature would work. Everything else is just plain weird and confusing to me.

For scenario 2, option 2, "Start inferring string? instead. Warn on s.Length" again seems to me to be the option that keeps this feature simple and consistent.

[CyrusNajmabadi]

One of the main purposes of the "damnit" operation ! is to ensure that this can be adopted in a non-infectious manner. The moment you get to a place in your code where you want to nip things in the bud (for whatever reason makes sense for you) you can ! things away and figure out how you want to deal with that area later.

It is a core goal of ours that you be able to adopt this feature in a gradual manner.

[DavidArno]

@CyrusNajmabadi,

The ! operator makes sense as a way of easing folk into adopting this feature. However, as I mention in Nullable reference types: Mads's current thoughts, for it to be a genuine "figure out how you want to deal with that area later" feature, there has to be a point in that gradual adoption process where the compiler warns on using ! too. Otherwise it'll risk becoming a "deal with that area, never" operator.

[jnm2]

@DavidArno What if the flow analyzer is never smart enough to allow me to remove all !s? For example, if it doesn't recognize string.IsNullOrWhiteSpace (or some custom method) as a null check?

[HaloFour]

@jnm2

That would be another advantage to Nullable reference types: Mads's current thoughts, the analyzers could have configuration to support a variety of APIs that could be considered null-checks, both BCL and potentially our own.

[DavidArno]

There are two three (or more!) possible solutions to that:

1. Introduce some means of telling the compiler that some method call, eg string.IsNullOrWhiteSpace is a null-check, so the flow analysis can treat it as not null (eg some sort of [NotNullOnTrueResult] attribute).
2. Allow ! via a warning suppression attribute at the point of use.
3. EDIT: @HaloFour's suggestion of configuring the analysers.

I'd really like option 1, but it may well be hard to implement in practice.

[panost]

The bang! operator should be named as the "2B$ mistake" operator.

I think R# approach with NotNull, CanBeNull, ItemNotNull attributes, accompanied with ContractAnnotation and some flow analysis should be enough. Thankfully you can't put attributes on local variables, in contrast with the no-nullable ref types approach.

I believe that nullability is a contractual attribute and enforcing it on the type-system is wrong

[jnm2]

I believe that nullability is a contractual attribute and enforcing it on the type-system is wrong

If something is a contract, isn't the type system the ideal place to express it?

[panost]

@jnm2 Like exceptions that might be thrown from a method, for example ?
No, I do not think that is correct

[HaloFour]

@panost

I think R# approach with NotNull, CanBeNull, ItemNotNull attributes, accompanied with ContractAnnotation and some flow analysis should be enough.

That's exactly what these proposals suggest. The ? suffix is simply shorthand for applying the attribute on a parameter or whatever. The real "enforcement" is just enhanced flow analysis on those locals. The type system is unaffected. A string? is still a string. It's the variable that is different based on that hint and actual usage.

[jnm2]

@panost

Like exceptions that might be thrown from a method, for example ?

That's probably the weakest possible example you could come up with, but funny you mention that. Indeed. I don't want to derail the conversation though.

[panost]

@HaloFour You mean that for example Func<string?,string> is interchangeable with Func<string,string?>. How about ValueTuple<string?,string> with ValueTuple<string,string?>. C# can't put attributes to those. It may not affect the underlying type system at CLR level, but still you have to deal with all those exceptions using that bang! operator. Another problem to solve and that is a problem you have to solve for each statement and each assignment. Is not limited with what arguments the method accepts and what is the return value as is with R# solution or any other solution that uses contracts.

@jnm2 Java has this and Java programmers, don't find it funny at all. It is an example of a contractual attribute implemented in a type-system, with disastrous (in my opinion) results

[HaloFour]

@panost

The attributes would apply to the delegate parameter itself, not the generic type arguments. This is identical to how dynamic works today. Still strictly metadata. The underlying type system couldn't be changed even if the C# team wanted to, at least not without massive effort that they've plainly stated that they wouldn't be interested in.

As for checked exceptions, sure they exist in Java. Watching how annoying they are and how infrequently they're used appropriately is exactly why the CLR and C# don't support them. They omitted them on purpose.

[panost]

To be honest, I don't understand how dynamic relates to nullable type definitions and metadata. My understanding was that the compiler will perform just a Type erasure of everything related to nullable ref types and leaving only some attributes to methods and method's attributes and without any "exotic" code generation.

[HaloFour]

@panost

To be honest, I don't understand how dynamic relates to nullable type definitions and metadata.

I was referring to the application of that metadata. There is no dynamic type in the CLR or BCL. It's just a normal object and the C# compiler emits DynamicAttribute attributes to notate which of those should be treated as dynamic. This applies to generic type arguments as well by tracking an array of bool values which recursively walks the type definition. The nullability attributes will be applied in the same manner, which is how the compiler will know that the generic type argument of a specific type is nullable even when attributes can't be applied directly there.

For example, given the following method:

public void Foo(Dictionary<string, Func<string?, int>> dict) { }

The compiler will emit the following:

public void Foo(
    [Nullable(new bool[] {
        false, // Dictionary<,>
        false, // string
        false, // Func<,>
        true,  // string
        false  // int
    })] Dictionary<string, Func<string, int>> dict
) { }

So yes, from the CLR's point of view, ValueTuple<string, string?> and ValueTuple<string?, string> are the same type, both ValueTuple<string, string>. If you use that type at a boundary like a field, a property, a parameter or a return type then the compiler will emit the appropriate attributes to provide hints as to the intention and nullability of those values.

[CyrusNajmabadi]

for it to be a genuine "figure out how you want to deal with that area later" feature, there has to be a point in that gradual adoption process where the compiler warns on using ! too.

Sure. I'm sure there will be an analyzer somewhere (or maybe something built in)that says that these should be warnings (or errors). You could then opt in whenever it suited you.

I see no reason why any of this would be aggressively forced on people. The point is to make it so you can gently and inexpensively adopt this at whatever pace makes sense for you. If you don't do that, then people will not be able to opt in at all here.