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

Potential source breaking changes due to params collection #73857

Open
hez2010 opened this issue Jun 5, 2024 · 3 comments
Open

Potential source breaking changes due to params collection #73857

hez2010 opened this issue Jun 5, 2024 · 3 comments
Assignees
Labels
Area-Compilers Bug New Feature - Collection Expressions untriaged Issues and PRs which have not yet been triaged by a lead
Milestone

Comments

@hez2010
Copy link

hez2010 commented Jun 5, 2024

Version Used: 9.0.100-preview.6.24304.2

Steps to Reproduce:

string.Concat(["a", "b"]);

Diagnostic Id: CS0121

Expected Behavior: No compilation error

Actual Behavior:

CS0121 The call is ambiguous between the following methods or properties: 'string.Concat(scoped ReadOnlySpan<object?>)' and 'string.Concat(scoped ReadOnlySpan<string?>)'

Note that this is a source breaking change from C# 12 where the code used to compile.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Jun 5, 2024
@CyrusNajmabadi
Copy link
Member

@cston I believe this is the issue you were mentioning to me. We should see what the spec says and if we got it wrong in the spec or the impl. My expectation would 100% be that ROS<T> is better than ROS<U> when there is an implicit conversion from T to U. Just like void X(T t) is better than void X(U u) when dealing with a single element.

I'm thinking we might have spec'ed this wrong, but the above was definitely the intent. :)

@cston cston self-assigned this Jun 5, 2024
@cston
Copy link
Member

cston commented Jun 5, 2024

My expectation would 100% be that ROS<T> is better than ROS<U> when there is an implicit conversion from T to U.

Currently, better conversion rules for collection expressions treat this case as ambiguous.

Perhaps there should be an additional rule to prefer System.ReadOnlySpan<T> over System.ReadOnlySpan<U> for a collection expression if an implicit conversion exists from T to U.

@jaredpar
Copy link
Member

jaredpar commented Jun 5, 2024

Note that this is a net9.0 only problem, or at least primarily. If we do take changes here to betterness think that we need to consider typing it to langversion 13

@jaredpar jaredpar assigned 333fred and unassigned cston Aug 27, 2024
333fred added a commit that referenced this issue Sep 20, 2024
* Initial prototype work

* Correctly handle spreads

* Update test baselines

* Add dedicated interpolated string handler test

* Implement params changes and update tests

* Support falling back to C# 12 rules.

* Add more tests, including the table from https://github.com/dotnet/csharplang/blob/main/proposals/collection-expressions-better-conversion.md and an explicit test for #73857.

* Add tests to the CSharp 12 list as well

* Update language feature status.

* Cleanup

* Correct spread handling

* Only include expected output on .NET Core

* PR feedback.

* Add suggested numeric test.

* Document breaking changes

* More feedback.

* Add examples

* Add example of interpolated string breaking change

* PR feedback

* Update language feature status.

* More tests around dynamic and tuples

* Add collection expressions to test plan

* Spread testing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Bug New Feature - Collection Expressions untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

No branches or pull requests

5 participants