Skip to content
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]: Task<T> nullability covariance #3950

Open
1 of 5 tasks
jcouv opened this issue Sep 28, 2020 · 22 comments
Open
1 of 5 tasks

[Proposal]: Task<T> nullability covariance #3950

jcouv opened this issue Sep 28, 2020 · 22 comments

Comments

@jcouv
Copy link
Member

jcouv commented Sep 28, 2020

Task<T> nullability covariance

  • Proposed
  • Prototype: Not Started
  • Implementation: Not Started
  • Specification: Not Started

Summary

Task<T> nullability covariance (LDM tentatively approved, needs design proposal for task-like types)

It should be possible to return a Task<string!> for a Task<string?>.

using System.Threading.Tasks;
#nullable enable
public class C 
{
    public Task<(bool Result, string? Extras)> M() 
    {
        return Task.FromResult((true, "")); // currently produces a warning
    }
}

Motivation

See some examples of this problem: dotnet/roslyn#40759, dotnet/roslyn#40757

Unresolved questions

  • needs design proposal for task-like types

Design meetings

Split issue from #3868
https://github.com/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-09-28.md#nullability-improvements
https://github.com/dotnet/csharplang/blob/main/meetings/2024/LDM-2024-09-06.md#taskt-nullability-covariance

@szalapski
Copy link

szalapski commented Sep 7, 2021

Is there a general proposal for this?

That is, any operation that takes a Foo<T?> should be able to take a Foo<T>, since for reference types, T? is the same type as T. True?

If I supply a non-nullable object as a nullable parameter, it works just fine, but when I supply a generic of a nullable as a generic of a non-nullable, it gives me a warning. It isn't null forgiveness that I need, but the opposite--non-null forgiveness, which I thought should be implicit and not really a conscious thing that we would have to do.

@jnm2
Copy link
Contributor

jnm2 commented Sep 7, 2021

@szalapski I don't think that's true, for the same reason that any operation that takes a Foo<object> might not be able to take a Foo<string>. This is what in and out specify on generic parameters.

Let's say something expects a Foo<T?> because it wants to call Add(T?) and add null to it. Then it is not able to take a Foo<T> or else you end up with a null added to the Foo<T> and no warning about it in any location.

var list = new List<string>();
DoSomething(list); // This should, and does, cause a warning; you can't pass `List<string>` as `List<string?>`
Console.WriteLine(list[0].Length); // Or this happens

void DoSomething(List<string?> list) => list.Add(null);

@szalapski
Copy link

szalapski commented Sep 7, 2021

You are right.

Thinking it through, I intuitively expected that the Foo<T> would be implicitly cast to a Foo<T?> when the nullable version is accepted as a parameter. However, given that it doesn't and can't, how do I explicitly "cast" it? After all, T and T? are the same type, so casting doesn't make sense here.

@jnm2
Copy link
Contributor

jnm2 commented Sep 7, 2021

You use the nullability warning suppression operator, which is postfix !:

DoSomething(list!);

It's called the https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/operators/null-forgiving operator, but I'm calling it the null-blindness operator and hoping it catches on. :D

@szalapski
Copy link

It isn't null forgiveness that I need, but the opposite--non-null forgiveness, which I thought should be implicit and not really a conscious thing that we would have to do.

@jnm2
Copy link
Contributor

jnm2 commented Sep 7, 2021

Nevertheless, the operator is a nullability warning suppression operator and is the way intended for you to suppress this particular nullability warning.

@RikkiGibson
Copy link
Contributor

The nullability improvements working group discussed this issue on 24th October. Here are the notes: https://github.com/dotnet/csharplang/blob/main/meetings/working-groups/nullability-improvements/NI-2022-10-24.md.

@alrz
Copy link
Contributor

alrz commented Oct 26, 2022

Essentially, in Task<string?> task = Task.FromResult("value"), the type of task would be contributed to the inference of TResult in Task Task.FromResult(TResult).

Would that be limited to nullability only?

There's a discussion on that as a general type inference feature: #92 which is what Java does already.

@pentp
Copy link

pentp commented Oct 26, 2022

But why limit it to only nullability covariance? Making Task<T> covariant on the type would be very helpful.
Option 2 (<out T> in class types) doesn't work as discussed, but some attribute that would make the type covariant for external observers might work?

@RikkiGibson
Copy link
Contributor

Would that be limited to nullability only?

There's a discussion on that as a general type inference feature: #92 which is what Java does already.

That's very interesting. Apologies that I missed these comments when they were originally made. I'll try and take a look at this discussion when I can.

@RikkiGibson
Copy link
Contributor

But why limit it to only nullability covariance? Making Task<T> covariant on the type would be very helpful. Option 2 (<out T> in class types) doesn't work as discussed, but some attribute that would make the type covariant for external observers might work?

I think the reason is we feel better about breaking the rules if the feature is limited to just nullability. The result field in a Task is mutable, so on the "language-theoretical" level it's not safe to make it covariant. There might be some kind of guardrail you can insert which makes it "likely enough" that the requirements around safe covariant conversions won't be violated, but it's not clear.

@TahirAhmadov
Copy link

It's probably too late now, but what are the downsides of creating and using something like ITask<T>? I imagine it's a bit more expensive due to virtual calls? Also, this doesn't really work with ValueTask<T>.

I think option 1 is best. Yes, it introduces a certain amount of work, but I think it brings enough benefits to justify it. Option 0 has a smell to it - now the compiler has to remember a list of types, meh - and option 2 doesn't really work for Task<T> as discussed.

@sab39
Copy link

sab39 commented Nov 5, 2022

The working group notes talk about the idea of <out T> in class types in general - is there an existing issue or discussion thread for this? While it may be a "wrecking ball to hit a nail" for this specific scenario, it seems worth exploring in general as a longer-term feature. I can definitely see uses for being able to declare return types of Task<out T> or ImmutableList<out T> or ImmutableDictionary<in TKey, out TValue>, and being restricted by the language into only the members of the type that can be used covariantly or contravariantly in the type in question.

@CyrusNajmabadi
Copy link
Member

I can't see how the immutable collections could use variance. They are not purely covariant. For example: https://learn.microsoft.com/en-us/dotnet/api/system.collections.immutable.immutablelist-1.add?view=net-7.0#system-collections-immutable-immutablelist-1-add(-0)

An ImmutableList of strings is not one of objects. You would not be able to add an object to the actual instance, despite the type implying it was possible.

@sab39
Copy link

sab39 commented Nov 5, 2022

I'm proposing that ImmutableList<out T> is a different type from ImmutableList<T>, that would not allow calling methods like Add that take T as inputs. You'd be able to convert ImmutableList<T> to ImmutableList<out T>, but not in the other direction; you could convert ImmutableList<string> to ImmutableList<out object> without issue because only covariant members could be called. The idea would be that MyClass<out T> would behave roughly like an imaginary interface IMyClass<out T>, implemented by MyClass<T> and consisting of all the members of MyClass<T> that would be legal in such an interface.

@CyrusNajmabadi
Copy link
Member

Interesting concept :-). I could envision something like that being possible.

@TahirAhmadov
Copy link

TahirAhmadov commented Nov 5, 2022

In addition to what @sab39 wrote. other types don't have such a limitation, for example, ReadOnlyDictionary<,>.
PS. What about structs, such as ImmutableArray<T>? Can those be made covariant?

@Joe4evr
Copy link
Contributor

Joe4evr commented Nov 7, 2022

I'm proposing that ImmutableList<out T> is a different type from ImmutableList<T>, that would not allow calling methods like Add that take T as inputs. You'd be able to convert ImmutableList<T> to ImmutableList<out T>, but not in the other direction; you could convert ImmutableList<string> to ImmutableList<out object> without issue because only covariant members could be called. The idea would be that MyClass<out T> would behave roughly like an imaginary interface IMyClass<out T>, implemented by MyClass<T> and consisting of all the members of MyClass<T> that would be legal in such an interface.

This sounds like something interesting, but it seems to touch on full covariance on types, rather than nullable covariance which is purely compiler-sugar but currently a friction point.

One solution for the issue about nullability and Task<T> (but also relevant for some other APIs, even third-party packages) I had thought of some time ago, if it's primarily about creating the object in a nullable-friendly way and not about consumption, is a new nullability attribute that can be applied at the method that creates the task.

Concretely, like the OP says, this produces an warning:

public Task<string?> GetAsync() => Task.FromResult("");

But an attribute could be placed on Task.FromResult on the type parameter:

public static Task<T> FromResult<[NullableCovariant] T>(T value);

The effect here would be that at callsites of this method, if the inferred type argument is a (non-nullable) reference type and the output is target typed to the nullable version of the type argument, the compiler is allowed to lift the inferred type argument to the nullable version.
Simply put, even though FromResult("") currently infers T = string!, the added attribute would make the compiler look at the callsite's target type (similar to what it does for target-typed new()) which it sees to be Task<string?> and so, since the regular type matches and nullability goes from ! to ?, the compiler may change its inference on FromResult to T = string? to match.

@KalleOlaviNiemitalo
Copy link

I'd rather have the attribute on the type parameter of class Task<[NullableCovariant] TResult> so that it would also cover cases like

using System.Threading.Tasks;

static class C {
    static void M(out Task<string?> task) {
        N(out task);
    }

    static void N(out Task<string> task) {
        task = Task.FromResult("");
    }
}

even though I suppose it's quite easy to suppress warnings by adding !.

@sab39
Copy link

sab39 commented Nov 7, 2022

This sounds like something interesting, but it seems to touch on full covariance on types, rather than nullable covariance which is purely compiler-sugar but currently a friction point.

Yeah, I was thinking about as a separate and longer term idea. Should I start a discussion to propose it as such?

@TahirAhmadov
Copy link

TahirAhmadov commented Nov 7, 2022

Come to think of it, the Task.FromResult scenario would also benefit from general covariance:

Task<object> FooAsync()
{
  return Task.FromResult(""); // currently this is an error, but it would be helpful if it worked
}
Task<objecct> BarAsync()
{
  return Task.FromResult(0); // to make this work, it would be outside of the scope of regular covariance, 
  // but would be cool nonetheless
  // if we go with the "target-typing" approach, and not the covariance approach, this should work fine
}

@inf9144
Copy link

inf9144 commented Nov 6, 2023

Anything new here? Got my self in this situation, not only with Task<T> but also with MyType<T>,

Have a method and cant make an overload for this because string is a reference type and those types are identical:
void Xyz<TFrom>(Expression<Func<TFrom, MyType<string>>> param);
void Xyz<TFrom>(Expression<Func<TFrom, MyType<string?>>> param);

My hacky workaround would be something like

void Xyz<TFrom>(Expression<Func<TFrom, MyType<TString>>> param)
  where TString: IEnumerable<char>?

This should really be fixed because it affects all generic types and their usage as parameters.....
string is fullfilling string? so i cant get why this warning appears on the direction of notnull to nullable assignements. It's a bug in my eye, no matter if we talk about variance here.

dotnet/roslyn#40757

//EDIT:
To clarify - because people on the other thread over on roslyn does not seem to get what my problem is.
My problem with this hole in the language syntax is:
You need the possibility to declare a function which can takeT<X?>and does not CS8619 on providing an T<X>. If you dont have something like that - there is now way out of this warning. It hit's every function with a generic type as parameter. If your type does not declare some interface or baseclass and you dont have the luxury to declare your method with T where TX : X? you always hit CS8619.

To clarify more: I dont want those two overloads, but right now i cant express to the compiler, that i want a function that takes T<X?> and T<X> without getting warnings on every call that "does not fit" (in my eyes it does fit because string and string? are the same type and string? can accept even more than string) the nullability.

//EDIT2:
More clarification on the topic variance from my perspective. If you see this as variance issue it would be a problem to just declare in or out on the generic type parameters like you can do on interfaces, because this would not be what you would like to express. You dont want the class working/castable/assignable for T<XBase> or T<XDerived>. This would also come with the cost that every contravariance or covariance in C# brings - you cannot use the generic type in all situations on the generic class/struct anymore. (Return Values, Method Parameters, ...).
Can you understand why i dont match this problem to type variance but to nullability? If this is a variance problem, you would need to have the ability to explicitly define the variance in means of nullability. So having an invariant type which is nullable co-/contravariant. Only then you could relax those restrictions that normally come with covariance/contravariance in general, because (yes on runtime level) you can be sure you have the very same types (in case of reference types).

//EDIT3:
And it would not be nice to only solve this for Task / ValueTask because this hits also custom structs that have those generics and behave like ValueHolder, Optionals or ....

//EDIT4:
Did a large addendum and i now know that my arguments were partially if not completely wrong but i still have a point about handling nullable types covariant while the type system does not handle them this way and made an example of it. See here:
dotnet/roslyn#40757 (comment)

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

No branches or pull requests