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

Surface a warning at the declaration site for members that will be hidden by the language [InlineArray] support #68868

Closed
tannergooding opened this issue Jul 3, 2023 · 3 comments · Fixed by #69064

Comments

@tannergooding
Copy link
Member

tannergooding commented Jul 3, 2023

Summary

The new [InlineArray] support adds some implicit functionality for the attributed types. This implicit functionality is not defined as an actual API and can mask any explicit API surface the user has defined themselves.

For example, the following program:

using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;

S1 s1 = default;
_ = s1[0];
_ = s1[(nuint)0];

[InlineArray(10)]
public struct S1
{
    public int _x;

    [UnscopedRef]
    public ref int this[int index]
    {
        get
        {
            Console.WriteLine("int indexer");
            return ref Unsafe.Add(ref Unsafe.As<S1, int>(ref this), index);
        }
    }

    [UnscopedRef]
    public ref int this[nuint index]
    {
        get
        {
            Console.WriteLine("nuint indexer");
            return ref Unsafe.Add(ref Unsafe.As<S1, int>(ref this), index);
        }
    }
}

Outputs:

nuint indexer

The expected output from the user might have been:

int indexer
nuint indexer

Expected Behavior

The compiler should surface a warning indicating that the member is now hidden and inaccessible:

// ...
    [UnscopedRef]
    public ref int this[int index] // CS----: Inline array member is inaccessible in C# 12 or later
    {
// ...

A warning, rather than an error, is emitted to ensure that users are not forced to break binary compatibility if they are migrating an existing type to use [InlineArray].

Alternative Designs

The language could also actually define a public indexer for the various types it supports and then surface CS0111: Type 'S1' already defines a member called 'this' with the same parameter types. However, this would increase the size of the type and could lead to "less ideal" IL emitted since it would be one more thing the JIT has to process and potentially inline.

Relates to test plan #67826

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Jul 3, 2023
@jcouv jcouv added this to the C# 12.0 milestone Jul 3, 2023
@alrz
Copy link
Member

alrz commented Jul 4, 2023

The compiler should surface a warning indicating that the member is now hidden and inaccessible:

Isn't it actually useful to be able to replace compiler generated indexer? I'd imagine TemporaryArray could possibly use InlineArray for its fixed storage.

@tannergooding
Copy link
Member Author

Yes, it’s potentially useful to be able to replace. The compiler could alternatively prefer the user defined overload if any.

That is, however, a bit inverted from how most language features work I think

@AlekseyTs
Copy link
Contributor

AlekseyTs commented Jul 5, 2023

Isn't it actually useful to be able to replace compiler generated indexer?

While that could be useful for a user, a user declared indexer will not provide the same semantics that element access in the language provides. A user is still able to provide a method, for example.

@jcouv jcouv removed the untriaged Issues and PRs which have not yet been triaged by a lead label Jul 5, 2023
AlekseyTs added a commit to AlekseyTs/roslyn that referenced this issue Jul 17, 2023
… the language for an element access expression

Closes dotnet#68868.
AlekseyTs added a commit that referenced this issue Jul 20, 2023
… the language for an element access expression (#69064)

Closes #68868.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment