dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.13k stars 4.7k forks source link

Non-nullable reference types support #7988

Closed fubar-coder closed 11 months ago

fubar-coder commented 7 years ago

Maybe it's the ideal time to add non-nullable reference types when the CLR already needs to be changed for dotnet/runtime#7734?

This would allow dotnet/csharplang#36 to avoid the creation of if (ReferenceEquals(x, null)) throw ... code which would penalize the usage of non-nullable references.

shmuelie commented 7 years ago

While this would be great from a forward compat perspective dotnet/csharplang#36 would still need to work downlevel so it wouldn't remove the need for that.

fubar-coder commented 7 years ago

When implemented/supported in/by the CLR:

When implemented by the compiler:

mikedn commented 7 years ago

The CLR could make the null check. This has the advantage that it will only be done when really needed (e.g. when passing a "may-be-null" to a "not-null" reference variable

And how does CLR know it's really needed or not?

shmuelie commented 7 years ago

@fubar-coder So now I can only use this feature if I target CLR vNext? I'd rather have the slow down (which honestly on a modern CPU is nothing) than need a newer CLR on a machine.

fubar-coder commented 7 years ago

@SamuelEnglard We already need a new CLR for dotnet/runtime#7734. So why not doing non-nullable reference types the right way?

fubar-coder commented 7 years ago

@mikedn It should be specified in the assembly meta data if the type is a non-nullable reference type. When the value it has is already of a non-nullable ref type, then it doesn't need to produce code to check if it's null.

shmuelie commented 7 years ago

@fubar-coder because we have no idea when dotnet/runtime#7734 will land. CLR changes are extremely slow moving and hard to get through. While I'm not against the idea of the CLR having the ability to be aware of non-nullable ref types C#'s feature should not be tied to it. That feature is already going to take long enough as it is.

fubar-coder commented 7 years ago

@SamuelEnglard The proposed feature is already there when you use ReSharper. IOW: You don't have an advantage when you already use R#. And when we use the "fake" non-nullable-ref types, then we're in a situation similar to Java with its generics and its type erasure (which was AFAIK considered as a possible implementation), but the F# guys pushed for the "real thing".

EDIT: IMHO it's better to already have the feature baked-in and let the programming language(s) catch up. Later, the CLR guys won't have much interest in adding this feature.

EDIT 2: According to dotnet/csharplang#52 it's targeted for C# 8.

mikedn commented 7 years ago

It should be specified in the assembly meta data if the type is a non-nullable reference type. When the value it has is already of a non-nullable ref type, then it doesn't need to produce code to check if it's null.

If the CLR can avoid the null check in the given circumstances what stops the C# compiler from doing the same thing?

Non-nullability is largely a language thing because it's the language that's supposed to stop you to pass a null where you should not. The runtime cannot do that, except by throwing exceptions and that doesn't solve anything.

shmuelie commented 7 years ago

@fubar-coder I'm going to "step back" here since I fully admit my issues here are less how would this work/is this a good idea for CLR and more SHOULD the CLR do this. That's a debate that can best happen after I think.

fubar-coder commented 7 years ago

@mikedn It's about the guarantees given by the CLR. When your function wants a non-nullable ref type, then you shouldn't be able to get null. Especially in combination with code from older compilers only the CLR is able to provide this guarantees, not the compiler.

Edit: non-nullability from a language is convention. From the CLR, it's a guarantee. The CLR already throws exceptions, so this is nothing unheard of.

mikedn commented 7 years ago

It's still not very clear why the VM has to do this rather than the C# compiler. There may be a good reason for this if you're willing to end up with a NullReferenceException rather than some custom exception (e.g. ArgumentNullException).

The thing is that the runtime (well, the JIT actually) can produce null checks that are slightly cheaper than if (x == null) throw....

Considering a method like void foo(string! s) (or whatever the syntax for non-nullable reference type will be) the JIT could generate code like:

mov ecx, ...   ; get this from somewhere
mov edx, ...   ; get the s argument from somewhere
cmp [edx], edx ; null check s
call foo       ; call foo

If s happens to be null then a NullReferenceException will be generated at the callsite, similar to the NullReferenceException you get when calling on a null this.

And since the JIT is in charge of generating all native code, no matter what managed compiler produced the IL then you'd be guaranteed that s is never null inside foo, even if called from a managed language that doesn't understand non-nullable references and doesn't generate null checks by itself.

fubar-coder commented 7 years ago

That's exactly my point. I want cheap null checks (or none at all). When the CLR knows - in your example - that edx can never be null, because the callers variable is a non-nullable reference type, then the check can be removed too.

If the C# compiler must generate those, then those will become expensive, because they must be done inside foo like this:

public void foo(string s) {
  if (s == null) throw ArgumentNullException(nameof(s));
  // do something with s
}

Regarding the exception thrown when trying to pass a null to a non-nullable reference type, a NullReferenceException is perfectly fine. A simple ArgumentNullException (with default message) and the argument in question might be OK even though it might be slower, because you'll lose this disadvantage as soon as the caller uses non-nullable reference types too.

BTW: This sounds like a misunderstanding. The C# compiler for me is Roslyn which only produces IL. The machine code is only produced by the CLR using its JIT compiler (if available) for the given platform.

mikedn commented 7 years ago

When the CLR knows - in your example - that edx can never be null, because the callers variable is a non-nullable reference type, then the check can be removed too.

Yes, the JIT is already capable of generating and eliminating null checks in some cases so it seems that this could build upon existing infrastructure.

If the C# compiler must generate those, then those will become expensive, because they must be done inside foo like this:

Yes, and the JIT has no way to eliminate those because it doesn't know what code will call foo since it doesn't do inter-procedural analysis.

Sounds like a good plan and perhaps can be extended to other cases beyond method calls. For example, assigning to a non-nullable field could also generate a null check.

@jkotas Opinions?

jkotas commented 7 years ago
ldvirtftn Object.ToString
pop
AaronRobinsonMSFT commented 5 years ago

@jkotas Is there something we can link this to for 3.0? I think this support is actually in unless I am missing something.

jkotas commented 5 years ago

The C# 8 nullable reference types annotations have been aimed at diagnostics. They are not used for any optimizations today.

This issue is about using these annotations for optimizations. I think that the most promising path for that is https://github.com/dotnet/coreclr/issues/17469#issuecomment-481481078 .

ghost commented 1 year ago

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.

ghost commented 11 months ago

This issue will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.