await parsed as a keyword in a non-async lambda within an async method

[Neme12]

Version used: Visual Studio 15.6.1.

using System;

class C
{
    async void M()
    {
        Action a = () =>
        {
            await x;
        };
    }
}

await x is parsed as an AwaitExpression even though the lambda itself is not async. If I replace the lambda with a non-async anonymous method or local function, it is parsed as a variable declaration instead.

Am I correct thinking that this is a bug in the parser?

[CyrusNajmabadi]

ParseAnonymousMethod has:

        private AnonymousMethodExpressionSyntax ParseAnonymousMethodExpression()
        {
            bool parentScopeIsInAsync = IsInAsync;
            IsInAsync = false;
            SyntaxToken asyncToken = null;
            if (this.CurrentToken.ContextualKind == SyntaxKind.AsyncKeyword)
            {
                asyncToken = this.EatContextualToken(SyntaxKind.AsyncKeyword);
                asyncToken = CheckFeatureAvailability(asyncToken, MessageID.IDS_FeatureAsync);
                IsInAsync = true;
            }

But ParseLambdaExpression is missing the IsInAsync = false; bit:

            bool parentScopeIsInAsync = IsInAsync;
            SyntaxToken asyncToken = null;
            if (this.CurrentToken.ContextualKind == SyntaxKind.AsyncKeyword &&
                PeekToken(1).Kind != SyntaxKind.EqualsGreaterThanToken)
            {
                asyncToken = this.EatContextualToken(SyntaxKind.AsyncKeyword);
                asyncToken = CheckFeatureAvailability(asyncToken, MessageID.IDS_FeatureAsync);
                IsInAsync = true;
            }

I don't think this was intentional. @gafter ?

[lorcanmooney]

The old native compiler (4.7.2556.0) treats it as a variable declaration.

[gafter]

This may be a bug, but a bug in the handling of local functions.

From the spec:

Inside of an async function, await cannot be used as an identifier

A non-async local function that is inside an async method is still "inside" of an async function, and therefore await should be a keyword.

In short, I don't think this is a bug.

[gafter]

I believe this is not a bug. I believe we discussed this with the LDM when Roslyn was being implemented and agreed that the implemented behavior is what we intended (even though the native compiler was inconsistent about it).

[CyrusNajmabadi]

FWIW, this behavior seems surprising, and not actually desirable. It seems sensible and clear to say that each lambda/method is affected only by the modifiers on it. So the parsing of statements is only affected by the immediately contained lambda/anon-method/method.

This is also how TS/JS works.

I am curious to hear what @MadsTorgersen thinks here. it seems odd that we would have this:

async void Outer() {
    Func<Task> f1 = () => { /*... await is a keyword here... but can't actually use it becaue func is not async. */ }
    Func<Task> f1 = async () => { /* ... await is ok here ... */ }
}

If 'await' is not ok in 'f1' due to it not being 'async', it seems like we should just treat it like a normal identifier.

--

Note: it may be that hte spec can be interpreted to support the current behavior. However, i don't think that interpretation if valuable or desirable. it is pretty strange, inconsistent with the industry, and does not lead to a better programming experience in my experience.

--

Could this be reopened? Or at least moved to csharplang to make a more explicit decision about what behavior is desirable/intentional? At the very least, i think the spec should call out this case and make it clear what the intended behavior is.

[gafter]

@CyrusNajmabadi If the goal was to minimize the scope of keywords, otherwise reserving them as identifiers, I would agree. But that is not the goal. I would think we'd want to maximize the scope of keywords to avoid confusion to the reader. The only reason we treat await as a contextual keyword is to avoid breaking existing code. But existing code (before C# 5) didn't have an async keyword on it. So within the scope of an async keyword, it is safe (not breaking of existing code) to treat await as a keyword.

[CyrusNajmabadi]

That's a fine reason for me. As mentioned, i think '2' is def something that the language can do. My primary concern is simply that the spec is pretty unclear here as to how to interpret these things. I think we should document clearly what the meaning is, and then we should ensure that all our function-like features follows that meaning :)

[svick]

@CyrusNajmabadi

As mentioned, i think '2' is def something that the language can do.

How come? Wouldn't that be a breaking change?

Right now, code like the following compiles:

class await {}

class C
{
    async void Outer()
    {
        void f1() { await x; }
    }
}

But option 2 would prohibit this code, which would make it a breaking change, right? Or are you saying that it's an acceptable breaking change?

As far as I can tell, option 1 is the only consistent solution that is not breaking.

[CyrusNajmabadi]

Or are you saying that it's an acceptable breaking change?

Yes. It would likely be an acceptable breaking change. The changes of someone typing the above is too marginal to care about (esp. with how recent local functions were added to the language).