New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Nullable reference types and nullability checking #5032

Closed
MadsTorgersen opened this Issue Sep 5, 2015 · 158 comments

Comments

@MadsTorgersen
Contributor

MadsTorgersen commented Sep 5, 2015

Null reference exceptions are rampant in languages like C#, where any reference type can reference a null value. Some type systems separate types into a T that cannot be null and an Option<T> (or similar) that can be null but cannot be dereferenced without an explicit null check that unpacks the non-null value.

This approach is attractive, but is difficult to add to a language where every type has a default value. For instance, a newly created array will contain all nulls. Also, such systems are notorious for problems around initialization and cyclic data structures, unless non-null types are permitted to at least temporarily contain null.

On top of these issues come challenges stemming from the fact that C# already has null-unsafe types, that allow both null values and dereferencing. How can a safer approach be added to the language without breaking changes, without leaving the language more complex than necessary and with a natural experience for hardening existing code against null errors in your own time?

Approach: Nullable reference types plus nullability warnings

The approach suggested here consists of a couple of elements:

  • Add a notion of safe nullable reference types T? to the language, in addition to the current ones T which are from now on non-nullable.
  • Detect and warn on cases where nullable types are dereferenced, or when null is values are assigned to (or left in) non-nullable variables.
  • Offer opt-in means to deal with breaking behavior and compat across versions and assemblies.

The feature will not provide airtight guarantees, but should help find most nullability errors in code. It can be assumed that most values are intended not to be null, so this scheme provides the minimal annotation overhead, leaving current code to be assumed non-nullable everywhere.

Nullable reference types are helpful, in that they help find code that may dereference null and help guard it with null checks. Making current reference types Non-nullable is helpful in that it helps prevent variables from inadvertently containing an null value.

Nullable and non-nullable reference types

For every existing reference type T is now "non-nullable", and there is now a corresponding nullable reference type T?.

Syntactically speaking, nullable reference types don't add much, since nullable value types have the same syntax. However, a few syntactic corner cases are new, like T[]?.

From a semantic viewpoint, T and T? are mostly equivalent: they are the same type in that they have the same domain of values. Specifically, because the system is far from watertight, non-nullable reference types can contain null. The only way in which they differ is in the warnings caused by their use.

For type inference purposes T and T? are considered the same type, except that nullability is propagated to the outcome. If a reference type T is the result of type inference, and at least one of the candidate expressions is nullable, then the result is nullable too.

If an expression is by declaration of type T?, it will still be considered to be of type T if it occurs in a context where by flow analysis we consider it known that it is not null. Thus we don't need new language features to "unpack" a nullable value; the existing idioms in the language should suffice.

string s;
string? ns = "hello";

s = ns; // warning
if (ns != null) { s = ns; } // ok

WriteLine(ns.Length); // warning
WriteLine(ns?.Length); // ok

Warnings for nullable reference types

Values of nullable reference types should not be used in a connection where a) they are not known to (probably) contain a non-null value and b) the use would require them to be non-null. Such uses will be flagged with a warning.

a) means that a flow analysis has determined that they are very likely to not be null. There will be specific rules for this flow analysis, similar to those for definite assignment. It is an open question which variables are tracked by this analysis. Just locals and parameters? All dotted names?

b) means dereferencing (e.g. with dot or invocation) or implicitly converting to a non-nullable reference type.

Warnings for non-nullable reference types

Variables of non-nullable reference types should not be assigned the literal null or default(T); nor should nullable value types be boxed to them. Such uses will result in a warning.

Additionally, fields with a non-nullable reference type must be protected by their constructor so that they are a) not used before they are assigned, and b) assigned before the constructor returns. Otherwise a warning is issued. (As an alternative to (a) we can consider allowing use before assignment, but in that case treating the variable as nullable.)

Note that there is no warning to prevent new arrays of non-nullable reference type from keeping the null elements they are initially created with. There is no good static way to ensure this. We could consider a requirement that something must be done to such an array before it can be read from, assigned or returned; e.g. there must be at least one element assignment to it, or it must be passed as an argument to something that could potentially initialize it. That would at least catch the situation where you simply forgot to initialize it. But it is doubtful that this has much value.

Generics

Constraints can be both nullable and non-nullable reference types. The default constraint for an unconstrained type parameter is object?.

