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

C# 7.x §15.15.2 task builder "accessible" members #879

Closed
KalleOlaviNiemitalo opened this issue Aug 7, 2023 · 2 comments · Fixed by #1027
Closed

C# 7.x §15.15.2 task builder "accessible" members #879

KalleOlaviNiemitalo opened this issue Aug 7, 2023 · 2 comments · Fixed by #1027
Assignees
Milestone

Comments

@KalleOlaviNiemitalo
Copy link
Contributor

KalleOlaviNiemitalo commented Aug 7, 2023

Describe the bug

In the C# 7.x draft, §15.15.2 (Async functions / Task-type builder pattern) says that the task builder type must have "the following accessible members". It is not clear what "accessible" means there. Does it mean accessible by the async function, by the task type, or something else? Can the members be internal if all relevant code is in the same assembly?

(In any case, the task type must be accessible by the async function, in order to be the return type; and the task builder type must be accessible by the task type, in order to be named in the AsyncMethodBuilderAttribute.)

Example 1

Is the following allowed? The members are accessible by the async method and also by the task type. Roslyn however rejects this code. SharpLab

using System;
using System.Runtime.CompilerServices;

class C {
    async CustomTask M() {} // error CS0656
}

[AsyncMethodBuilder(typeof(CustomTaskBuilder))]
internal struct CustomTask
{
}
    
internal struct CustomTaskBuilder {
    internal static CustomTaskBuilder Create() => default; // internal causes the error
    public void Start<TStateMachine>(
        ref TStateMachine stateMachine)
        where TStateMachine : IAsyncStateMachine {}
    public void SetStateMachine(
        IAsyncStateMachine stateMachine) {}
    public void SetException(
        Exception exception) {}
    public void SetResult() {}
    public void AwaitOnCompleted<TAwaiter,TStateMachine>(
        ref TAwaiter awaiter,
        ref TStateMachine stateMachine)
        where TAwaiter : INotifyCompletion
        where TStateMachine : IAsyncStateMachine {}
    public void AwaitUnsafeOnCompleted<TAwaiter,TStateMachine>(
        ref TAwaiter awaiter,
        ref TStateMachine stateMachine)
        where TAwaiter : INotifyCompletion
        where TStateMachine : IAsyncStateMachine {}
    public CustomTask Task => default;
}

Example 2

Is the following allowed? The task builder type is not accessible by the async method, so its members are not accessible by that either. They are however accessible by the task type, and Roslyn allows this code. SharpLab

using System;
using System.Runtime.CompilerServices;

class C {
    async Outer.CustomTask M() {} // OK
}

internal class Outer
{
    [AsyncMethodBuilder(typeof(Middle.CustomTaskBuilder))]
    internal struct CustomTask
    {
    }
    
    private struct Middle
    {
        internal struct CustomTaskBuilder {
            public static CustomTaskBuilder Create() => default;
            public void Start<TStateMachine>(
                ref TStateMachine stateMachine)
                where TStateMachine : IAsyncStateMachine {}
            public void SetStateMachine(
                IAsyncStateMachine stateMachine) {}
            public void SetException(
                Exception exception) {}
            public void SetResult() {}
            public void AwaitOnCompleted<TAwaiter,TStateMachine>(
                ref TAwaiter awaiter,
                ref TStateMachine stateMachine)
                where TAwaiter : INotifyCompletion
                where TStateMachine : IAsyncStateMachine {}
            public void AwaitUnsafeOnCompleted<TAwaiter,TStateMachine>(
                ref TAwaiter awaiter,
                ref TStateMachine stateMachine)
                where TAwaiter : INotifyCompletion
                where TStateMachine : IAsyncStateMachine {}
            public CustomTask Task => default;
        }
    }
}

Example 3

Is the following allowed? As in Example 2, the task builder type is not accessible by the async method but is accessible by the task type. This time however, Roslyn rejects this code. SharpLab

using System;
using System.Runtime.CompilerServices;

class C {
    async Outer.CustomTask M() {} // error CS1983
}

