Nullable reference types: Unconstrained generic causes error CS0453 about csharplang HOT 34 CLOSED

@HaloFour Nullable<Ref<string>> still needs the additional bool in Nullable<T> . I think if this is intended to have CLR support, it'd be better to allow reference types in Nullable<T> and flatten it in memory when T is a reference type.

if the generic type argument is a reference type it would be weird for T? to be something other than that reference type.

What if we change the type if and only if T is not known to be a reference type or value type?

// current case
public void F<T>(T? arg) where T : struct {}
public void F<T>(Nullable<T> arg) where T : struct {}

// arg can't be null when we have nullable reference types
// even though it is known to be a reference type (generic or otherwise)
public void F<T>(T arg) where T : class {}

// so you will need to change it to T?
// but it must not change its type to not break existing code
public void F<T>(T? arg) where T : class {}
public void F<T>([Nullable] T arg) where T : class {}

// when T is not known to be a reference type or value type
// we can take advantage of CLR support and use Nullable<T>
public void F<T>(T? arg) {}
public void F<T>(Nullable<T> arg) {}

from csharplang.

@Joe4evr

The problem is that it's likely that the developer did not intend for ints.FirstOrDefault() to return 0 if the collection is empty. Rather they wanted it to return null.

Well, Enumerable.FirstOrDefault is a bad example since that's exactly what it does. But in many cases the developer might always want to return null, regardless of whether the generic type argument is a value type or a reference type.

from csharplang.

CC @MadsTorgersen

from csharplang.

I seem to recall this being a known issue, that without specific CLR changes it would not be possible to unify nullable value and reference types given that the former can only be represented by Nullable<T> where T must be a value type. In order for the compiler to correctly implement this method either the struct or class generic constraint would have to be specified, and to handle both you'd need to overload as follows:

    public static class Guard
    {
        public static void NotNull<T>(T? value, string name) where T : class
        {
            if (ReferenceEquals(value, null))
            {
                throw new ArgumentNullException(name);
            }
        }

        public static void NotNull<T>(T? value, string name) where T : struct
        {
            if (!value.HasValue)
            {
                throw new ArgumentNullException(name);
            }
        }
    }

Notably, while such helper methods are common, I imagine that the compiler would have no idea that they are guarding against the values being null and thus would not aid flow analysis:

public void DoSomething(Person? person)
{
    Guard.NotNull(person, nameof(person));
    Console.WriteLine($"Hello {person.Name}!"); // CS8202: Possible dereference of a null reference
}

from csharplang.

@HaloFour the compiler currently barfs on such helper method pairs, see #7459

from csharplang.

@orthoxerox

In your case the parameter lists would be identical regardless of the generic constraint. In the code I listed above the parameter lists would be different as the struct generic constraint would force the compiler to treat T? as Nullable<T>, where the class generic constraint would cause the compiler to treat T? as just an annotated T. You're already allowed to overload T and Nullable<T>.

That said I've not attempted to build or play with this feature branch so I don't know if it would compile despite the fact that the signatures would be different.

from csharplang.

Guys, although I appreciate you thinking along, let's try not to make this issue bigger than need be.

1. No changes in overload resolution, as this concerns existing working code.
2. Resharpers [CanBeNull] attribute just translates to [System.Runtime.CompilerServices.NullableAttribute], which the parameter gets decorated with in IL. So no CLR changes and no behavior changes at runtime. The ? (which is just syntactic sugar for the attribute) is only there to guide nullability flow analysis at compile time.
3. object.ReferenceEquals(x, null) and x == null work for both nullable value types and reference types, today, without nullable reference type support. Nothing needs to change, as this mechanism already allows to check both for null.

The Guard example code I posted was just one of the places I ran into this. There are several others of the same type: 1, 2, 3.

@HaloFour Your DoSomething example makes no sense to me: the parameter contract indicates to allow null, and then have the implementation immediately throw when null is passed in?

I'm using the Guard.XXX as a last-resort enforcement at runtime, on public API surface points only: throw when null is passed in anyway, as the compiler does not emit a null check. I don't need calling that method to infer ah well, from here the value must be not-null because that was already in the method contract in the first place. So yes, it's true that the compiler does not infer, but it's not a problem for me.

from csharplang.

@HaloFour Your DoSomething example makes no sense to me: the parameter contract indicates to allow null, and then have the implementation immediately throw when null is passed in?

Good point 😄

from csharplang.

@bkoelman

1. This wouldn't concern existing code as you're not permitted to decorate T with ? today unless T is constrained to struct.
2. The equivalence of T? to [Nullable] T is only possible when T is constrained to a class. I imagine that you would be allowed to skip the syntax candy and annotate the parameter manually which would achieve the same result but probably not cause the compiler to balk at the lack of a constraint.
3. This is true, but that's not the crux of the problem here.

As for my example, I'm just demonstrating that code of that kind would not be useful for flow analysis. It has nothing to do with that specific example or the purpose of Guard.NotNull.

Off-topic but my helper validation methods like that almost always return the passed value, that way I can use them in constructors:

