Skip to content

Ignore generic type parameter names when matching #40359

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Ignore generic type parameter names when matching #40359

wants to merge 6 commits into from

Conversation

ericstj
Copy link
Member

@ericstj ericstj commented Apr 22, 2024

Generic type parameter names are not significant to compat.

Remove these from the matching key, but make the CannotChangeParameterName rule compare them.

@ericstj
Ignore generic type parameter names when matching
bccc12f 
Generic type parameter names are not significant to compat.
�Remove these from the matching key, but make the ParameterName rule
compare them.
@ericstj ericstj requested a review from ViktorHofer April 22, 2024 17:06
@ericstj ericstj requested review from andriipatsula and a team as code owners April 22, 2024 17:07
@ghost ghost added Area-ApiCompat untriaged Request triage from a team member labels Apr 22, 2024
@ericstj ericstj self-assigned this Feb 3, 2025
@ericstj ericstj requested a review from ViktorHofer February 24, 2025 20:31
@ericstj ericstj enabled auto-merge (squash) March 11, 2025 16:05
@build-analysis build-analysis bot mentioned this pull request Mar 24, 2025
Open
3 tasks
@ericstj
Copy link
Member Author

ericstj commented Mar 24, 2025

Looks to me like this might be hitting a real failure in source-build. My best guess is we mapped methods with different numbers of generic parameters - perhaps the name formatting isn't including the commas as expected? I'll add some tests to cover generic methods with different number of type parameters.

@ericstj
Add test for member mapping to ensure type parameters considered
afa9809 
Also introduce an exception instead of assert when members mapped with mismatched parameter counts.
@build-analysis build-analysis bot mentioned this pull request Mar 25, 2025
Open
@ericstj
Copy link
Member Author

ericstj commented Mar 26, 2025 •
edited
Loading

Updating with where I am at in investigation --

 Unexpected parameter count mismatch for 
M:System.Numerics.TotalOrderIeee754Comparer`1.GetHashCode(`0) System.Int32 System.Numerics.TotalOrderIeee754Comparer<>.GetHashCode() 1 vs
M:System.Numerics.TotalOrderIeee754Comparer`1.GetHashCode System.Int32 System.Numerics.TotalOrderIeee754Comparer<>.GetHashCode() 0

So my change also ends up removing the `0 as the type parameter argument in the method. I'll need to take a closer look at the workaround in ToComparisonDisplayString to see if I can constrain it to only apply to parts that are not method parameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ViktorHofer ViktorHofer ViktorHofer approved these changes

@andriipatsula andriipatsula Awaiting requested review from andriipatsula

Assignees

@ericstj ericstj

Labels
Area-ApiCompat untriaged Request triage from a team member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ericstj @ViktorHofer