internal class Outer
{
    [AsyncMethodBuilder(typeof(CustomTaskBuilder))]
    internal struct CustomTask
    {
    }
    
    private struct CustomTaskBuilder {
        public static CustomTaskBuilder Create() => default;
        public void Start<TStateMachine>(
            ref TStateMachine stateMachine)
            where TStateMachine : IAsyncStateMachine {}
        public void SetStateMachine(
            IAsyncStateMachine stateMachine) {}
        public void SetException(
            Exception exception) {}
        public void SetResult() {}
        public void AwaitOnCompleted<TAwaiter,TStateMachine>(
            ref TAwaiter awaiter,
            ref TStateMachine stateMachine)
            where TAwaiter : INotifyCompletion
            where TStateMachine : IAsyncStateMachine {}
        public void AwaitUnsafeOnCompleted<TAwaiter,TStateMachine>(
            ref TAwaiter awaiter,
            ref TStateMachine stateMachine)
            where TAwaiter : INotifyCompletion
            where TStateMachine : IAsyncStateMachine {}
        public CustomTask Task => default;
    }
}

Example 4

The same as Example 3, except now with internal struct CustomTaskBuilder. The members are accessible by the async method and also by the task type. Roslyn allows the code. SharpLab

Example 5

The same as Example 3, except now with internal protected struct CustomTaskBuilder. The members are accessible by the async method and also by the task type. Roslyn rejects the code, even though internal protected generally is more accessible than internal. SharpLab

Expected behavior

Declare that the task builder type shall be accessible by the async function (and thus also by the generated state machine).
Declare that the listed members of the task builder type shall be public. Do not say whether they are "accessible".

Example Roslyn (.NET SDK 7.0.109) Expected behavior
Example 1 Reject Reject, because member is not public
Example 2 Allow Reject, because task builder type is not accessible by async function
Example 3 Reject Reject, because task builder type is not accessible by async function
Example 4 Allow Allow
Example 5 Reject Allow

Additional context

None.


Associated WorkItem - 187391

@KalleOlaviNiemitalo
Copy link
Contributor Author

KalleOlaviNiemitalo commented Aug 7, 2023

Alternative fix

Declare that the task builder members shall be public.
Declare that the task type and task builder type shall have the same declared accessibility.
Declare that both types shall be declared in namespace scope or in the same containing type (which then cannot be a generic type).
Treat anything more complex as an extension.

Example Roslyn (.NET SDK 7.0.109) Alternative fix
Example 1 Reject Reject, because member is not public
Example 2 Allow Reject, because task builder type is nested but task type is not
Example 3 Reject Reject, because declared accessibility does not match
Example 4 Allow Allow
Example 5 Reject Reject, because declared accessibility does not match

@jskeet
Copy link
Contributor

jskeet commented Sep 20, 2023

Apologies, triaged this too late for C# 7. Will address address in C# 8.

@jskeet jskeet modified the milestones: C# 8.0, Pre-C# 8.0 Sep 20, 2023
@BillWagner BillWagner self-assigned this Nov 30, 2023
@BillWagner BillWagner moved this from 🔖 Ready to 🏗 In progress in dotnet/docs December 2023 sprint Dec 7, 2023
BillWagner added a commit to BillWagner/csharpstandard that referenced this issue Dec 15, 2023
Clarify the required accessibility of any task builder type and its required members.

Note that we will need to update this text once dotnet/roslyn#71283 is fixed and merged.
BillWagner added a commit to BillWagner/csharpstandard that referenced this issue Dec 15, 2023
Clarify the required accessibility of any task builder type and its required members.

Note that we will need to update this text once dotnet/roslyn#71283 is fixed and merged.
@BillWagner BillWagner moved this from 🏗 In progress to 👀 In review in dotnet/docs December 2023 sprint Dec 15, 2023
@BillWagner BillWagner moved this from 🔖 Ready to 👀 In review in dotnet/docs January 2024 sprint Jan 11, 2024
@github-project-automation github-project-automation bot moved this from 🔖 Ready to ✅ Done in dotnet/docs February 2024 sprint Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: 👀 In review
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants