Roslyn: Use `in` for arguments

[VSadov]

From LDM discussion today:
1) parameters should be declared with in instead of ref readonly

2) plain method call with consider both byval and in candidates, and continue producing an error if ambiguity
3) in at call site guarantees the in candidate is picked and no copy is made
4) async spilling should never make a copy if in (an error should be produced in this case)

This bug is about parts 2, 3, 4.

Other parts are tracked in https://github.com/dotnet/roslyn/issues/22381

[alrz]

1 is the exact opposite of a recent PR to remove in syntax! I guess it's because of 3, right? Is 3 mandatory from now on (making in and out consistent with each other)?

[VSadov]

Yes, indirectly, this is because of 3.

No, not mandatory. "in" at call site means "only resolve to 'in' and no copies please".
Not specifying "in" means "I do not care - byval or in - just make a call"

[VSadov]

Examples:

M(in int x) {...}

int x = 42;
M(in x); // ok and guaranteed no-copy

byte b = 3;
M(in b) // error

M(in 5); // error

// all ok, will copy when necessary
M(x);
M(b);
M(await SomeAsync());
M(42);

[tannergooding]

@VSadov, it still seems "bad" that you can only disambiguate on in.

That is, if you have MyMethod(MyStruct myStruct) and you add MyMethod(in MyStruct myStruct), users who recompile will be forced to upgrade to MyMethod(in MyStruct myStruct) and the previous overload MyMethod(MyStruct myStruct) will not be callable.

It essentially makes adding a ref readonly overload a forced breaking change, which will likely prevent several APIs in libraries like CoreFX from getting "upgraded".

[tannergooding]

I think there is a sort-of disconnect here on making this feature easy to use and allowing existing code to upgrade to it.

Not requiring the user to specify "in" when the case is not ambiguous makes it easy to use in entirely new code. However, due to requirements/desire to not introduce breaking changes to existing libraries, this also means that people consuming things like CoreFX will be having to attribute "in" a lot anyways.

Additionally, this means that when users do provide new APIs, users moving to the new version of the library will be forced to upgrade their code (they only have the option to disambiguate with "in", otherwise things will fail) which will likely prevent new adoption or entirely prevent the library from providing these overloads in the first place (outside of extension methods so users can manually opt-in, but this also hurts discoverability of those APIs and can lead to other issues when the API requires access to the private members of the underlying type).

[tannergooding]

CC @jcouv, @jaredpar for the above comment

[4creators]

IMO choice of in instead of ref readonly is very confusing. From perspective of many developers, particularly those with many other languages experience in means passing parameter by value whereas ref readonly self defines it's action. Furthermore, I would consider enabling ref readonly to participate in overload resolution (like in C++ where const keyword in parameter definition changes method signature).

Point 3 should be:

ref readonly at call site guarantees the ref readonly candidate is picked and no copy is made

[HaloFour]

Doesn't this (and the corresponding meeting notes) belong over on csharplang rather than here?

[VSadov]

It is possible to overload on byval/in.

In many cases it would be possible to direct overload resolution towards the desired target using "in", named arguments, static calls (when extension method), etc.
Not having to name _all_ named arguments after the first one in 7.2 makes it an especially easy option.

It would not be a recommended pattern though - just like overloading on byval/ref - not recommended and not very popular either.

There are many ways to make API not ambiguous - why would you intentionally follow the pattern that could be harder to use? The new API does not _need_ to have exactly same name, same parameter number/order/names.

To take advantage of new APIs the client must recompile anyways.

Overal - Design of new feature always need to compromise between different audiences.
Users who want APIs differ only on byval/in are hard to accommodate without hurting others and they have many other options.

[VSadov]

@HaloFour csharplang will definitely have notes. This is just a workitem to track necessary adjustments in the implementation.

[tannergooding]

@VSadov, I am explicitly talking about the case where:

• Today a user is consuming MyMethod(MyStruct val) from v1 of MyLibrary.
• I want to take advantage of ref readonly, but do not want to break back-compat in v2 of my library

I cannot add MyMethod(in MyStruct val) because this forces users to upgrade and explicitly specify in at all callsites. They have no mechanism of staying on the previous byval overload if they want to upgrade/recompile (possibly to take advantage of other new features) but are not ready to switch over to the new overloads.

So, in order to take advantage of the feature I have to:

• Use extension methods
• Use an API with a different name (ex: MyMethodRef(in MyStruct val)), etc.
• Something else

This hurts discoverability of my APIs, increases the number of items displayed in intellisense, makes quick-fixes from the IDE more difficult, etc.

[jcouv]

It's probably better to wait for Mads to publish notes to csharplang. The roslyn repo issues on this are just capturing action items that came out of last meeting.

@tannergooding The upgrade scenario you're concerned about for libraries was discussed. If this proves to be a problem, the language may introduce a tie-breaker that prefers byval parameters.
So instead of getting an error when there is ambiguity between byval and in versions of the same API, the byval version would continue to be picked, until the call site adds in to force the new version.

As you pointed out, there are ways to work around (different method name, parameter order or names, ...) in the meantime.

[tannergooding]

Thanks! I'm probably going to post on @MadsTorgersen notes when they go up, but I feel like having the tiebreaker from the start will be required, for CoreFX, if nothing else.