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

Diagnostic is unclear when passing ref readonly to ref readonly parameter using 'ref' #73459

Open
RikkiGibson opened this issue May 13, 2024 · 4 comments
Labels
Area-Compilers Bug Concept-Diagnostic Clarity The issues deals with the ease of understanding of errors and warnings.
Milestone

Comments

@RikkiGibson
Copy link
Contributor

Version Used: fca6e1f

Steps to Reproduce:

SharpLab

class C
{
    void M0(ref readonly int i) { }
    void M1(ref readonly int i)
    {
        M0(ref i); // error CS8329: Cannot use variable 'i' as a ref or out value because it is a readonly variable
        M0(in i); // ok
    }
}

It appears that this error is by design. @jjonescz are there notes handy for why the design is this way?

Given the above I think the diagnostic message should simply be adjusted here to indicate that the user should use in at the call side instead.

I personally found it surprising that ref was not permitted here, because other kinds of arguments can be passed using ref for this parameter without issue. It was not obvious to me that using ref was actually requiring the argument to be a writable reference, even though the parameter will not assign to the referent.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels May 13, 2024
@jjonescz jjonescz added the Concept-Diagnostic Clarity The issues deals with the ease of understanding of errors and warnings. label May 14, 2024
@jjonescz
Copy link
Contributor

jjonescz commented May 14, 2024

Note that you would get the same error with in parameter (so this issue is orthogonal to the ref readonly feature):

class C
{
    void M0(in int i) { }
    void M1(in int i)
    {
        M0(ref i); // error CS8329: Cannot use variable 'i' as a ref or out value because it is a readonly variable
    }
}

ref modifier simply cannot be used with readonly references. But I agree the diagnostic message could be clearer.

@RikkiGibson
Copy link
Contributor Author

I am still interested to know why the design requires writability for a ref readonly argument passed using ref keyword.

The reason I found this design so confusing is that it is inconsistent with all the existing places that I can create ref readonlys, including assignment to ref readonly locals and return values.

int i = 1;
ref readonly int ri = ref i; // ok
ri = ref i; // reassignment also ok
M(ref i); // ok?
M(ref ri); // error!

ref readonly int M(ref readonly int i) => ref i; // ok

So, when you get started using this feature, and ref keyword works for some arguments, it leads user to think: ok, I'm supposed to use ref for these arguments, like all the other places that use ref readonlys in the language. Until they get around to actually passing a readonly reference by-ref, in which case an error occurs.

If we really have a strong reason for requiring writability here and not in the other places, then OK. Let's resolve by simply issuing a diagnostic which is consistent with all the other places that a wrong argument ref kind is being used: error CS1620: Argument 1 must be passed with the 'in' keyword.

@jjonescz
Copy link
Contributor

jjonescz commented May 14, 2024

I am still interested to know why the design requires writability for a ref readonly argument passed using ref keyword.

The reasons are explained in the proposal under alternatives:

image

@RikkiGibson
Copy link
Contributor Author

Thanks Jan!

@jaredpar jaredpar added this to the Backlog milestone May 14, 2024
@jaredpar jaredpar added Bug and removed untriaged Issues and PRs which have not yet been triaged by a lead labels May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Bug Concept-Diagnostic Clarity The issues deals with the ease of understanding of errors and warnings.
Projects
None yet
Development

No branches or pull requests

3 participants