Discussion thread for arbitrary async returns #10902
Comments
|
AsyncEnumerator? Not quite sure how the syntax would go (e.g. where await is) Kind smilar or same as the Observable. foreach (var row in await dataStream)
{
// ...
} |
|
@benaadams, that's for a different topic: #261 |
|
@ljw1004 Just FYI, I've added your branch to TryRoslyn, e.g. http://is.gd/Yjvb2P. |
|
@ashmind That is totally awesome! I've updated links. |
|
I'm having doubts about the class MyTasklike<T>
{
[EditorBrowsable(EditorBrowsable.Never)]
public static MyBuilder<T> GetAsyncMethodBuilder() => ...
}The problem is that this mechanism can never work for tasklike interfaces like |
|
@ljw1004 could it pick up interfaces via extension? e.g. static class MyTasklikeExtensions<T>
{
[EditorBrowsable(EditorBrowsable.Never)]
public static MyBuilder<T> GetAsyncMethodBuilder(this IAsyncAction asyncAction) => ...
} |
|
We'd discussed this when we first implemented async and were pretty down on the idea of an extension method where you deliberately have to pass a null I'm also a bit worried that the question of whether something is tasklike is no longer an inherent property of the type but instead depends on the context you're looking it up from. I think this context information has never previously had to be threaded into this part of overload resolution. More notes on the subject: https://github.com/ljw1004/roslyn/blob/features/async-return/docs/specs/feature%20-%20arbitrary%20async%20returns%20-%20discussion.md#discuss-connection-between-tasklike-and-builder |
|
@ljw1004 quite tricky! I suppose as long it doesn't preclude a future use on interfaces. It could be deferred and either be resolved as option 1, or in conjunction with one of the other interface proposals as a follow up, after seeing where they go? e.g. Proposal: Virtual extension methods #258, Proposal: Static Interfaces #2204 |
|
I'm loving this. Having access to the task-like type construction lets you do all sorts of things that aren't really in the spirit of For example, I threw together an implementation of https://github.com/ckimes89/arbitrary-async-return-nullable/ edit: I should note that the current implementation isn't compatible with asynchronous Tasklikes. |
|
An addition to the rational of why things like ValueTask are important. Due to So if one method at the bottom of the chain is actually async and the call chain is 30 methods deep with each on |
|
Love to see this as well, it always felt a little off to me that async, the language feature, had a dependency on a specific type instead of a pattern as is common with most of the other language features |
|
@aL3891 Well there's precedent for the language having a dependency on a type, given how |
|
True, but at least that an interface :) |
|
Continuing on the theme of abusing this feature, I managed to mimic yield/IEnumerable with Tasklikes: static void Main()
{
var list = DoListThings();
foreach(var item in list)
Console.WriteLine(item);
// prints 1, 2, 3
}
static async ListTaskLike<int> DoListThings()
{
await ListTaskLike.Yield(1);
await ListTaskLike.Yield(2);
await ListTaskLike.Yield(3);
return await ListTaskLike.Break<int>();
}Also, it's got a neat little monadic property to it as well in that you can await child ListTaskLikes and all of the values will be included (like LINQ's SelectMany). Doing this in C# with yield requires a foreach loop: static async ListTaskLike<int> DoListThings()
{
await ListTaskLike.Yield(1);
await DoNestedListThings();
return await ListTaskLike.Break<int>();
}
static async ListTaskLike<int> DoNestedListThings()
{
await ListTaskLike.Yield(2);
await ListTaskLike.Yield(3);
return await ListTaskLike.Break<int>();
}
foreach(var item in DoListThings())
Console.WriteLine(item);
// prints 1, 2, 3Code is here: https://github.com/ckimes89/arbitrary-async-return-nullable/blob/list-tasklike/NullableTaskLike/ListTaskLike.cs |
|
I want to step back and take stock of the key open issues remaining in the design rationale: Q1. How to identify a tasklike, and find its builder? [link]
Q2. Can we reduce the number of heap allocations? [link]
Q3. Should await even be handled by the builder? [link]
Q4. Can an async method body interact with its builder? [link]
Q5. Debugging support missing? [link]
Q6. Dynamic? [link]
Q7. IObservable? [link]
Q8. Back-compat breaks? [link]
|
|
Q2. Always nice 😸 Arbitrary-async-returns so far is a very powerful feature by itself and stands on its own merits. However, if there is likely to be conflict between the two designs then might be cause for pause. Would really like the arbitrary-async-returns though so it would make me 😢 |
|
Q1. How to identify a tasklike, and find its builder? I'm a big fan of the static method since it seems way cleaner, but as you mentioned elsewhere it doesn't work with interfaces. It could be combined with another way of specifying the builder, but it seems really goofy to have two different ways to do it. If a builder can be specified by an extension method, are there any requirements on where that extension method is located (e.g. same namespace, assembly)? If not, how are conflicts resolved? Normal extension methods can be invoked statically but that's obviously not going to work here. Here, I lean towards enforcing that the extension method is defined in the same assembly and namespace. It doesn't allow you to extend third-party types, but I'd argue that it's up to the designer of the type whether or not it should be used as an async return. Also, if you want to extend a third-party type then you can still just extend the actual type and create your own builder.
I don't like attributing the async method at all. It's incredibly cumbersome. |
|
Q3. Should await even be handled by the builder? I'm a bit torn on this one. On the one hand, implementing AwaitOnCompleted correctly will probably have plenty of landmines for an implementer to trip on so they shouldn't be exposed to what amounts to an implementation detail. On the other hand, you would lose the ability to do some of the weirder stuff I did like the ListTaskLike (though it could be argued that that's a benefit). Is there some middle ground, like inheriting from a TasklikeBuilder class that has some methods implemented for you already? |
|
Q2. Maybe possible for some new tasklikes that haven't yet been written? I think you would need to inline some of the stuff in AsyncMethodBuilder.cs from the clr to get the cold path down to one allocation (and keep a hot path at zero). A bunch of that code is internal though. Q3. By default we can shell out to an async method builder that already exists (this is what I did in the enumerable value task builder), but allowing us to inject at this point on the ASM is nice. Q4. I think intellisense/tooling around this is at least as important as debugging features. Q6. Would not changing the late-binder now introduce backwards compat issues if/when we do consider changing it? Q7. Option 2 sounds right from my perspective and should be related to IAsyncEnumerable features I think. On the other hand, why not both? In the IAsyncEnumerable code it feels a little strange to call public IObservable<string> Option1()
{
var m = new SM1();
m.State = -1;
m.Builder = ObservableBuilder<string>.Create();
m.Builder.Start(ref machine);
return m.Builder.Task;
}by virtue of not having any public IObservable<string> Option2()
{
var m = new SM2();
m.State = -1;
m.Builder = ObservableBuilder<string>.Create();
m.Builder.Prepare(ref machine);
return m.Builder.TaskFactory;
}when |
|
I added a new "key question": Q8. Back-compat breaks? [link]
// Example 1: in C#6 this code compiles fine and picks the first overload,
// but in C#7 it picks the second for being better
void f(Func<Task<double>> lambda)
void f(Func<ValueTask<int>> lambda)
f(async () => 3);
// Example 2: in C#6 this code compiles fine and picks the first overload,
// but in C#7 it gives an ambiguity error
void g(Func<Task<int>> lambda)
void g(Func<ValueTask<int>> lambda)
g(async () => 3);
// Example 3: in C#6 this code compiles fine and picks the first overload with T=Task<int>,
// but in C#7 it picks the second with T=int for being more specific.
void h<T>(Func<T> lambda)
void h<T>(Func<ValueTask<T>> lambda)
h(async () => 3); |
|
@bbarry could you spell out what additional "intellisense/tooling" you're thinking about, beyond debugger support? |
|
Editor features mostly, for example on the method: async IAsyncActionWithProgress<string> TestAsync(HttpClient client)
{
await client.GetStringAsync(url, async.CancellationToken);
async.Progress?.Invoke("10%");
}It would be nice if the keyword |
|
Q8. should it break? In that the If it did break, users of However... if the Which might mean new type other than |
|
What I was trying to express is better summarised by: What would compat issues mean cross version e.g. C#6 app using C#7 lib and C#7 app using C#6 lib |
|
I don't like an attribute-based approach because the following disadvantages are too large.
|
|
Two cents on the modifier approach -- if you go with explicit annotation, why not use Also I think static extension methods are not the only future extensibility story. I'm sure there are cases for extension attributes as well (even though it might require a framework change). |
|
@ashmind For whatever reason there's a strong reluctance in C# to let attributes influence language semantics. That's why C# uses "this" keyword to indicate an extension method, rather than [Extension] like VB does. |
|
@ljw1004 It would be great if C# team described this goal and its exceptions — e.g. tasklike vs caller attributes. This would be useful for future issues as well. For example if the choice is based on target audience or usage frequency, this issue seems OK — advanced feature, likely to be used for only a few classes. |
I think this alone is enough to discount the mixed proposal. If the modifier proposal went ahead, I think it would be best to accept both a keyword modifier and an attribute modifier form; both As to a decision of |
|
@ashmind Caller attributes do not affect the language's static semantics. |
C# LDM notes from 2016.08.24 -- topic "how to identify a tasklike"This was the topic for yesterday's LDM. We rejected my three proposals, and picked up the suggestion of @ashmind above (which indeed several people have independently proposed). Thanks @ashmind. I'll be updating the feature spec shortly. In the meantime, here are the raw notes from LDM. DECISION: USE AN ATTRIBUTEPreviously we recognized a tasklike by the presence of a static If an attribute with the fully-qualified name The The attribute can be defined anywhere. The compiler merely checks for it by name. One interesting thing about the way C# works, you can have internal attribute classes (as shown in the above example). It doesn't have to be internal but we recommend this is how to write tasklike types in a library DLL. When a user writes a project that references your DLL, the compiler will know that your type is tasklike, but the user's code won't be able to use that attribute. (that said, it's possible that a future version of mscorlib will include the attribute as public, and so if you also declare it in your library you'll get a compile-time error; at that time you'd have to split into two versions of your library.) Requirement 1: a tasklike's builder type must have the same generic arity as the tasklike type itself. Requirement 2: a builder type must have a public static parameterless method "static BuilderType Create()" which returns a value of the builder type. Requirement 3: the attribute must have a constructor which takes a It's implementation-defined whether the above two requirements are validated when we declare a tasklike type or validated when we declare an async method/lambda that returns that tasklike type. If the former, then we would also have to validate the requirements upon meta-import of a tasklike type, and deem it non-tasklike if the requirements aren't met. I prefer the latter. (Requirement 3 might instead be rolled into the test of whether a type is tasklike). DiscussionWe had discussed the idea of using attributes early on, and indeed my first prototype used attributes. In my evaluation at the time I had said "it's ugly" and "it requires us to ship the attribute". The second objection proved false because of the "internal" trick described above, which I hadn't known. The first objection -- well, it is ugly, but we also came to feel the alternatives were a little ugly too. Should we have used a modifier like What about the objection from people like @gafter that it's wrong for an attribute to influence the static semantics of the language (i.e. typing, type inference, type-directed overload resolution)? Well, @gafter was mollified by the thought that whenever he sees a tasklike type with the attribute, he'll close his eyes and imagine his "happy place" where it really is a modifier. Only the above 5 people will write it as an attribute anyway. The attribute has one neat benefit -- it can be used on WinRT types like There's also a downside to the attribute. It means that you can't take someone else's pre-existing type and make it tasklike via "extension statics" or similar, as noted by @ufcpp. That is a shame. We figure that the need for this is pretty small. It will be ameliorated if the type authors themselves do the work in a timely fashion (e.g. like We discussed one avenue for future expansion that was advocated by @stephentoub, and I think @bbarry that this is what you described too. Imagine if you wanted to return a pre-existing type, but for sake of your own method, you want to pick a different builder type. We might decide to re-use the attribute for this purpose but place it on the async method rather than the tasklike type. (This won't work for async lambdas of course since they have no place to hang the attribute; it's also not clear if we'd limit this feature solely to return types which are already considered tasklike). [AsyncBuilder(typeof(MyOwnTaskBuilder))]
async Task FredAsync() {...}
[AsyncBuilder(typeof(MyOwnBufferBlockBuilder))]
async BufferBlock<int> JonesAsync() {...}We discussed another avenue for future expansion that was described by @MadsTorgersen. The previous proposal let us have a builder type whose generic arity differed from that of the tasklike. If we ever decide that this flexibility is desirable in future, we're open to achieving the same thing with a // Previous proposal:
public class MyTasklike<T>
{
public static MyBuilder<string,int,T> CreateAsyncBuilder() => ...
}
// How that might be done with attributes
[AsyncBuilderFactory(typeof(BF))] public class MyTasklike<T> {...}
internal struct BF
{
public static MyBuilder<string,int,T> Create<T>(MyTasklike<T> dummy) => ...
// The compiler would construct default(tasklike), and invoke this function with it,
// and that's how it would get an instance of the builder.
} |
|
Yeah it could be nice to be able to write: though it isn't exactly what I was suggesting (I did think it might be nice but left that out of my comment as extra fluff that might be distracting). As you have said, it is likely there might be 5 people who write their own tasklikes. I suspect it is more likely that there will be several people who come up with interesting unforseen abuses of the system to do things like @ckimes89's nullable stuff above, so this level of customization seems unnecessary. I was suggesting that the compiler adds such an attribute when building the method to annotate that the method is in fact built by some async builder to reflection. There is a slight tooling issue today in that user writes: builds as: And the user (and tools) can see this attribute via reflection and present a richer experience when suggesting Further I was suggesting it would be nice for authors to be able to omit the (in this hypothetical, the attribute and keyword are interchangeable and if both are used, the attribute wins) Impact on existing libraries would be perhaps worse tooling interactions (though suggesting an await pattern on every use of |
|
@bbarry wrote:
I'm suspicious of that... I think you generally still want to await a background compute task or (after starting it) an unstarted task. Could you spell out (maybe with concrete examples) what kinds of things are intended to be awaited and which ones aren't? [this is a derail from the topic of this issue, but I figure we can continue with it for a couple of posts before it merits its own issue...] |
Did you mean to continue this sentence? |
|
suppose I was writing some parallel code: static int ParallelTaskImageProcessing(Bitmap source1,
Bitmap source2, Bitmap layer1, Bitmap layer2,
Graphics blender)
{
Task toGray = Task.Factory.StartNew(() =>
SetToGray(source1, layer1));
Task rotate = Task.Factory.StartNew(() =>
Rotate(source2, layer2));
Task.WaitAll(toGray, rotate);
Blend(layer1, layer2, blender);
return source1.Width;
}Someone could have instead done this: static int ParallelTaskImageProcessing(Bitmap source1,
Bitmap source2, Bitmap layer1, Bitmap layer2,
Graphics blender)
{
Task toGray = SetToGray(source1, layer1);
Task rotate = Rotate(source2, layer2);
Task.WaitAll(toGray, rotate);
Blend(layer1, layer2, blender);
return source1.Width;
}which could be written: static Task<int> ParallelTaskImageProcessing(Bitmap source1,
Bitmap source2, Bitmap layer1, Bitmap layer2,
Graphics blender)
{
return Task.WhenAll(
SetToGray(source1, layer1),
Rotate(source2, layer2)
)
.ContinueWith(t =>
{
Blend(layer1, layer2, blender);
return source1.Width;
});
}and could have a tool assisted conversion: static async Task<int> ParallelTaskImageProcessing(Bitmap source1,
Bitmap source2, Bitmap layer1, Bitmap layer2,
Graphics blender)
{
await Task.WhenAll(
SetToGray(source1, layer1),
Rotate(source2, layer2)
);
Blend(layer1, layer2, blender);
return source1.Width;
}I am fairly sure that would still work, but now the code is going through the machinery for Then again perhaps I'm thinking too hard and shouldn't worry about trying to await IO bound workflows that are likely to wait and not awaiting CPU bound ones. |
|
@bbarry I think the only solution is convention and developer education. Consider a "mixed" async method: async Task DoStuffAsync()
{
var buf = await GetBufAsync();
for (int i=0; i<buf.Length; i++) do_cpu_bound_stuff;
}Should you be awaiting this or not? Ultimately the answer has nothing to do with whether or not the method was written with the async modifier. The answer is that, as an "abstraction" principle, you should write methods which are either CPU-light (and they may very well return an awaitable) or CPU-heavy (and they shouldn't block on IO and the caller should be the one to decide which and how many threads to allocate to them). |
|
I've now implemented the When is something tasklike?Which of the following are tasklike? [AsyncBuilder("hello")] class Tasklike1 { }
[AsyncBuilder(typeof(void))] class Tasklike2 { }
[AsyncBuilder(typeof(B3))] class Tasklike3 { }
class B3<T> { } // generic arity differs from that of tasklike
[AsyncBuilder(typeof(B4))] class Tasklike4 { }
class B4<T> { private static string Create() => null; ... } // Create method returns wrong typeI adopted the following rule: A type is considered tasklike if it has the
Nesting and genericsWhich of the following should be allowed? class Outer<T>
{
class Builder { }
[AsyncBuilder(typeof(Builder))] class Tasklike1 { }
[AsyncBuilder(typeof(Outer<>.Builder))] class Tasklike2 { }
[AsyncBuilder(typeof(Outer<T>.Builder))] class Tasklike3 { }
[AsyncBuilder(typeof(Outer<int>.Builder))] class Tasklike4 { }
}
The following is allowed, because the builder isn't in a generic class: class Outer
{
class Builder { }
[AsyncBuilder(typeof(Outer.Builder))] class Tasklike { }
}AccessibilityOn the builder type, it looks for a public Can you can make the builder type itself have less accessibility, e.g. make it internal? That will allow your library to declare async methods that return your tasklike type, but no one else will be able to. I think this is a recipe for confusion and I'll disallow it. |
I think this is ok. You are leveraging the machinery of the compiler to build a method in a particular way (as an instance of this state machine pattern). It is possible that the builder has no purpose in the public api or even that it requires particular knowledge to write and could be harmful for a casual user to build an async method with. An example I am thinking is perhaps there is an unmanaged API out there which could otherwise be used in an async pattern, except that various parameters need to be configured before each await state. Given an where It would be possible to require the library to hand write the machinery here, but why should the compiler enforce such an arbitrary restriction? |
|
It seems strange to me to have a tasklike that only you can actually use as a tasklike. The reason for using an internal builder would be that it provides some useful functionality for you, but not useful enough that you would want to expose it to a caller of your library - which seems like a pretty strange situation. Also, given the attribute based approach, no caller of your library could introduce their own tasklike functionality for your ostensibly tasklike type. On the other hand, allowing internal builder types is a superset of the functionality offered by requiring all public builder types, so it's not like we're sacrificing capabilities to support internal builders. If a strange situation arises (like the example from @bbarry) then you're not forced by the language to make a potentially unsafe API. |
|
I switched to requiring the builder to have the same accessibility as the tasklike... it's a minor issue, and the arguments in both directions (keep them the same vs allow them to be different) aren't all that major, and the implementation turned out to be easy+clean to require them to be the same. The PR for the change is here: #13405 @jaredpar spotted a flaw with the attribute approach. We had proposed that a DLL which declares a tasklike type would also declare its own internal copy of the AsyncBuilderAttribute class. But consider what happens when you try to produce a reference assembly out of this DLL. You'd need to include the internal type! This seems crummy and against the spirit of either reference assemblies or the internal modifier or both. So instead: we won't rely on the "internal" trick. Instead, ValueTask.dll will declare AsyncBuilderAttribute as public. Library authors who wish to declare tasklikes can either declare a public AsyncBuilderAttribute themselves too, or take a dependency on ValueTask, whichever is easier. In future we still do anticipate adding AsyncBuilderAttribute to mscorlib (and to System.Runtime in the .NETCore nuget world). On such platforms, ValueTask would switch to having a type forwarder for its AsyncBuilderAttribute. |
Won't that cause issues if I want to reference both |
|
@svick it will cause an issue, but only if you wish to refer to the attribute (i.e. if you're authoring a tasklike). If you merely write an async method that returns a tasklike, then your code won't refer to the attribute, and you won't suffer the ambiguity warning. There will be far fewer people authoring tasklikes than writing I think "easier" cuts both ways in this case. I'd hope that tasklike authors will take care to optimize for ease of consumption of their tasklikes.
|
I want to delve into this question a little further. The way I implemented is that the compiler calls a In this design, a type type can pass the Why should it be so? Why not come up with a different scheme, to which the question alludes, where I didn’t go for this option because it would be a more costly check without benefit. And unless we do the full costly check, it would feel like a halfway-house. Halfway never feels right! In C# Language Design Meeting, @gafter pushed for us not to use a
We settled on using an attribute to control both bullets, There are two opposing language design philosophies in play here: (1) the two bullets are conceptually different; (2) it's confusing if the two bullets are out of sync. In respect of (2), I required builders have the same accessibility as the tasklike. This rules out the ability to have a type which consumers of your library think of in some technical ways as something that can be returned from an async method/lambda without themselves being able to return it from an async method/lambda. In respect of (1), and in a design that only really is noticeable in error situations, I said that the first bullet is controlled solely by the presence of One thing I like about this approach is that it lets us relax the rules for what is a valid builder type in future (e.g. allow it to be more generic, or covariant, or come from an inheritance hierarchy) without it being a breaking change. The typical way the language gets broken is because relaxed rules cause overload resolution changes. But, with this approach, that won't happen. |
|
Now that the final PR has been merged, I think it's time to close this thread. Thanks everyone for your help -- it's been a fun ride! |
|
Thanks for this really great feature. I'm about to release a new version of my package, and I wanted to be reasonably sure that this language feature isn't going to be rolled back at the last minute (as happened with some C# 6.0 features in VS2015). I understand we're still in RC and no guarantees can be made, but is it reasonable to depend on generalized async return types? |
This is a discussion thread for arbitrary async returns, as specified in https://github.com/ljw1004/roslyn/blob/features/async-return/docs/specs/feature%20-%20arbitrary%20async%20returns.md
You can try it out online! tryroslyn.azurewebsites.net
Please check the spec and design rationale before adding your comment - it might have been addressed in the latest draft.
(The previous discussion thread was here: #7169. I closed that thread after we more or less settled on a proposal.)
The text was updated successfully, but these errors were encountered: