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

[Proposal]: Params collections and older language versions #8061

Closed
jaredpar opened this issue Apr 17, 2024 · 1 comment
Closed

[Proposal]: Params collections and older language versions #8061

jaredpar opened this issue Apr 17, 2024 · 1 comment

Comments

@jaredpar
Copy link
Member

jaredpar commented Apr 17, 2024

Params collections and older language versions

Consider the following code which is analogous to how the .NET libraries team is going to use params collections in net9

public static class Console
{
  public static void WriteLine(string format, params object[] args);
  public static void WriteLine(string format, params ReadOnlySpan<object> args);
}

The consumption of this API is virtually transparent to consumers. They call this API the same way they've always called it.

Console.WriteLine("hello {0}", someExpression);

When the customer transitions from targeting net8 to net9 the new params ReadOnlySpan<object> API will be available and the compiler will silently choose it. This is the desired behavior (push customers to pit of success). Much like our interpolated string changes, customers need to know nothing, they just upgrade and get the benefits.

There is an issue though when a customer has the following setup:

<LangVersion>12</LangVersion>
<TargetFramework>net9.0</TargetFramework>

When a customer with this setup migrates to net9 they will hit the following diagnostic:

The feature 'params collections' is currently in Preview and unsupported. To use Preview features, use the 'preview' language version.

This happens because the changes we made to overload resolution to prefer params ReadOnlySpan<T> are unconditional but using the feature is guarded by LangVersion. This is a problem because it's completely valid to have LangVersion set lower than the default for the TargetFramework.

Potential Solutions

This problem is all about how new features are handled with older language versions. As part of writing up this issue I did an experiment to see how this works for a variety of existing features. The results show that the compiler is wildly inconsistent in how it approaches this problem. The approaches vary between seemingly not caring about consumption, caring about consumption in some cases but not others or behavior that I think is best described as bugs.

Given there is no obvious common case to use here wanted to present a few options.

Ignore LangVersion on consumption

In this approach the compiler would simply ignore language version when consuming params ReadOnlySpan<T>. There are several examples of this to draw from:

  1. Enhanced #line directives: these were largely introduced to solve Razor ENC issues. After deploying them the specifics of Razor meant that the feature was basically unusable if it was tied to language version. LDM discussed this conflict and decided in the interest of pragmatism that we would ignore lang version for definition of #line (there is no consumption scenario).
  2. dynamic and generics: these have effectively zero consumption guards in language version. dynamic in particular is analogous to params ReadOnlySpan<T> because it introduces extra code to facilitate the call site. Essentially, it pulls in code patterns from a higher C# language version and puts them directly into a C# language version that doesn't support it. Given the age of this code it's unclear if this is a bug, explicit design or just not considered. Features from the pre-4.0 days though seem to follow this pattern.
  3. covariant returns: these can be consumed from lower language versions.

Taking this approach is a bit dicey because of the relationship between collection expressions and params collections. The call site of a params collections member will essentially be implemented by effectively evaluating the arguments as a collection expression. This means we are potentially injecting C# 11+ code patterns into C# 10 (or early) code bases. That somewhat cuts against the idea of having language version guards.

Guard applicable function member changes on LangVersion

In this approach the compiler would change overload resolution such that the new rules only come into place in C# 13 and above. There are a few examples of this to draw from:

  1. Better overload candidates: In C# 7.3 the compiler made a few changes to improve overload resolution such as not considering instance members when binding a method invocation on a type. These changes only take effect when using 7.3 or above. Older language versions simply don't see these changes and get the same ambiguity errors they used to.
  2. readonly members: the compiler will only avoid unnecessary copies for readonly members when using 8 or higher.

Believe that we could take this approach in params collection by only applying the applicable function member changes on language version 13 or higher. That means calls like Console.WriteLine(format, arg) would never hit the params ReadOnlySpan<object> member because we don't consider the expanded form.

This would allow for C# 12 or lower to call params Span<T> members if they explicitly passed a Span<T> type. For example:

Span<object> span = (new object[] { "hello" }).AsSpan();
Console.WriteLine(format, span);

This code would work using any language version that supported Span<object>. This is in keeping with spirit of language version though as it's not taking advantage of any C# 13 features.

Guard all overload changes on LangVersion

This is a variant of the above approach where we also guard better function member changes on language version. That seems reasonable but I do not see any particular advantage to doing this. If there is such a case we should consider this though.

Conclusion

I personally lean towards the "Guard applicable function member" approach. Even though there is plenty of precedent for ignoring language version on consumption it's not in the spirit of the rule. Given there is a simple approach that avoids this and is analogous to other approaches we've taken in the past I think it's the best path forward.

@miyu
Copy link

miyu commented Jul 15, 2024

For the "Guard applicable function member changes on LangVersion" solution, it would be nice to emit a compiler warning for the edge-case. This would be a step towards option 3: Keep the error.

The error was a feature for me. In perf-optimized code (which is frequently where we see Span used), it's undesirable to accidentally default to a more expensive allocating code-path. Because of this current 'bug', I learned my code could be upgraded to a better path. Defaulting to the 'worse' path would have been a hidden pessimization for me even though admittedly it'd be following the behavior of prior .net releases.

@333fred 333fred closed this as completed Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants