Proposal: "sync" block in async/iterator methods

[gafter]

@svick commented on Sat May 14 2016

A relatively common request is to do something in a state machine method (i.e. async method or iterator method) before the state machine is created, either for efficiency (common with async) or for correctness (common with iterators).

In C# 7, these cases can be solved fairly well with local functions (the second example uses arbitrary async returns #10902):

public static IEnumerable<T> Where<T>(this IEnumerable<T> source, Func<T, bool> predicate)
{
    if (source == null || predicate == null)
        throw new ArgumentNullException();

    IEnumerable<T> WhereImpl()
    {
        foreach (var item in source)
        {
            if (predicate(item))
                yield return item;
        }
    }

    return WhereImpl();
}

public static ValueTask<int> DelayWithResult(int delay)
{
    if (delay == 0)
        return ValueTask<int>.FromValue(delay);

    async ValueTask<int> DelayImpl()
    {
        await Task.Delay(delay);
        return delay;
    }

    return DelayImpl();
}

My proposal is to make this even nicer, by including a sync block at the start of the method that contains the code that should execute before the state machine is created. Also, if the sync block returns or throws, the result is returned or thrown directly, without wrapping in Task, tasklike, or IEnumerable<T>.

This means the examples above become:

public static IEnumerable<T> Where(this IEnumerable<T> source, Func<T, bool> predicate)
{
    sync
    {
        if (source == null || predicate == null)
            throw new ArgumentNullException();
    }

    foreach (var item in source)
    {
        if (predicate(item))
            yield return item;
    }
}

public static async ValueTask<int> DelayWithResult(int delay)
{
    sync
    {
        if (delay == 0)
            return ValueTask<int>.FromValue(delay);
    }

    await Task.Delay(delay);
    return delay;
}

This is similar to Defer async state machine creation #10449, except that it's explicit. Being explicit has both advantages (the change of semantics regarding what happens to thrown exceptions is allowed) and disadvantages (the optimization doesn't happen automatically).

The keyword sync is not ideal: it's not currently a keyword and could be confusing for iterator methods. Suggestions for alternative syntax are welcome.

---

@alrz commented on Sat May 14 2016

I think #119 and #8181 are related. Perhaps these should be merged to a single feature.

public int Insert(T item, int index) {
    pre { guard (index >= 0 && index <= Count); }
    post { guard (value >= 0 && value < Count); }
    return InsertCore(item, index);
}

public static IEnumerable<T> Where<T>(this IEnumerable<T> source, Func<T, bool> predicate) {
    pre {
        guard (source != null) else throw new ArgumentNullException(nameof(source));
        guard (predicate != null) else throw new ArgumentNullException(nameof(predicate));
    }
    foreach (var item in source) {
        if (predicate(item))
            yield return item;
    }
}

public static async ValueTask<int> DelayWithResult(int delay) {
    pre { guard (delay > 0) else return ValueTask<int>.FromValue(delay); }
    await Task.Delay(delay);
    return delay;
}

• Only a single pre and post is allowed in the method body and they must come first.
• No await or yield return is allowed in these blocks so that the state machine creation is deferred.
• The else clause is optional in a guard statement just like method contracts.
• In a post block value refers to the return value and can be disambiguated with @value if there was an identifier named value in the scope.

---

@temporaryfile commented on Sat May 14 2016

Your sync block would give me a structural way to allow async constructors. The language could enforce that all operations that must be done in a constructor, must be be done in a sync block if it's an async constructor.

---

@alrz commented on Thu May 26 2016

@svick As it turns out, your implementation with local functions DOES NOT defer the state machine creation in future branch. You must not capture any parameter or variable.

---

@AlgorithmsAreCool commented on Thu May 26 2016

This reminds me a little of powershell's begin, process, end blocks for pipeline functions. Thier aim is for code structure, not performance but the idea is similar.

Here is how they do it.

function Paint-Car
{
    param($car)

    begin {
        #setup the prereqs

        $paintColor = Mix-Paint("blue")
    }

    process {
        #The loop body 
        echo "painting $car!"

        ApplyPaint -Paint $paintColor -Car $car
    }

    end {
         #cleanup
        Cleanup-Paint -Paint $paintColor
    }
}

....

