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

Collection expressions: prefer ReadOnlySpan<T> over Span<T> in overload resolution #70328

Merged
merged 7 commits into from
Oct 13, 2023

Conversation

cston
Copy link
Member

@cston cston commented Oct 10, 2023

See updated better conversion from expression: dotnet/csharplang#7591

Fixes #70318

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 10, 2023
@cston cston marked this pull request as ready for review October 11, 2023 18:33
@cston cston requested a review from a team as a code owner October 11, 2023 18:33
@jcouv jcouv changed the base branch from release/dev17.8 to main October 11, 2023 21:18
@cston cston marked this pull request as ready for review October 12, 2023 15:12
Comment on lines 1765 to 1766
static void F1(int[] value) { }
static void F1(object[] value) { }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember the exact notes but isn't this supposed to be unambiguous just like (int,int,int) and (object,object,object)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not aware of a decision to prefer one array type over another for a collection expression conversion in overload resolution, other than the existing rule to prefer type T1 over T2 if there is an implicit conversion from T1 to T2.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My mistake, I thought most-specific rules can interact with the common-type from elements.

Copy link
Contributor

@alrz alrz Oct 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Relevant notes: https://github.com/dotnet/csharplang/blob/main/meetings/2023/LDM-2023-09-20.md
Though that's only a special-case around ref structs to resolve this, and only in one direction.

Diagnostic(ErrorCode.ERR_AmbigCall, "F1").WithArguments("Program.F1(System.ReadOnlySpan<int>)", "Program.F1(System.ReadOnlySpan<object>)").WithLocation(10, 9),
// (11,9): error CS0121: The call is ambiguous between the following methods or properties: 'Program.F2(Span<string>)' and 'Program.F2(Span<object>)'
// F2(["a", "b"]);
Diagnostic(ErrorCode.ERR_AmbigCall, "F2").WithArguments("Program.F2(System.Span<string>)", "Program.F2(System.Span<object>)").WithLocation(11, 9));
Copy link
Contributor

@RikkiGibson RikkiGibson Oct 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't feel right to me, but I can see that it adheres to the specification. I think we should resolve at the same time as #70238, hopefully in 17.9.

@cston cston merged commit e98c10a into dotnet:main Oct 13, 2023
25 checks passed
@cston cston deleted the 70318 branch October 13, 2023 23:08
@ghost ghost added this to the Next milestone Oct 13, 2023
cston added a commit to cston/roslyn that referenced this pull request Oct 16, 2023
arkalyanms added a commit that referenced this pull request Oct 17, 2023
Collection expressions: prefer ReadOnlySpan<T> over Span<T> in overload resolution (#70328)
@jjonescz jjonescz modified the milestones: Next, 17.9 P1 Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Overload resolution prefers Span<T> over ReadOnlySpan<T> for collection expression
5 participants