Skip to content

Extensions: Ignore ref kind of extension parameter for static members in betterness#79885

Merged
jcouv merged 2 commits intodotnet:mainfrom
jcouv:extension-static
Sep 3, 2025
Merged

Extensions: Ignore ref kind of extension parameter for static members in betterness#79885
jcouv merged 2 commits intodotnet:mainfrom
jcouv:extension-static

Conversation

@jcouv
Copy link
Member

@jcouv jcouv commented Aug 12, 2025

Closes #79171

Relates to test plan #76130

ParameterSymbol parameter = getParameterOrExtensionParameter(i, memberResolutionResult, parameters, member);

parameter1RefKind = parameter.RefKind;
parameterRefKind = GetParameterBetternessRefKind(parameter, member);
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 don't think this is observable. The callers of this local function who care about the returned parameterRefKind are for scenarios where the receiver is a COM import type, but a COM import type must be class or interface, which can't be used for extension parameter with ref modifier.

@jcouv jcouv force-pushed the extension-static branch from f4c85d3 to 97d3d09 Compare August 22, 2025 22:47
@jcouv jcouv force-pushed the extension-static branch from 97d3d09 to 81caf40 Compare August 22, 2025 23:00
@jcouv jcouv marked this pull request as ready for review August 22, 2025 23:02
@jcouv jcouv requested a review from a team as a code owner August 22, 2025 23:02
@jcouv
Copy link
Member Author

jcouv commented Aug 25, 2025

@dotnet/roslyn-compiler for second review. Thanks

@RikkiGibson RikkiGibson self-assigned this Aug 25, 2025
@jcouv jcouv requested a review from RikkiGibson August 26, 2025 22:04
@jcouv
Copy link
Member Author

jcouv commented Aug 28, 2025

@dotnet/roslyn-compiler for second review. Thanks

1 similar comment
@jcouv
Copy link
Member Author

jcouv commented Aug 29, 2025

@dotnet/roslyn-compiler for second review. Thanks

Copy link
Member

@RikkiGibson RikkiGibson left a comment

Choose a reason for hiding this comment

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

LGTM modulo the comments

private static RefKind GetParameterBetternessRefKind<TMember>(ParameterSymbol parameter, TMember member) where TMember : Symbol
{
// For static extension members, the ref kind of the extension parameter shouldn't affect betterness.
bool isExtensionParameterOfStaticExtensionMember = parameter is { ContainingSymbol: TypeSymbol { IsExtension: true } } && member.IsStatic;
Copy link
Member

@RikkiGibson RikkiGibson Sep 1, 2025

Choose a reason for hiding this comment

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

nit: it might also be good to check parameter == parameter.ContainingSymbol.ExtensionParameter. #Resolved

comp.VerifyEmitDiagnostics(
// (1,22): error CS9286: 'int' does not contain a definition for 'P' and no accessible extension member 'P' for receiver of type 'int' could be found (are you missing a using directive or an assembly reference?)
// System.Console.Write(int.P);
Diagnostic(ErrorCode.ERR_ExtensionResolutionFailed, "int.P").WithArguments("int", "P").WithLocation(1, 22));
Copy link
Member

@RikkiGibson RikkiGibson Sep 1, 2025

Choose a reason for hiding this comment

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

Could you explain this result? I expected the diagnostic to mention that the lookup is ambiguous, but, instead the message indicates there is no member with the given name. It would be good to at least mark this as something we can improve in the future. #Resolved

Copy link
Member Author

Choose a reason for hiding this comment

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

Added link to existing tracking issue for this

@jcouv jcouv enabled auto-merge (squash) September 3, 2025 07:28
@jcouv jcouv merged commit 09be0d7 into dotnet:main Sep 3, 2025
23 of 24 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Sep 3, 2025
@jcouv jcouv deleted the extension-static branch September 3, 2025 09:48
@akhera99 akhera99 modified the milestones: Next, 18.0 P1 Sep 22, 2025
@akhera99 akhera99 added this to the 18.0 P2 milestone Sep 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Compilers Feature - Extension Everything The extension everything feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extensions: betterness for static members should only consider type of extension parameter, not other modifiers

4 participants