List-Cars -Name Ford* | Paint-Car

---

@svick commented on Sat May 28 2016

@alrz If I understand the generated code correctly, it does defer the state machine creation. But it also allocates a closure (a.k.a. display class) as soon as the method starts, so it does not avoid allocation as I was indeed assuming.

---

@svick commented on Sat May 28 2016

@playsomethingsaxman I don't understand how would sync block allow async constructors. If you have an async method, but it only contains a sync block, then according to my proposal, it should be the same as a non-async method.

---

@alrz commented on Sat May 28 2016

@svick You are right, it's a separate class, but there will be an allocation anyways.

---

@temporaryfile commented on Thu Jun 02 2016

@svick Watch:

public async Constructor()
{
    // All normal field initialization must be done in this block.
    // Easy.
    sync
    {
    }

    // Being able to inline how the class should initialize asynchronously is a MAJOR
    // readability boon.
    await SwitchToThreadPoolAsync();
    BlahBlahBlah();
}

By the way, why did they gut async set?

---

@svick commented on Thu Jun 02 2016

@playsomethingsaxman But how do you await the constructor? If it's more like async void, then I don't think that's a good idea.

Also, await SwitchToThreadPoolAsync() is generally considered problematic. If you instead used Task.Run(), you wouldn't even need sync in the first place.

---

@temporaryfile commented on Thu Jun 02 2016

The constructor isn't to be awaited, if you need that then there's the static factory method pattern. It simply continues initialization without having to call a separate dangling async method that also has the danger of being called again by other class members.

I've built epicly scaleable code with constructs like SwitchTo() because intent just flows so much better and faster. Everyone else is just shooting themselves in the foot with dogma. You can't crank serious code out with Task.Run(). And the .NET Framework thread pool is pure crap anyway, no offense. I've seen it crack and shatter embarrassingly with my butt on the line. It doesn't scale to anything, and people get around it by throwing more servers at the problem. I think that's the real reason for .NET Core, to secretly rewrite the crap, I don't think anyone says it out loud, lol

---

@jnm2 commented on Thu Aug 11 2016

I would love to see async constructors return Task<T>. There's a good discussion in #6788 on async constructors.

[Thaina]

I felt that this should not be keyword at all. The task system should be always implemented like proposed automatically

[yaakov-h]

That would make it a breaking change.

[alrz]

@yaakov-h How?

[yaakov-h]

using System;
using System.Threading.Tasks;

public static class Program
{
	public static void Main()
	{
		var task = Foo(null);
		Console.WriteLine("Foo called.");
		
		try
		{
			task.Wait();
		}
		catch (AggregateException ex)
		{
			Console.WriteLine(ex.InnerException.Message);
		}
	}
    
	static async Task Foo(object n) {
		_ = n ?? throw new ArgumentNullException(nameof(n));
		await Task.Delay(10);
	}
}

Currently the program prints:

Foo called.
Value cannot be null.
Parameter name: n

If you change the state machine generation to process the null check before kickoff, the program does not print anything, and crashes with an ArgumentNullException.

[jnm2]

@yaakov-h ftfy 😃

[yaakov-h]

@jnm2 ugh I had to fight with it because sharplab doesn’t support running code with Task, so I moved to dotnetfiddle which uses tabs.

[bondsbw]

An alternative solution I call "anonymous return function", which combines the return call site with the function definition and removes the need to specify the function signature:

public static IEnumerable<T> Where<T>(this IEnumerable<T> source, Func<T, bool> predicate)
{
    if (source == null || predicate == null)
        throw new ArgumentNullException();

    return 
    {
        foreach (var item in source)
        {
            if (predicate(item))
                yield return item;
        }
    };
}

public static ValueTask<int> DelayWithResult(int delay)
{
    if (delay == 0)
        return ValueTask<int>.FromValue(delay);

    return async
    {
        await Task.Delay(delay);
        return delay;
    };
}

This would not be as general as it does not help in cases with multiple return sites, but it does seem (to me) to be a bit more familiar. I'll add as a separate proposal if it gets several upvotes.

[gulshan]

I also think immediately executing anonymous local function is the way to go. Can the return type be something like ITask<T>?

[yaakov-h]

What would ITask<T> be/do?