A warning is issued if a type parameter with at least one non-nullable reference constraint is instantiated with a nullable reference type.

A type parameter with at least one non-nullable constraint is treated as a non-nullable type in terms of warnings given.

A type parameter with no non-nullable reference constraints is treated as both a nullable and a non-nullable reference type in terms of warnings given (since it could without warning have been instantiated with either). This means that both sets of warnings apply.

? is allowed to be applied to any type parameter T. For type parameters with the struct constraint it has the usual meaning. For all other type parameters it has this meaning, where S is the type with which T is instantiated:

  • If S is a non-nullable reference type then T? refers to S?
  • Otherwise, S? refers to S

Note: This rule is not elegant - in particular it is bad that the introduction of a struct constraint changes the meaning of ?. But we believe we need it to faithfully express the type of common APIs such as FirstOrDefault().

Opting in and opting out

Some of the nullability warnings warn on code that exists without warnings today. There should be a way of opting out of those nullability warnings for compatibility purposes.

When opting in, assemblies generated should contain a module-level attribute with the purpose of signaling that nullable and non-nullable types in signatures should generate appropriate warnings in consuming code.

When consuming code references an assembly that does not have such a top-level attribute, the types in that assembly should be treated as neither nullable nor non-nullable. That is, neither set of warnings should apply to those types.

This mechanism exists such that code that was not written to work with nullability warnings, e.g. code from a previous version of C#, does indeed not trigger such warnings. Only assemblies that opt in by having the compiler-produced attribute, will cause the nullability warnings to happen in consuming code accessing their signatures.

When warnings haven't been opted in to, the compiler should give some indication that there are likely bugs one would find by opting in. For instance, it could give (as an informational message, not a warning) a count of how many nullability warnings it would have given.

Even when a library has opted in, consuming code may be written with an earlier version of C#, and may not recognize the nullability annotations. Such code will work without warning. To facilitate smooth upgrade of the consuming code, it should probably be possible to opt out of the warnings from a given library that will now start to occur. Again, such per-assembly opt-out could be accompanied by an informational message reminding that nullability bugs may be going unnoticed.

Libraries and compatibility

An example: In my C# client code, I use libraries A and B:

// library A
public class A
{
  public static string M(string s1, string s2);
}

// library B
public class B
{
  public static object N(object o1, object o2);
}

// client C, referencing A and B
Console.WriteLine(A.M("a", null).Length);
Console.WriteLine(B.N("b", null).ToString());

Now library B upgrades to C# 7, and starts using nullability annotations:

// upgraded library B
public class B
{
  public static object? N(object o1, object o2); // o1 and o2 not supposed to be null
}

It is clear that my client code probably has a bug: apparently it was not supposed to pass null to B.N. However, the C# 6 compiler knows nothing of all this, and ignores the module-level attribute opting in to it.

Now I upgrade to C# 7 and start getting warnings on my call to B.N: the second argument shouldn't be null, and I shouldn't dot into the return value without checking it for null. It may not be convenient for me to look at those potential bugs right now; I just want a painless upgrade. So I can opt out of getting nullability warnings at all, or for that specific assembly. On compile, I am informed that I may have nullability bugs, so I don't forget to turn it on later.

Eventually I do, I get my warnings and I fix my bugs:

Console.WriteLine(B.N("b", "")?.ToString());

Passing the empty string instead of null, and using the null-conditional operator to test the result for null.

Now the owner of library A decides to add nullability annotations:

// library A
public class A
{
  public static string? M(string s1, string s2); // s1 and s2 shouldn't be null
}

As I compile against this new version, I get new nullability warnings in my code. Again, I may not be ready for this - I may have upgraded to the new version of the library just for bug fixes - and I may temporarily opt out for that assembly.

In my own time, I opt it in, get my warnings and fix my code. I am now completely in the new world. During the whole process I never got "broken" unless I asked for it with some explicit gesture (upgrade compiler or libraries), and was able to opt out if I wasn't ready. When I did opt in, the warnings told me that I used a library against its intentions, so fixing those places probably addressed a bug.

@MgSam

This comment has been minimized.

MgSam commented Sep 5, 2015

So are the thoughts on the ! operator from the Aug 18 notes not part of the official proposal yet?

Additionally, fields with a non-nullable reference type must be protected by their constructor so that they are a) not used before they are assigned, and b) assigned before the constructor returns. Otherwise a warning is issued. (As an alternative to (a) we can consider allowing use before assignment, but in that case treating the variable as nullable.)

This rule seems like a non-starter to me, as it is impossible for the compiler to verify this if you use a helper method called from the constructor to assign fields. That is unless you provide an in-line way of disabling warnings (essentially telling the compiler that you know for certain the fields are being assigned). ReSharper has such a feature to disable warnings.

@HaloFour

This comment has been minimized.

HaloFour commented Sep 5, 2015

@MgSam Isn't that currently a pain-point with non-nullable fields in Apple Swift? What's worse is that the base constructor could call a virtual method overridden in the current class that could access those fields before they've been assigned so not even the constructor is good enough.

@tpetricek

This comment has been minimized.

tpetricek commented Sep 5, 2015

This looks nice!

Can the proposal be clarified on how are the nullability annotations going to be stored at the CLR level? (I suppose these have no runtime meaning and they would just produce a .NET attribute or some other sort of metadata?)

It would be nice if some future version of the F# compiler could read & understand these and offer some sort of "strict" mode where nullable types are automatically exposed as option<'T> (or something along those lines).

One other possible related idea would be to have an annotation not just when referring to a type, but also when defining a type - for example, types defined in F# do not allow null value (when used safely from F#) and so when the C# compiler sees them, it should (ideally) always treat them as non-nullable, but when working with a type that came from some old .NET library that has nulls everywhere, it makes sense to treat the type as nullable by default. (This would perhaps make things too complicated - but some declaration-site/use-site option might be worth considering...)

@rmschroeder

This comment has been minimized.

rmschroeder commented Sep 5, 2015

Seconding Tomas' points above, this is a great as it stands, but it would be nice if there could be some way to enable a 'strict' mode that guaranteed no nulls and if that could be trusted by consumers of libraries, or is there some way to determine if a dependency did not ignore the warnings?
I would also like to hear about how this would be exposed via metadata, reflection, etc.

@mariusschulz

This comment has been minimized.

mariusschulz commented Sep 5, 2015

What I love about this approach is that non-nullability would be the default for reference types, which I think is a lot better than having to opt in to it explicitly every single time (like string! or object!).

@HaloFour In Swift, you can't call a base class' constructor until all (non-nullable) fields have been initialized. That prevents exactly the problem you mentioned.

@deiruch

This comment has been minimized.

deiruch commented Sep 5, 2015

I like this idea and implementation very much. +1 vote to enable (possible) warnings for arrays of non-nullable references.

@YuvalItzchakov

This comment has been minimized.

YuvalItzchakov commented Sep 5, 2015

First of all, kudos to the language team for this proposal. It definitely looks like effort and time has been put to thinking about this.

Regarding the proposal, It seems to me that non-nullability in C#, given its age and maturity would be confusing to many developers, senior and junior as one. Given that the proposal is mainly around compiler time warnings and not errors, this would lead to even more confusion, as it would feel like something developers can easily ignore. Also, we are so used to nullability being around structs, that suddenly allowing them on reference types can add to that same confusion and peculiar feeling.

Think feature doesn't "feel" like it can be done properly without baking it into the CLR. I think attempts to work around not creating a breaking change wouldn't be as good as it can be.

@FlorianRappl

This comment has been minimized.

FlorianRappl commented Sep 5, 2015

One question that directly appeared was how library authors should handle parameters, which are not supposed to be null, but are given as null.

The example reads:

public static string? M(string s1, string s2); // s1 and s2 shouldn't be null

Now prior to C# 7 we could introduce a runtime check such as:

public static string M(string s1, string s2) // s1 and s2 can be null
{
  if (Object.ReferenceEquals(s1, null))
    throw new ArgumentNullException(nameOf(s1));
  if (Object.ReferenceEquals(s2, null))
    throw new ArgumentNullException(nameOf(s2));
  /* ... */ // won't crash due to s1 or s2 being null
}

This result, of course, in a runtime error if null is passed-in. In C# 7, knowing that the input should be non-null, a warning in generated. Now we could omit these checks and maybe have a crash, which is a consequence of passing in null references, however, since the user has been warned the origin should be obvious.

public static string? M(string s1, string s2) // s1 and s2 shouldn't be null
{
  /* ... */ // could crash due to s1 or s2 being null
}

Is it supposed to be used like that or would the recommendation be to have both kind of checks (a runtime guard and the compiler mechanism to have an initial warning)?

Needless to say that the runtime guard may have other benefits and provides backwards compatibility, but also adds noise that could be reduced with the new approach.

Additionally I agree to @YuvalItzchakov (especially regarding baking it into the CLR).

@sandersaares

This comment has been minimized.

sandersaares commented Sep 5, 2015

@FlorianRappl I imagine the null-checking operator from #5033 would be an ideal match for that scenario if it could also apply to method arguments.

public static string? M(string s1!, string s2!)
{
}

Or maybe with an alternate syntax (inconsistent with null-checking operator elsewhere but potentially more suitable for method arguments):

public static string? M(string! s1, string! s2)
{
}

Such an approach would check for null (at runtime) in addition to emitting non-nullable reference warnings at compile time.

The visuals look a bit odd at first glance but having an easy way to specify "throw if null" is very valuable, especially if we also consider that nullability analysis by the compiler would be one basis for the non-nullable reference type warnings, meaning that you would normally run into the following issue:

public static void Bar(string s) { ... }

public static void Foo(string? s)
{
    Argument.VerifyIsNotNull(s, nameof(s)); // Think green, save energy by saving 1 line of code!

    Bar(s1); // Warning: potentially null value used.
    // The compiler can't tell (without extra annotations) that a null check was done out of band.
}

With the null-checking operator applying to a method argument, this is no problem:

public static void Bar(string s) { ... }

public static void Foo(string s!)
{
    Bar(s1); // The compiler knows that a null check was done, so no problem here.
}
@HaloFour

This comment has been minimized.

HaloFour commented Sep 5, 2015

@mariusschulz It solves one problem by creating another. Since the ctor must come first you're stuck having to initialize those fields via initializers to some arbitrary non-null but still invalid values.

@tpetrina

This comment has been minimized.

tpetrina commented Sep 5, 2015

What if we have two functions defined as

public void foo(string s) {}
public void foo(string? s) {}

Are these two different functions or will this trigger an error? Can we restrict generics to accept only non-nullable references?

@HaloFour

This comment has been minimized.

HaloFour commented Sep 5, 2015

@tpetricek

Considering that the ? is just shorthand for attribute-based metadata those two functions would have the same signature and such an overload would not be permitted.

@ruuda

This comment has been minimized.

ruuda commented Sep 5, 2015

string s;
string? ns = "hello";

s = ns; // warning
if (ns != null) { s = ns; } // ok

I am a big fan of non-nullable types and I applaud this proposal, but in my opinion this is a bad example. If string? ns = SomeFunctionThatReturnsAString() this would make sense, but in the current case perhaps it would be more appropriate to warn about the unnecessary nullability of ns, as it is assigned a non-null value.

@govert

This comment has been minimized.

govert commented Sep 5, 2015

+1 for making non-nullable the default, and not introducing string!. This proposal points us in the right direction, even if the road is long.

Although the scope here is limited to a language feature, one might anticipate a future where the runtime type system is extended to include non-nullable reference types, and where the runtime can enforce type safety for non-nullable types. In other words, an 'airtight' system should still be the end goal.

With such a future in mind, +1 also for the suggestion of @rmschroeder to have an opt-in 'strict' compilation mode which ensures the resulting assembly would (potentially) be type-safe in such a non-nullable-aware runtime. When compiling in 'strict' mode:

  • The warnings should be errors.
  • Arrays should be definitely initialized, perhaps in one of a few compiler-recognized constructs like an appropriate for(;;) loop, or a ToArray() call on an Enumerable of non-nullable values, or a designated array initializer method taking some int => T delegate. So there would be a limited set of statically-checked ways to build a 'strict' non-nullable array.
  • other edge cases be dealt with in a conservative way, that always ensures non-nullable type safety.

References between 'strict' and non-'strict' assemblies are still a problem. At least the compiler should be able to generate a null-checking wrapper that allows a 'strict' assembly to be safely used by a non-'strict' assembly, and vice versa. The wrapper would generate the null check to ensure that void MyMethod(string s) never receives a null when called from another assembly through the wrapper. Maybe the checking code is built into the 'strict' assembly, but not invoked when called from another 'strict' assembly.