public Foo(Bar? bar) : base(Guard.NotNull(bar, nameof(bar)) { ... }

from csharplang.

The equivalence of T? to [Nullable] T is only possible when T is constrained to a class. I imagine that you would be allowed to skip the syntax candy and annotate the parameter manually which would achieve the same result but probably not cause the compiler to balk at the lack of a constraint.

What does it buy you? As long as you don't have a class constraint you wouldn't be able to assign null to a variable of type T.

from csharplang.

@alrz

What does it buy you?

Disambiguation. T? can be either Nullable<T> or [Nullable] T and the compiler can't possibly know which without the constraint.

Note that I'm not defending the behavior, just describing it.

from csharplang.

@HaloFour Disambiguation for nothing? The point is to be able to use a generic nullable variable short of being a value type or reference type. In your example T woudn't be a "nullable" in any sense.

from csharplang.

@alrz

I'm not sure I understand what you're saying. I'm talking very specifically about T?, not T. The generic type T? is ambiguous without the generic type constraints as it could mean Nullable<T> or [Nullable] T. Whether or not T can be nullable isn't particularly relevant, T? very explicitly can be nullable.

If you're arguing that it shouldn't be that way that's fine, and I don't disagree with that.

from csharplang.

@HaloFour

You can't have these two methods side by side because overloading doesn't depend on the return type:

// null is known to be a valid value as a struct
T? F<T>() where T : struct => null;

// null is known to be a valid value as a class
T? F<T>() where T : class => null;

and you may not be able to make it one method due to nullable reference types' restriction:

T? F<T>() => null; // ERROR

Because as you said, the compiler doesn't know if it's a Nullable<T> or not. Now your suggestion:

[return: Nullable]
T F<T>() => null; // ERROR: T is not even nullable

from csharplang.

@alrz

Sure, you couldn't overload based on the return type like that, but you might be able to if the nullable types were parameters instead.

As for "achieving the same result" I meant in terms of contract (method signature), not in implementation. You're correct that you can't actually explicitly return null from that method. Let's modify that example to:

[return: Nullable]
T F<T>() => default(T);

I'm making the big assumption that the compiler is not going to try to validate the direct use of the attribute in such a fashion. If that's the case then this method would compile. Then the question is how the consumer deals with such a method. Obviously if the generic type argument is a value type then the return value could not be null, but if called with a generic type argument of a reference type perhaps it will respect the attribute and treat the return value as nullable:

string s = F<string>(); // CS8202: Possible dereference of a null reference

I could be wrong, though.

from csharplang.

@alrz

More specific to this issue, I assume that the following implementation of the above method would compile and work:

    public static class Guard
    {
        public static void NotNull<T>([Nullable] T value, string name)
        {
            if (ReferenceEquals(value, null))
            {
                throw new ArgumentNullException(name);
            }
        }
    }

from csharplang.

@HaloFour Still, your example won't work for value types,

int? i = F<int>();  
Assert(i == null);

Because nullable reference types have nothing to do with the type system, unlike nullable value types.

Anywho, I think NotNull is not a good example here because the point is to avoid it. Meanwhile, when you don't expose the explicit nullability via the type system, you didn't actually solve any problem.

As I said in #7445, I believe if you want to take advantage of null checks without breaking existing code the best option is to use attributes and an analyzer (which works just fine already). And keep T? as an actual type to be unified with Nullable<T>.

from csharplang.

@alrz

Actually that example compiles just fine because you can implicitly cast from int to int?. 😄 But the result will never be null.

As said, I'm describing the issue from the perspective of the proposal as it currently is and how I expect it has been implemented thus far. I agree that the attribute/erasure method is not without some big flaws. But I also kind of doubt that we'll see any real changes to that proposal.

from csharplang.

@HaloFour Assertion fails. I expect it to be null but default(int) is 0.

from csharplang.

@alrz

Well that's just a bad expectation on your part. That code is still perfectly legal, and has been since C# 2.0.

Again, the question is whether or not the compiler will care to validate the application of the raw Nullable attribute.

from csharplang.

@HaloFour It does compile but it doesn't do what it supposed to do. Even if the compiler cares to validate the application with respect to Nullable attribute, then there is no way to return null from that method. If you are proposing a conversion with respect to NullableAttribute, then the question is where it should do that even based on the attribute, because there is no way to know that if F is returning 0 or default.

from csharplang.

@alrz

It does exactly what it was coded to do, it returns the default value for the generic type argument. I'm not proposing that it does anything different than it does today. You are right that it cannot explicitly return null, nor can it return null for the default value of a value type, but I've never made any claim that it would or that it should. I'm certainly not proposing any implicit conversion or other behavior. If a developer wants to apply the Nullable attribute in invalid ways that cannot be prevented by the CLR, so be it. The compiler can simply ignore it when consuming that method.

Beyond that I don't really know how to respond to your statements. I've repeated several times that I am discussing this issue from the context of the current state of the proposal, which does not include any changes that may unify nullable value and reference types. I know of your proposals and I've seen no motion on them. The recent commits seem to indicate that the team wishes to implement their proposal as it stands, complete with attributes and flow control, at least for the purposes of getting feedback. If the proposal does change, possibly with CLR changes to support a unified nullable type system, then so be it. I will have future discussions given the context of those changes.

from csharplang.

It does exactly what it was coded to do, it returns the default value for the generic type argument.

@HaloFour You suggested to rewrite [return: Nullable] T F<T>() => null; using default(T) I'm trying to say that it woudn't be the same. That's it. I thought that you told point (2) above as a workaround for this issue. That was where I confused.

from csharplang.

@alrz

I meant that specifically for the code in the sample above, the Guard.NotNull<T> method.

from csharplang.

#9936 asks to make this scenario "work".

from csharplang.

@gafter Thoughts on how that unification could be accomplished short of CLR/BCL changes? I imagine that the compiler could perform some tricks in order to make it possible to use non-constrained T? as a local variable type within a method body, but what about exposing that type as a parameter or return value?

Through CLR/BCL changes I figured the options might be a new struct that would effectively just wrap an object reference, which would allow object references to be stored in Nullable<T>, e.g. Nullable<Ref<string>>. Cleaner than that would be to lift the struct generic constraint on Nullable<T>. But either way if the generic type argument is a reference type it would be weird for T? to be something other than that reference type.

from csharplang.

Since T? doesn't affect the underlying type where it's a reference type the following will be permitted,

interface A<out T> where T : class { T? F(); }

As discussed in this thread, for T? to work with unconstrained generic types, it should also be allowed to be covariant,

interface A<out T>  { T? F(); }

That would make some interesting stuff possible.

from csharplang.

@HaloFour

I seem to recall this being a known issue, that without specific CLR changes it would not be possible to unify nullable value and reference types given that the former can only be represented by Nullable where T must be a value type

I think compiler could workaround this issue by transpiling nullable method without constraint to be 2 method. So we might not need to wait for CLR

public static class Ext
{
    // without struct constraint normally error, so not breaking thing I think
    public static void DoSomething<T>(this T? obj)
    {
    }
}

// transpiled

public static class Ext
{
    public static void DoSomething<T>(this T obj) where T : class
    {
    }

    // replace anywhere using struct into this method instead
    public static void _DoSomething<T>(this T? obj) where T : struct
    {
    }
}

The only downside is, if we have multiple nullable parameter it will need to generate 2^n method while transpiling

from csharplang.

I'm gonna reiterate what I suggested on the csharplang repo.

Why not have it so that methods returning open generic types like these are adorned with the attribute indicating that their return value can be null, but the compiler will just ignore the attribute if the type parameter is a non-nullable value type at the call-site?

void Foo(IEnumerable<int> ints, IEnumerable<string> strings)
{
    var i = ints.FirstOrDefault().GetHashCode(); //collection is of a value type, so no warning
    var s = strings.FirstOrDefault().GetHashCode(); //warning: 'FirstOrDefault()' would return 'null' if the collection was empty
}

from csharplang.

If the developer wanted a "null" for an empty list of ints, they could've used an IEnumerable<int?> instead (in which case, the compiler would warn against a potential null dereference, even if Nullable<T> is just a compiler sleight of hand).

Also, I used FirstOrDefault() as the example there because that's what the design doc for nullable reference types uses as an example of "wat do?" The point was more to demonstrate that a method returning an open generic type could be adorned with the "may return null" attribute, but the compiler could just not emit the null-dereference warning if the type parameter corresponding to the return type is a value type. Consider this slightly more elaborate example:

// Let's say these are in a different assembly, so the compiler only sees the metadata
[CanReturnNull] // Always returns a value, even if it has to be null for reference types
public static TValue GetValueOrDefault<TKey, TValue>(this IDictionary<TKey, TValue> dict, TKey key);

// No attribute: Either will return a non-null for reference types, or doesn't return at all
public static TValue GetValue<TKey, TValue>(this IDictionary<TKey, TValue> dict, TKey key);

// Now you write
void Foo(Dictionary<int, Guid> guids, Dictionary<int, string> strings)
{
    var s = strings.GetValueOrDefault(42).Length; //Warning - GetValueOrDefault() says it *can* return null and its return type parameter is a reference type, so dotting off of its result may produce a NRE
    var g = guids.GetValueOrDefault(42).ToString(); //OK - Guid is a non-nullable struct, so compiler ignores the attribute, because a NRE can't happen here
    var s2 = strings.GetValue(42).Length; //OK - No attribute on GetValue(), so as far as the compiler knows, a returned reference type value is never null, so no warning here either
}

from csharplang.

@Joe4evr

I think we should have new API for nullable added in BCL

As I was post this in corefx https://github.com/dotnet/corefx/issues/35169

from csharplang.

The proposed solution at https://github.com/dotnet/csharplang/blob/master/meetings/2019/LDM-2019-05-15.md#user-content-unconstrained-t sounds like it fits my needs here.

from csharplang.

Unconstrained generic types are allowed to be marked as nullable in C# 9.0, see dotnet/roslyn#29146. Though they don't reflect nullability in the case of value types.

from csharplang.

Closing as implemented in C# 9.0

from csharplang.