Permalink
Browse files

Remove implicit casts from ValueTask<T>

The C# team is considering adding a feature to C# that would allow for arbitrary types to be used as the return type from async methods, with ValueTask<T> being a key motivator.  As part of this, they're considering overload resolution in a situation like:
```C#
async Task Method(Func<Task<int>> f);
...
Method(async () =>
{
    await Blah();
    return 3;
});
```
where a developer previously had a method that accepted a ```Func<Task<int>>```, and then with ```ValueTask<T>``` adds an overload like:
```C#
async Task Method(Func<ValueTask<int>> f);
```
Presumably the ```ValueTask<int>``` overload should be preferred in this case.  However, if a developer added another overload:
```C#
async Task Method(Func<IAsyncOperation<int>> f);
```
(assuming IAsyncOperation could be used in a return position), we would not want that one preferred.  Thus, if this situation should not be considered ambiguous and require the developer to cast/type at the call site to specify which overload should be used (which might be the right answer), there would need to be some tie-breaking mechanism.

The current favorite mechanism of the rest of the C# design team besides me is using implicit conversions as the way to signal this to the compiler, such that if there's an implicit conversion from ```B``` to ```A``` and overloads that take a ```Func<B>``` and ```Func<A>```, ```Func<B>``` would be preferred.  For this to work with ```Task<T>``` and ```ValueTask<T>```, ```ValueTask<T>``` would need an implicit cast to ```Task<T>``` and could not have one in the other direction from ```Task<T>``` to ```ValueTask<T>```.

Today, ```ValueTask<T>``` has an implicit cast from ```Task<T>``` to ```ValueTask<T>```, just as it has an implicit cast from ```T``` to ```ValueTask<T>```.  A ```ValueTask<T>``` is just a wrapper for both a ```T``` or a ```ValueTask<T>``` (it has a field for each), and so this is just a quick way of creating that wrapper from either of the two members that make up the discriminated union.  In this direction, no information is lost, there are no heap allocations, etc.

In the other direction, implicitly casting a ```ValueTask<T>``` to a ```Task<T>``` may or may not be allocating: if the ```ValueTask<T>``` was created from a ```Task<T>```, then implicitly casting to a ```Task<T>``` would just return the stored task.  But if the ```ValueTask<T>``` was created from a value (which is the most common case in scenarios where ```ValueTask<T>``` is used, as it's used in places where you expect synchronous completion and thus to have a value rather than a task), then implicitly casting to a ```Task<T>``` would result in needing to allocate that task as part of the implicit cast, leading to implicit and silent allocations.

I very much dislike the idea of adding such an implicit cast from ```ValueTask<T>``` to ```Task<T>```.  However, by having the implicit cast in the opposite direction, we shut the door on being able to add such an implicit cast in the future for such overload resolution purposes.  Thus, this commit removes the current implicit casts in order to allow us to decide to add back either (or both) directions in the future.
  • Loading branch information...
stephentoub committed May 27, 2016
1 parent 395c645 commit eecacdc6432036b8e44efa29f3a06d8c1c9cfeca
@@ -3,7 +3,6 @@
// See the LICENSE file in the project root for more information.
using System.Collections.Generic;
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
@@ -79,18 +78,6 @@ public ValueTask(Task<TResult> task)
_result = default(TResult);
}
/// <summary>Implicit operator to wrap a <see cref="ValueTask{TResult}"/> around a task.</summary>
public static implicit operator ValueTask<TResult>(Task<TResult> task)
{
return new ValueTask<TResult>(task);
}
/// <summary>Implicit operator to wrap a <see cref="ValueTask{TResult}"/> around a result.</summary>
public static implicit operator ValueTask<TResult>(TResult result)
{
return new ValueTask<TResult>(result);
}
/// <summary>Returns the hash code for this instance.</summary>
public override int GetHashCode()
{
@@ -74,68 +74,6 @@ public void CreateFromNullTask_Throws()
Assert.Throws<ArgumentNullException>(() => new ValueTask<string>((Task<string>)null));
}
[Fact]
public void CastFromValue_IsRanToCompletion()
{
ValueTask<int> t = 42;
Assert.Equal(42, t.Result);
Assert.True(t.IsCompleted);
Assert.True(t.IsCompletedSuccessfully);
Assert.False(t.IsFaulted);
Assert.False(t.IsCanceled);
}
[Fact]
public void CastFromCompletedTask_IsRanToCompletion()
{
ValueTask<int> t = Task.FromResult(42);
Assert.Equal(42, t.Result);
Assert.True(t.IsCompleted);
Assert.True(t.IsCompletedSuccessfully);
Assert.False(t.IsFaulted);
Assert.False(t.IsCanceled);
}
[Fact]
public void CastFromFaultedTask_IsNotRanToCompletion()
{
ValueTask<int> t = Task.FromException<int>(new FormatException());
Assert.Throws<FormatException>(() => t.Result);
Assert.True(t.IsCompleted);
Assert.False(t.IsCompletedSuccessfully);
Assert.True(t.IsFaulted);
Assert.False(t.IsCanceled);
}
[Fact]
public void CastFromCanceledTask_IsNotRanToCompletion()
{
ValueTask<int> t = Task.FromCanceled<int>(new CancellationToken(true));
Assert.Throws<TaskCanceledException>(() => t.Result);
Assert.True(t.IsCompleted);
Assert.False(t.IsCompletedSuccessfully);
Assert.False(t.IsFaulted);
Assert.True(t.IsCanceled);
}
[Fact]
public void CastFromNullTask_Throws()
{
Task<int> ti = null;
Assert.Throws<ArgumentNullException>(() => (ValueTask<int>)ti);
Task<string> ts = null;
Assert.Throws<ArgumentNullException>(() => (ValueTask<string>)ts);
}
[Fact]
public void CreateFromTask_AsTaskIdempotent()
{
@@ -193,7 +131,7 @@ public void Awaiter_OnCompleted()
// OnCompleted typically won't be used by await, so we add an explicit test
// for it here.
ValueTask<int> t = 42;
ValueTask<int> t = new ValueTask<int>(42);
var mres = new ManualResetEventSlim();
t.GetAwaiter().OnCompleted(() => mres.Set());
Assert.True(mres.Wait(10000));
@@ -208,7 +146,7 @@ public void ConfiguredAwaiter_OnCompleted(bool continueOnCapturedContext)
// OnCompleted typically won't be used by await, so we add an explicit test
// for it here.
ValueTask<int> t = 42;
ValueTask<int> t = new ValueTask<int>(42);
var mres = new ManualResetEventSlim();
t.ConfigureAwait(continueOnCapturedContext).GetAwaiter().OnCompleted(() => mres.Set());
Assert.True(mres.Wait(10000));
@@ -223,7 +161,7 @@ public async Task Awaiter_ContinuesOnCapturedContext()
SynchronizationContext.SetSynchronizationContext(tsc);
try
{
ValueTask<int> t = 42;
ValueTask<int> t = new ValueTask<int>(42);
var mres = new ManualResetEventSlim();
t.GetAwaiter().OnCompleted(() => mres.Set());
Assert.True(mres.Wait(10000));
@@ -247,7 +185,7 @@ public async Task ConfiguredAwaiter_ContinuesOnCapturedContext(bool continueOnCa
SynchronizationContext.SetSynchronizationContext(tsc);
try
{
ValueTask<int> t = 42;
ValueTask<int> t = new ValueTask<int>(42);
var mres = new ManualResetEventSlim();
t.ConfigureAwait(continueOnCapturedContext).GetAwaiter().OnCompleted(() => mres.Set());
Assert.True(mres.Wait(10000));
@@ -263,90 +201,90 @@ public async Task ConfiguredAwaiter_ContinuesOnCapturedContext(bool continueOnCa
[Fact]
public void GetHashCode_ContainsResult()
{
ValueTask<int> t = 42;
ValueTask<int> t = new ValueTask<int>(42);
Assert.Equal(t.Result.GetHashCode(), t.GetHashCode());
}
[Fact]
public void GetHashCode_ContainsTask()
{
ValueTask<string> t = Task.FromResult("42");
ValueTask<string> t = new ValueTask<string>(Task.FromResult("42"));
Assert.Equal(t.AsTask().GetHashCode(), t.GetHashCode());
}
[Fact]
public void GetHashCode_ContainsNull()
{
ValueTask<string> t = (string)null;
ValueTask<string> t = new ValueTask<string>((string)null);
Assert.Equal(0, t.GetHashCode());
}
[Fact]
public void OperatorEquals()
{
Assert.True((ValueTask<int>)42 == (ValueTask<int>)42);
Assert.False((ValueTask<int>)42 == (ValueTask<int>)43);
Assert.True(new ValueTask<int>(42) == new ValueTask<int>(42));
Assert.False(new ValueTask<int>(42) == new ValueTask<int>(43));
Assert.True((ValueTask<string>)"42" == (ValueTask<string>)"42");
Assert.True((ValueTask<string>)(string)null == (ValueTask<string>)(string)null);
Assert.True(new ValueTask<string>("42") == new ValueTask<string>("42"));
Assert.True(new ValueTask<string>((string)null) == new ValueTask<string>((string)null));
Assert.False((ValueTask<string>)"42" == (ValueTask<string>)(string)null);
Assert.False((ValueTask<string>)(string)null == (ValueTask<string>)"42");
Assert.False(new ValueTask<string>("42") == new ValueTask<string>((string)null));
Assert.False(new ValueTask<string>((string)null) == new ValueTask<string>("42"));
Assert.False((ValueTask<int>)42 == (ValueTask<int>)Task.FromResult(42));
Assert.False((ValueTask<int>)Task.FromResult(42) == (ValueTask<int>)42);
Assert.False(new ValueTask<int>(42) == new ValueTask<int>(Task.FromResult(42)));
Assert.False(new ValueTask<int>(Task.FromResult(42)) == new ValueTask<int>(42));
}
[Fact]
public void OperatorNotEquals()
{
Assert.False((ValueTask<int>)42 != (ValueTask<int>)42);
Assert.True((ValueTask<int>)42 != (ValueTask<int>)43);
Assert.False(new ValueTask<int>(42) != new ValueTask<int>(42));
Assert.True(new ValueTask<int>(42) != new ValueTask<int>(43));
Assert.False((ValueTask<string>)"42" != (ValueTask<string>)"42");
Assert.False((ValueTask<string>)(string)null != (ValueTask<string>)(string)null);
Assert.False(new ValueTask<string>("42") != new ValueTask<string>("42"));
Assert.False(new ValueTask<string>((string)null) != new ValueTask<string>((string)null));
Assert.True((ValueTask<string>)"42" != (ValueTask<string>)(string)null);
Assert.True((ValueTask<string>)(string)null != (ValueTask<string>)"42");
Assert.True(new ValueTask<string>("42") != new ValueTask<string>((string)null));
Assert.True(new ValueTask<string>((string)null) != new ValueTask<string>("42"));
Assert.True((ValueTask<int>)42 != (ValueTask<int>)Task.FromResult(42));
Assert.True((ValueTask<int>)Task.FromResult(42) != (ValueTask<int>)42);
Assert.True(new ValueTask<int>(42) != new ValueTask<int>(Task.FromResult(42)));
Assert.True(new ValueTask<int>(Task.FromResult(42)) != new ValueTask<int>(42));
}
[Fact]
public void Equals_ValueTask()
{
Assert.True(((ValueTask<int>)42).Equals((ValueTask<int>)42));
Assert.False(((ValueTask<int>)42).Equals((ValueTask<int>)43));
Assert.True(new ValueTask<int>(42).Equals(new ValueTask<int>(42)));
Assert.False(new ValueTask<int>(42).Equals(new ValueTask<int>(43)));
Assert.True(((ValueTask<string>)"42").Equals((ValueTask<string>)"42"));
Assert.True(((ValueTask<string>)(string)null).Equals((ValueTask<string>)(string)null));
Assert.True(new ValueTask<string>("42").Equals(new ValueTask<string>("42")));
Assert.True(new ValueTask<string>((string)null).Equals(new ValueTask<string>((string)null)));
Assert.False(((ValueTask<string>)"42").Equals((ValueTask<string>)(string)null));
Assert.False(((ValueTask<string>)(string)null).Equals((ValueTask<string>)"42"));
Assert.False(new ValueTask<string>("42").Equals(new ValueTask<string>((string)null)));
Assert.False(new ValueTask<string>((string)null).Equals(new ValueTask<string>("42")));
Assert.False(((ValueTask<int>)42).Equals((ValueTask<int>)Task.FromResult(42)));
Assert.False(((ValueTask<int>)Task.FromResult(42)).Equals((ValueTask<int>)42));
Assert.False(new ValueTask<int>(42).Equals(new ValueTask<int>(Task.FromResult(42))));
Assert.False(new ValueTask<int>(Task.FromResult(42)).Equals(new ValueTask<int>(42)));
}
[Fact]
public void Equals_Object()
{
Assert.True(((ValueTask<int>)42).Equals((object)(ValueTask<int>)42));
Assert.False(((ValueTask<int>)42).Equals((object)(ValueTask<int>)43));
Assert.True(new ValueTask<int>(42).Equals((object)new ValueTask<int>(42)));
Assert.False(new ValueTask<int>(42).Equals((object)new ValueTask<int>(43)));
Assert.True(((ValueTask<string>)"42").Equals((object)(ValueTask<string>)"42"));
Assert.True(((ValueTask<string>)(string)null).Equals((object)(ValueTask<string>)(string)null));
Assert.True(new ValueTask<string>("42").Equals((object)new ValueTask<string>("42")));
Assert.True(new ValueTask<string>((string)null).Equals((object)new ValueTask<string>((string)null)));
Assert.False(((ValueTask<string>)"42").Equals((object)(ValueTask<string>)(string)null));
Assert.False(((ValueTask<string>)(string)null).Equals((object)(ValueTask<string>)"42"));
Assert.False(new ValueTask<string>("42").Equals((object)new ValueTask<string>((string)null)));
Assert.False(new ValueTask<string>((string)null).Equals((object)new ValueTask<string>("42")));
Assert.False(((ValueTask<int>)42).Equals((object)(ValueTask<int>)Task.FromResult(42)));
Assert.False(((ValueTask<int>)Task.FromResult(42)).Equals((object)(ValueTask<int>)42));
Assert.False(new ValueTask<int>(42).Equals((object)new ValueTask<int>(Task.FromResult(42))));
Assert.False(new ValueTask<int>(Task.FromResult(42)).Equals((object)new ValueTask<int>(42)));
Assert.False(((ValueTask<int>)42).Equals((object)null));
Assert.False(((ValueTask<int>)42).Equals(new object()));
Assert.False(((ValueTask<int>)42).Equals((object)42));
Assert.False(new ValueTask<int>(42).Equals((object)null));
Assert.False(new ValueTask<int>(42).Equals(new object()));
Assert.False(new ValueTask<int>(42).Equals((object)42));
}
[Fact]
@@ -3031,7 +3031,7 @@ private enum ParseTextFunction
{
if (!task.IsSuccess())
{
return ParseTextAsync_AsyncFunc(task);
return new ValueTask<ValueTuple<int, int, int, bool>>(ParseTextAsync_AsyncFunc(task));
}
outOrChars = _lastParseTextState.outOrChars;
@@ -3057,9 +3057,9 @@ private enum ParseTextFunction
task = ParseTextAsync_Surrogate(outOrChars, chars, pos, rcount, rpos, orChars, c);
break;
case ParseTextFunction.NoValue:
return ParseText_NoValue(outOrChars, pos);
return new ValueTask<ValueTuple<int, int, int, bool>>(ParseText_NoValue(outOrChars, pos));
case ParseTextFunction.PartialValue:
return ParseText_PartialValue(pos, rcount, rpos, orChars, c);
return new ValueTask<ValueTuple<int, int, int, bool>>(ParseText_PartialValue(pos, rcount, rpos, orChars, c));
}
}
}

0 comments on commit eecacdc

Please sign in to comment.