While it makes sense to decouple the language feature from having a runtime that is non-nullable-aware, let's also keep in mind where we really want to go. That will allow C# 7.0 to build our trusted assemblies of the future.

@JamesNK

This comment has been minimized.

Member

JamesNK commented Sep 5, 2015

There is already a "strict" mode, it's called treat warnings as errors.

@govert

This comment has been minimized.

govert commented Sep 5, 2015

@JamesNK That could work, if we have:

  • an 'airtight' conservative warning mode that will generate warning when the hard-to check cases fail, like arrays or fields that are not definitely initialized, and
  • something (an attribute) baked into the assembly that indicates it was built in the 'non-nullable-warnings-as-errors-mode' so that the tooling and future infrastructure can recognize these.

I kind of see this like "unsafe" code, which opts out of type safety, and is not implemented merely as warnings.

@naasking

This comment has been minimized.

naasking commented Sep 5, 2015

The discussion of generics is incomplete. For one, it doesn't actually explain how to declare a non-null type parameter. Unconstrained type parameters are T:object? by default, so then if you explicitly constraint it as T:object, it's non-nullable?

Except T:object is currently forbidden as a constraint by [1]: constraints can't be special type "object". I'm all for eliminating that constraint entirely since it makes no sense (along with the sealed class/struct constraint error!).

Finally, creating a guaranteed non-null array seems achievable by using a new array constructor:

public static T[] Array.Create<T>(int length, Action<T[], int> initialize)
    where T : object

Then a simple flow analysis ensures that the unmodified index parameter is used to initialize the array parameter.

[1]

private static bool IsValidConstraintType(TypeConstraintSyntax syntax, TypeSymbol type, DiagnosticBag diagnostics)

@xdaDaveShaw

This comment has been minimized.

xdaDaveShaw commented Sep 5, 2015

Just a thought as someone who uses "Treat Warnings as Errors" on everywhere, there's no mention of how likely false warnings are to appear and how are people going to deal with them. Will it be case of people just sticking ?. instead of . to try and avoid dereferencing null, which could lead people to introduce other subtle bugs if they are not thinking things through.
The last think I'd want to see is a code base full of #pragma disable CSXXXX around blocks of code - it's bad enough when Code Analysis flags false warnings and they need to be suppressed, but at least those have support for a Justification - which can be enforced with another CA rule if needed :).

@danieljohnson2

This comment has been minimized.

danieljohnson2 commented Sep 5, 2015

I think you really should separate the checked-nullable-references (that is: T? for reference type T) from this proposal; there is some value in having a clear indication that this or that field or parameter is expected to be null sometimes, and having the compiler enforce that null is checked for.

But it seems to me that he rest of this is just not viable as given. The proposal is to declare the existing references types non-nullable, without enforcing this at all. All existing C# code is to be declared broken, and to be punished with an avalanche of useless warnings.

I do feel some confidence about that last point: I don't believe you can generate the warnings accurately enough. The false positives will be overwhelming. We'll all wind up silencing that particular warning.

This is a shame. The advantage of non-nullable references, if they are enforced somehow, is not that we are forced to check for null everywhere. It is that we would not need to anymore.

@MiddleTommy

This comment has been minimized.

MiddleTommy commented Sep 5, 2015

I think getting rid of null (mostly) all together is a bad idea. We are so entrenched with it. If your reference type can not be null we will be doing default value checking and error catching more.
We need better automatic ways to handle null.
I think we need something like default values to ease our problem.

public int Count(this IEnumerable items != null) { return ....;}

In this method items is guaranteed to not be null by the compiler. There can be compiler checks to help this but also automatic ArgumentNullException throwing.

I use null a lot instead of throwing exceptions. I always felt exceptions mean something went wrong. Null means something doesn't exist not that that is wrong.

So there will be a host of people who would like the != null to just pass the method by returning a default
public int Count(this IEnumerable items != null ?? 0)
{ return ....;}

This is stating that if items is null don't throw an exception, return the default value instead.

@Eirenarch

This comment has been minimized.

Eirenarch commented Sep 5, 2015

I dislike the proposal. Reliance on warnings, not proving correctness and punishing existing code with a bunch of warnings all feel like we are discussing JavaScript rather than C#. I'd prefer a proposal that works in fewer cases but is proven to be correct and results in errors rather than warnings. The current one seems very confusing to existing programmers and will be very frustrating when the compiler declares their existing project broken.

@ScottArbeit

This comment has been minimized.

ScottArbeit commented Sep 6, 2015

This is brilliant. Can anything be easy and perfect 16 years into the lifespan of C#? No. Is this a robust and minimalist way to accomplish this? Yes. And two years after it's available in C# 7? We'll go through the phase of having warnings, we'll clean up our code, and we'll finally get rid of (almost) all of the null-reference exceptions we're all plagued with. Thank you, Mads, as a .NET developer since Beta 2, I really appreciate it.

@jonathanmarston

This comment has been minimized.

jonathanmarston commented Sep 6, 2015

I like the idea of non-nullable reference types in C#, but I'm not sold on them only being implemented as compiler warnings. Consider the following code:

string GetAsString(object o)
{
    return o.ToString();
}

Under the current proposal the method above would not produce any warnings or errors, but neither the compiler nor the runtime actually provide any guarantees that "o" isn't null.

Yes, the caller to this method would receive a warning if the compiler determines that it is not "very likely" that the value being passed is not null, but what if this method is part of a shared class library? And what if the consumer of this library isn't using C# 7, or ignored the warning? This code would generate an NRE just as it would today.

With the current proposal the writer of the library would still need to add a runtime null check to be safe, even though he was told that a reference type without a question mark is non-nullable. The compiler is just giving the illusion of null-safety without actually providing it. Developers will get used to receiving warnings when they write unsafe code (most of the time), assume that if they don't get them that they wrote null-safe code, and start neglecting to write proper runtime null checks when necessary, in some ways making matters worse than they are today!

@Obertyukh

This comment has been minimized.

Obertyukh commented Oct 4, 2016

2 Major Issues is:

  • Back Compatibility wish C#6 code and or libraries
  • Default values for NonNullable fields

Solution Proposal wish non null guarantee:

Back Compatibility wish C#6 code and libraries can be made using Compiler ability to emit method names (for public methods of public classes) with special characters in it so we can not see those methods in code.

So if this method:

public String StrMagic( String s1, String s2 )
{
    //Do some magic and return str
}

C#7 compiler will emit like:

public String StrMagic( String s1, String s2 )
{
    if(s1 == null || s2 == null)
        throw new ArgumentException();

    return StrMagic>>@@NonNull@@( s1, s2 );
}
public String StrMagic>>@@NonNull@@( String s1, String s2 )
{
    //Do some magic and return str
}

than pre C#7 compiler will always use undecorated methods and never see decorated.
And C#7 compiler and VisualStudio can show to us exactly this decorated method instead of Undecorated and apply all rules of non nullability.

So there we always have checks for null for old code and no checks for code from C#7 and all this is transparent for programmer and just work properly.

Library programmer have guarantee of nun nullability in his code and C#6 customer will have Exceptions on invalid arguments pass.

Default values for NonNullable fields can be made by allowing partial initializing of objects (this is matter for loading grapth of object from saved state when we can not construct objects with correct links in time of construction). Partially initialized Objects must have special type like ( Part<SomeType>, or SomeType^ ) and constructor that construct partially initialized Object must return this type so we know that this object dont safe to pass to functions that expect fully initialized object but we can create functions that can operate on partially initialized object, this will be used mostly in loading Code or in some Big and Hard object initialization routine. But after full initialization we can get fully InitializedObject from partRefernce like this:

SomeType^ myObj = SomeType( ); //partially init because need to set non null fields
myObj.field1 = new Object();//Whatever
myObj.Name = "SOme Name";//Whatever

SomeType finalObj = myObj.FinalizeInitialization( );
//or
SomeType? finalObjNullable = myObj.FinalizeInitialization( );

FinalizeInitialization is special function generated by compiler that checks all nonnullable fields to be non null and return initialized reference to SomeType object;

from now we can pass this object to regular code that have guarantee of full initialization of object.

After this additions we can have not nullability warnings but true and safe nullability errors.

@yaakov-h

This comment has been minimized.

yaakov-h commented Oct 8, 2016

@Obertyukh The mangled method names aren't needed. AFAIK, overloads can also be based on modopts and modreqs.

In theory, we could have nullability annotations turn into modopts and older compilers would recognise them (and pass nulls), or modreqs and older compilers I believe cannot handle them.

