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

[API Proposal]: Allow method built with an AsyncMethodBuilder to return something else than T #95472

Closed
dr1rrb opened this issue Nov 30, 2023 · 3 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Threading.Tasks

Comments

@dr1rrb
Copy link

dr1rrb commented Nov 30, 2023

Background and motivation

When you implement a custom "Async Method Builder" to create your own instance of MyTask<T> the single type that the user can return in the function is T.

But, we could perfectly imagine cases where we can create a MyTask<T> with something else than T, for instance a T? (or a custom Option<T>).

My uses case is where MyTask<T> will implement IOtherInterface<GenericType<T>>. Restricting async method builder to only T means either:

  • Users will have to write MyTask<GenericType<T>>, which not possible since MyTask must have only one 1 type parameter, we cannot have type constraints on MyTask<T> where T : GenericType<T???>;
  • Having an implicit cast from GenericType<T> to T but this will result in a lost of information.

API Proposal

A method that uses an async method builder should accept that the user return any type for which there is a SetResult on the builder.

namespace MyLibrary;

[AsyncMethodBuilder(typeof(MyTaskMethodBuilder<>))]
public class MyTask<T>
{
	public void SetResult(T? value) { }
	public void SetResult(Option<T> value) { }
	public void SetException(Exception error) { }
}

public enum OptionType
{
	None,
	Some,
}

public record struct Option<T>(OptionType Type, T? Value)
{
	public static Option<T> Some(T value) => new(OptionType.Some, value);
	public static Option<T> None() => new(OptionType.None, default);
}

[EditorBrowsable(EditorBrowsableState.Never)]
public readonly struct MyTaskMethodBuilder<T>
{
	private readonly MyTask<T> _task = new();

	public MyTaskMethodBuilder() { }

	public MyTask<T> Task => _task;

	public static FeedMethodBuilder<T> Create()
		=> new();

	public void Start<TStateMachine>(ref TStateMachine stateMachine)
		where TStateMachine : IAsyncStateMachine
		=> stateMachine.MoveNext();

	public void SetStateMachine(IAsyncStateMachine stateMachine) { }

	public void SetException(Exception exception)
		=> _task.SetException(exception);

	public void SetResult(T? result)
		=> _task.SetResult(result);

	public void SetResult(Option<T> result)
		=> _task.SetResult(result);

	public void AwaitOnCompleted<TAwaiter, TStateMachine>(
		ref TAwaiter awaiter, ref TStateMachine stateMachine)
		where TAwaiter : INotifyCompletion
		where TStateMachine : IAsyncStateMachine
		=> awaiter.OnCompleted(stateMachine.MoveNext);

	public void AwaitUnsafeOnCompleted<TAwaiter, TStateMachine>(
		ref TAwaiter awaiter, ref TStateMachine stateMachine)
		where TAwaiter : ICriticalNotifyCompletion
		where TStateMachine : IAsyncStateMachine
		=> awaiter.OnCompleted(stateMachine.MoveNext);
}

API Usage

namespace MyUserApp;

public class MyUserClass
{
	public async MyTask<int> WhenReturnInt() => 42;
	public async MyTask<int> WhenReturnNullableInt() => default(int?);
	public async MyTask<int> WhenReturnOption() => Option<int>.None();
}

Alternative Designs

Two alternatives are possible:

  1. Instead of supporting "any type for which there is a SetResult on the builder", we could support only one type (e.g. SetResult(Option<T> result) - as an implicit cast from T to Option<T> is possible here).
  2. An acceptable solution for my case, would be to allow nullable value if the SetResult accepts it (i.e. SetResult(T? result)).

Risks

  • Allowing a method to return multiple types might be confusing
  • Might cause some issue with type inference
@dr1rrb dr1rrb added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Nov 30, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 30, 2023
@ghost
Copy link

ghost commented Nov 30, 2023

Tagging subscribers to this area: @dotnet/area-system-threading-tasks
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

When you implement a custom "Async Method Builder" to create your own instance of MyTask<T> the single type that the user can return in the function is T.

But, we could perfectly imagine cases where we can create a MyTask<T> with something else than T, for instance a T? (or a custom Option<T>).

My uses case is where MyTask<T> will implement IOtherInterface<GenericType<T>>. Restricting async method builder to only T means either:

  • Users will have to write MyTask<GenericType<T>>, which not possible since MyTask must have only one 1 type parameter, we cannot have type constraints on MyTask<T> where T : GenericType<T???>;
  • Having an implicit cast from GenericType<T> to T but this will result in a lost of information.

API Proposal

A method that uses an async method builder should accept that the user return any type for which there is a SetResult on the builder.

namespace MyLibrary;

[AsyncMethodBuilder(typeof(MyTaskMethodBuilder<>))]
public class MyTask<T>
{
	public void SetResult(T? value) { }
	public void SetResult(Option<T> value) { }
	public void SetException(Exception error) { }
}

public enum OptionType
{
	None,
	Some,
}

public record struct Option<T>(OptionType Type, T? Value)
{
	public static Option<T> Some(T value) => new(OptionType.Some, value);
	public static Option<T> None() => new(OptionType.None, default);
}

[EditorBrowsable(EditorBrowsableState.Never)]
public readonly struct MyTaskMethodBuilder<T>
{
	private readonly MyTask<T> _task = new();

	public MyTaskMethodBuilder() { }

	public MyTask<T> Task => _task;

	public static FeedMethodBuilder<T> Create()
		=> new();

	public void Start<TStateMachine>(ref TStateMachine stateMachine)
		where TStateMachine : IAsyncStateMachine
		=> stateMachine.MoveNext();

	public void SetStateMachine(IAsyncStateMachine stateMachine) { }

	public void SetException(Exception exception)
		=> _task.SetException(exception);

	public void SetResult(T? result)
		=> _task.SetResult(result);

	public void SetResult(Option<T> result)
		=> _task.SetResult(result);

	public void AwaitOnCompleted<TAwaiter, TStateMachine>(
		ref TAwaiter awaiter, ref TStateMachine stateMachine)
		where TAwaiter : INotifyCompletion
		where TStateMachine : IAsyncStateMachine
		=> awaiter.OnCompleted(stateMachine.MoveNext);

	public void AwaitUnsafeOnCompleted<TAwaiter, TStateMachine>(
		ref TAwaiter awaiter, ref TStateMachine stateMachine)
		where TAwaiter : ICriticalNotifyCompletion
		where TStateMachine : IAsyncStateMachine
		=> awaiter.OnCompleted(stateMachine.MoveNext);
}

API Usage

namespace MyUserApp;

public class MyUserClass
{
	public async MyTask<int> WhenReturnInt() => 42;
	public async MyTask<int> WhenReturnNullableInt() => default(int?);
	public async MyTask<int> WhenReturnOption() => Option<int>.None();
}

Alternative Designs

Two alternatives are possible:

  1. Instead of supporting "any type for which there is a SetResult on the builder", we could support only one type (e.g. SetResult(Option<T> result) - as an implicit cast from T to Option<T> is possible here).
  2. An acceptable solution for my case, would be to allow nullable value if the SetResult accepts it (i.e. SetResult(T? result)).

Risks

  • Allowing a method to return multiple types might be confusing
  • Might cause some issue with type inference
Author: dr1rrb
Assignees: -
Labels:

api-suggestion, area-System.Threading.Tasks, untriaged

Milestone: -

@teo-tsirpanis
Copy link
Contributor

This seems like a C# language feature. You should open an issue in https://github.com/dotnet/csharplang instead.

@teo-tsirpanis teo-tsirpanis closed this as not planned Won't fix, can't repro, duplicate, stale Nov 30, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Nov 30, 2023
@dr1rrb
Copy link
Author

dr1rrb commented Nov 30, 2023

Thanks @teo-tsirpanis, I was not sure but since I found the AsyncMethdoBuilderAttribute on this repo I opened it here. But you're right and actually this issue is a duplicate of dotnet/csharplang#3723

@github-actions github-actions bot locked and limited conversation to collaborators Dec 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Threading.Tasks
Projects
None yet
Development

No branches or pull requests

2 participants