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

Consider relaxing restrictions of async methods in blocks that do not contain await. (VS 17.11, .NET 9) #1331

Open
VSadov opened this issue Feb 20, 2018 · 12 comments
Assignees
Labels
Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification Proposal champion Proposal
Milestone

Comments

@VSadov
Copy link
Member

VSadov commented Feb 20, 2018

Async methods have several restrictions such as not allowing ref and ref-like/span locals.

The reason is the technical difficulties that may arise if such locals need to be hoisted. In a code that tries to mix await and span this could be inconvenient.

Besides there can be patterns which are obviously safe - In particular, in a block that does not contain await expressions, such locals would never be hoisted and therefore we could relax the restrictions.

@MkazemAkhgary
Copy link

Are you refering to methods that directly return a Task without awaiting it?

There is no need to mark that method as async. You can simply return task.

public async Task WaitForAll()
{
     return await Task.WhenAll(src);
}

Can be rewritten as

public Task WaitForAll()
{
     return Task.WhenAll(src);
}

Does this solve your problem?

@MkazemAkhgary
Copy link

if a local variable is not accessed after yielding some results or awaiting for results that can be also allowed. For example

public async Task WaitForAll()
{
     ref x = ...
     // work with x
     var result=  await something;
     // if x is not accessed here its ok!
}

@jnm2
Copy link
Contributor

jnm2 commented Feb 21, 2018

@MkazemAkhgary

Are you refering to methods that directly return a Task without awaiting it?

Does this solve your problem?

No, I don't think Vladimir is lost.

@VSadov
Copy link
Member Author

VSadov commented Feb 21, 2018

An example:

        async Task<int> Test()
        {
            await Task.Yield();

            int x = 42;

            // the following block does not contain await expressions
            // ref and ref-like locals are safe to use, but currently disallowed
            {
                Span<int> sp = stackalloc int[1];
                ref int r0 = ref sp[0];

                return r0 + x;
            }
        }

A workaround that I have seen in use.
But I think it just proves the point - some patterns are clearly safe and the language stands in the way.

        async Task<int> Test1()
        {
            await Task.Yield();

            int x = 42;

            // one possible workaround is a local function, but that is not nice
            return block();
            int block()
            {
                Span<int> sp = stackalloc int[1];
                ref int r0 = ref sp[0];

                return r0 + x;
            }
        }

@VSadov
Copy link
Member Author

VSadov commented Feb 21, 2018

Keying off "before/after await" is tricky in the presence of loops or other nontrivial control flow.
Lexically "after" does not imply it will not need to be hoisted.

"In a block with no awaits" seems much easier to reason about.

@MkazemAkhgary
Copy link

I see, sounds reasonable. its kind of like how await is forbindden in unsafe context since c# cant get pointer of fields. But unsafe block without awaits is ok.

@svick
Copy link
Contributor

svick commented Feb 21, 2018

"In a block with no awaits" seems much easier to reason about.

It also means that you have to change your reasonable code just to make the compiler happy. I think that "fighting with the compiler" should be avoided if possible.

Something like "there can be no await between declaration and usage of a stack-only variable" sounds much easier on the user to me. But yes, it has to take loops and similar constructs into account when evaluating what "between" means.

@vancem
Copy link

vancem commented Apr 12, 2018

If it easy to do it would also be nice if you can allow expressions that return Span to be legal in async methods (it may already be true). basically if

  • Span ReturnsSpan();
  • void ConsumesSpan(Span);
    then I should be able to do
        async Task<int> Test1()
        {
            await Task.Yield();
            ConsumesSpan(ReturnsSpan());
            await Task.Yield();
        }

This avoids needing to make a explicit block {} for the trivial case where there are no local variables.

@VSadov
Copy link
Member Author

VSadov commented Apr 12, 2018

Expressions/values are allowed. Locals are not allowed. The thing above should work fine.

@vancem
Copy link

vancem commented Apr 12, 2018

Great!

@VSadov
Copy link
Member Author

VSadov commented Apr 12, 2018

There is an obscure case where expressions are not allowed - when we have to stack spill them, but that is very rare.

        async Task<int> Test1()
        {
            await Task.Yield();
            ConsumesSpanAndStuff(ReturnsSpan(), await SomethingAsync());
            await Task.Yield();
        }

In the case above you would need to take the await out of the arg list. We cannot do that because of possible sideeffects that SomethingAsync can produce or observe.

Otherwise "flowing" spans is ok in async, storing into locals - no.

@tmat
Copy link
Member

tmat commented Jan 6, 2022

Related: dotnet/roslyn#40213

@jcouv jcouv added Proposal Proposal champion Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification labels Sep 4, 2024
@jcouv jcouv changed the title Consider relaxing restrictions of async methods in blocks that do not contain await. Consider relaxing restrictions of async methods in blocks that do not contain await. (VS 17.11, .NET 9) Sep 17, 2024
@jcouv jcouv added this to the 13.0 milestone Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Implemented Needs ECMA Spec This feature has been implemented in C#, but still needs to be merged into the ECMA specification Proposal champion Proposal
Projects
None yet
Development

No branches or pull requests

8 participants