Using modopts or modreqs has been discussed before and I think the biggest argument against it is a complete break of compatibility (particularly for the BCL), but if we could have the compiler generate backwards-compatible stubs, that might help.

e.g.

public void Foo(string a, string? b) { .... }

could get transformed into the following (or similar):

public void Foo (nonnull string a, string b) { .... }

public void Foo (string a, string b)
{
    if (a == null)
        throw new ArgumentNullException(nameof(a));
    Foo(a!, b);
}
@series0ne

This comment has been minimized.

series0ne commented Nov 9, 2016

I recently watched a video (The Future of C# and VB.NET) where the presenter was a member of the .NET team and was discussing the proposal for non-nullable reference types.

The proposal was

string? x = null; // nullable string
string x = ""; // non-nullable string

This presents a breaking change, and I for one do not think that this is a good idea!

My proposal for the roslyn and .NET guys would be to introduce an operator to handle this suitably.

string x = null; // nullable string, the way it's always been
string! x = ""; //non-nullable string.

I think this flows nicely with nullable value types too.

DateTime? dt; // nullable datetime.
DateTime dt; // non-nullable datetime.

string x; // nullable string.
string! x; // non-nullable string.

BANG (!) brings your attention to THIS IS NOT NULLABLE!

@HaloFour

This comment has been minimized.

HaloFour commented Nov 9, 2016

@series0ne

That was actually the original form of this proposal. See CodePlex: non-nullable reference types (the one billion $ mistake) and #227.

The argument behind the approach above is that the vast majority of the time today the developer intends for a parameter to be non-nullable. Requiring an explicit decoration for non-nullable references would require that this vast majority then requires additional adornment while the less common case would not. This would likely hamper the adoption of the feature.

There is the additional argument that the single ? suffix would unify the syntax between nullable references and nullable value types.

The changes above don't affect the type system. Existing code would compile and run just as it always did. Non-nullable references would instead of a feature that can be enabled that would perform flow-analysis and warn on potential null dereferencing. It doesn't have particularly sharp teeth but would hopefully at least aid the developer in avoiding common mistakes.

Note that I'm mostly rehashing the conversation here, not advocating for any particular position.

@Joe4evr

This comment has been minimized.

Joe4evr commented Jan 16, 2017

I would like to add something that I've thought of today, which I think could really compliment this feature (and reduce usage of !. to an absolute minimum for opt-ins).

Reading alternate null checks

Some developers use an additional property that indicates whether another property is null or not.

public class Foo
{
    //it is expected that SomeBar can be null
    public Bar? SomeBar { get; }

    //if true, SomeBar is not null
    public bool HasABar { get; }
}

As it stands, the compiler does not know the relation between these two properties, which is why the dammit-operator (!.) is to be introduced alongside this feature.

Proposal: Attribute for these properties/fields

API shape:

[AttributeUsage(AttributeTargets.Property | AttributeTargets.Field)]
public sealed class ChecksNullOnAttribute : Attribute
{
    public string PropertyName { get; }
    public bool TrueMeansNull { get; } //flag for if inverted behavior is wanted
    
    public ChecksNullOnAttribute(string propname, bool trueMeansNull = false);
}

Usage:

[ChecksNullOn(nameof(SomeBar))]
public bool HasABar { get; }

Now the compiler knows there's a relation between HasABar and the property named SomeBar that can be used during the static analysis.

void M(Foo foo)
{
    if (foo.HasABar) //compiler now knows this is equal to `foo.SomeBar != null`
    {
        Console.WriteLine(foo.SomeBar.ToString()); //safe to dereference `SomeBar`, no `!.` required
    }
}

Open questions

  • Is it possible/worthwhile to make the compiler check that the attribute is used only on bool fields/properties?
  • Should the compiler auto-emit this attribute if it encounters the most straightforward of cases:
    bool PropNotNull => SomeProp != null; / bool PropIsNull => SomeProp == null;?
@aluanhaddad

This comment has been minimized.

aluanhaddad commented Jan 16, 2017

@Joe4evr that is a good suggestion but unfortunately it introduces a race condition.

@gafter

This comment has been minimized.

Member

gafter commented Jan 17, 2017

I would like to add something that I've thought of today, which I think could really compliment this feature (and reduce usage of !. to an absolute minimum for opt-ins).

@Joe4evr This issue does not propose any ! syntax for opt-ins, What feature is your proposal intended to complement?

@Joe4evr

This comment has been minimized.

Joe4evr commented Jan 17, 2017

that is a good suggestion but unfortunately it introduces a race condition.

Not if the properties are immutable, but the compiler has no special knowledge of that either. ¯\_(ツ)_/¯
If the property was mutable, shouldn't if (foo.SomeBar != null) { /* deref SomeBar here */ } have the exact same race condition?

This issue does not propose any ! syntax for opt-ins, What feature is your proposal intended to complement?

Really? I could swear that every time I hear Mads talk about Nullable Reference Types, he adds that the plan is to include a "dammit-operator" !. so that when you opt-in to get this static analysis, you can override a compiler warning in cases where a potential null dereference is guarded in a way the compiler can't see.

My proposal is intended to compliment Nullable Reference Types as a whole by making the compiler aware of at least one alternate null checking pattern, so that there can be less cases where !. is genuinely warranted, increasing the source code compatibility when a developer enables these warnings.

@aluanhaddad

This comment has been minimized.

aluanhaddad commented Jan 17, 2017

Not if the properties are immutable, but the compiler has no special knowledge of that either. ¯_(ツ)_/¯
If the property was mutable, shouldn't if (foo.SomeBar != null) { /* deref SomeBar here */ } have the exact same race condition?

I believe the idea is that if (foo.SomeBar != null) would introduce a temp referring to foo.SomeBar. An immutable property may still change over time.

Really? I could swear that every time I hear Mads talk about Nullable Reference Types, he adds that the plan is to include a "dammit-operator" !. so that when you opt-in to get this static analysis, you can override a compiler warning in cases where a potential null dereference is guarded in a way the compiler can't see.

Yeah I saw a talk where he discussed the "damnit" operator. Perhaps that corresponds to a different issue, but I also thought it was in this issue. I can't find it however...

@Joe4evr

This comment has been minimized.

Joe4evr commented Jan 17, 2017

An immutable property may still change over time.

Doesn't setting a readonly backing field after an object has been initialized require full trust Reflection? Which isn't impossible, but seems like the most extremely rare of cases where this could go wrong to me. Otherwise the use of readonly is completely undermined and you might as well teach developers that it's not going to help thread-safety at all.

@gafter

This comment has been minimized.

Member

gafter commented Jan 17, 2017

Yes, I forgot the dammit operator x!.y is under consideration as part of this, to silence the compiler when the compiler thinks x might be null and the programmer is OK with a NullReferenceException if it is.

@mattwar

This comment has been minimized.

Contributor

mattwar commented Jan 17, 2017

@Joe4evr How common of a pattern is this? It does seem worthy to look for other patterns that prove null/non-null-ness and potentially make the compiler aware of them if they are common.

@Joe4evr

This comment has been minimized.

Joe4evr commented Jan 21, 2017

@mattwar Well to be honest, it's not like I surveyed any other developers, I just thought that this pattern seems useful enough that others might have adopted it independently.

Although I have seen a variation in a lib I'm heavily using at the moment, where a message can be put through a pipeline and every step of the pipeline returns a Result object indicating if that step succeeded or not. All these Result objects are consolidated under an IResult interface looking something like this:

public interface IResult
{
    public bool IsSuccess { get; }
    public string ErrorReason { get; }
}

Which gets used like so:

var result = await ProcessMessageAsync(msg);

if (!result.IsSuccess)
{
    //Log result.ErrorReason however you like

    //inside of the pipeline the same thing is checked and
    //either returns `result` or `continue`s if inside a loop
}
@Richiban

This comment has been minimized.

Richiban commented Feb 3, 2017

@Joe4evr

Some developers use an additional property that indicates whether another property is null or not.

This is exactly the situation that is also solved by discriminate unions. I'd much rather DUs were introduced to C# and the need for such a feature as you're proposing is obviated.

Your example written in a DU might look something like:

public enum class Foo
{
    None,
    WithBar(Bar bar)
}

or

public enum class Result
{
    Success,
    Failure(string errorReason)
}
@gafter

This comment has been minimized.

Member

gafter commented Mar 27, 2017

This is now tracked by dotnet/csharplang#36. It is championed by @mattwar .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment