Extensions: honor OverloadResolutionPriority attribute by jcouv · Pull Request #78219 · dotnet/roslyn

jcouv · 2025-04-19T20:08:52Z

Following LDM 2025-04-16

The "containing type" for purpose of ORPA is the enclosing static class.
We synthesize an ORPA on the implementation method for accessors to have same prioritization between usage as extension and usage via static/implementation method.

Relates to test plan #76130

AlekseyTs · 2025-04-22T18:35:36Z

src/Compilers/CSharp/Portable/Symbols/PropertySymbol.cs


-        internal bool CanHaveOverloadResolutionPriority => !IsOverride && !IsExplicitInterfaceImplementation && (IsIndexer || IsIndexedProperty);
+        internal bool CanHaveOverloadResolutionPriority
+            => !IsOverride && !IsExplicitInterfaceImplementation && (IsIndexer || IsIndexedProperty || this.GetIsNewExtensionMember());


|| this.GetIsNewExtensionMember()

I didn't think we decided to allow priorities on properties #Closed

AlekseyTs · 2025-04-22T18:37:02Z

src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs

                return (null, null);
            }
-            else if (IsIndexer && CSharpAttributeData.IsTargetEarlyAttribute(arguments.AttributeType, arguments.AttributeSyntax, AttributeDescription.OverloadResolutionPriorityAttribute))
+            else if ((IsIndexer || this.GetIsNewExtensionMember()) && CSharpAttributeData.IsTargetEarlyAttribute(arguments.AttributeType, arguments.AttributeSyntax, AttributeDescription.OverloadResolutionPriorityAttribute))


|| this.GetIsNewExtensionMember()

Similar comment #Closed

AlekseyTs · 2025-04-22T18:37:34Z

src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbolBase.cs

        internal sealed override int TryGetOverloadResolutionPriority()
        {
-            Debug.Assert(this.IsIndexer);
+            Debug.Assert(this.IsIndexer || this.GetIsNewExtensionMember());


|| this.GetIsNewExtensionMember()

Here as well #Closed

AlekseyTs · 2025-04-22T18:48:42Z

Done with review pass (commit 1)

AlekseyTs · 2025-04-24T14:39:30Z

src/Compilers/CSharp/Portable/Symbols/Extensions/SourceExtensionImplementationMethodSymbol.cs

+                {
+                    var arg = new TypedConstant(DeclaringCompilation.GetSpecialType(SpecialType.System_Int32), TypedConstantKind.Primitive, priority);
+
+                    AddSynthesizedAttribute(ref attributes, DeclaringCompilation.TrySynthesizeAttribute(


AddSynthesizedAttribute(ref attributes, DeclaringCompilation.TrySynthesizeAttribute(

I think we should simply copy the user applied attribute, the one applied to the property. #Closed

AlekseyTs · 2025-04-24T14:42:54Z

src/Compilers/CSharp/Portable/Symbols/Extensions/SourceExtensionImplementationMethodSymbol.cs

+            if (UnderlyingMethod is SourcePropertyAccessorSymbol { AssociatedSymbol: SourcePropertySymbolBase extensionProperty })
+            {
+                var priority = extensionProperty.OverloadResolutionPriority;
+                if (priority != 0)


priority != 0

Should the user applied attribute (if there is one) from the property be copied unconditionally? #Closed

AlekseyTs · 2025-04-24T14:53:25Z

src/Compilers/CSharp/Portable/Symbols/SubstitutedMethodSymbol.cs

        }

+        internal override int TryGetOverloadResolutionPriority()
+            => UnderlyingMethod.TryGetOverloadResolutionPriority();


UnderlyingMethod

OriginalDefinition? #Closed

AlekseyTs · 2025-04-24T14:56:03Z

src/Compilers/CSharp/Portable/Symbols/Extensions/RewrittenLambdaOrLocalFunctionSymbol.cs

        }

+        internal override int TryGetOverloadResolutionPriority()
+            => UnderlyingMethod.TryGetOverloadResolutionPriority();


UnderlyingMethod

_originalMethod? #Closed

AlekseyTs · 2025-04-24T14:56:44Z

src/Compilers/CSharp/Portable/Symbols/Retargeting/RetargetingMethodSymbol.cs

        }
+
+        internal override int TryGetOverloadResolutionPriority()
+            => UnderlyingMethod.TryGetOverloadResolutionPriority();


UnderlyingMethod

_underlyingMethod? #Closed

AlekseyTs · 2025-04-24T15:02:10Z

src/Compilers/CSharp/Portable/Symbols/Extensions/SourceExtensionImplementationMethodSymbol.cs

+                    var arg = new TypedConstant(DeclaringCompilation.GetSpecialType(SpecialType.System_Int32), TypedConstantKind.Primitive, priority);
+
+                    AddSynthesizedAttribute(ref attributes, DeclaringCompilation.TrySynthesizeAttribute(
+                        WellKnownMember.System_Runtime_CompilerServices_OverloadResolutionPriorityAttribute__ctor,


WellKnownMember.System_Runtime_CompilerServices_OverloadResolutionPriorityAttribute__ctor

It looks like we assume that this member can be successfully found. Why? #Closed

AlekseyTs · 2025-04-24T15:04:01Z

src/Compilers/Core/Portable/WellKnownMember.cs

        System_Text_Encoding__get_UTF8,
        System_Text_Encoding__GetString,

+        System_Runtime_CompilerServices_OverloadResolutionPriorityAttribute__ctor,


System_Runtime_CompilerServices_OverloadResolutionPriorityAttribute__ctor

I do not think we need this entry #Closed

AlekseyTs · 2025-04-24T15:05:03Z

Done with review pass (commit 2)

AlekseyTs

LGTM (commit 4), assuming CI is passing

jcouv · 2025-04-25T14:52:06Z

@jjonescz for review. Thanks

Extensions: honor OverloadResolutionPriority attribute

3aa9b6c

jcouv added Area-Compilers Feature - Extension Everything The extension everything feature labels Apr 19, 2025

jcouv self-assigned this Apr 19, 2025

ghost added the untriaged Issues and PRs which have not yet been triaged by a lead label Apr 19, 2025

jcouv marked this pull request as ready for review April 22, 2025 16:14

jcouv requested a review from a team as a code owner April 22, 2025 16:14

jcouv requested review from AlekseyTs and jjonescz April 22, 2025 16:14

AlekseyTs reviewed Apr 22, 2025

View reviewed changes

jcouv marked this pull request as draft April 23, 2025 07:51

Copy ORPA from extension property onto implementation method

9487441

AlekseyTs reviewed Apr 24, 2025

View reviewed changes

jcouv added 2 commits April 25, 2025 11:38

Address feedback

e5ef9ab

Add test for multiple

2649ffa

AlekseyTs approved these changes Apr 25, 2025

View reviewed changes

Merge remote-tracking branch 'dotnet/main' into extensions-orpa

0ade091

jcouv marked this pull request as ready for review April 25, 2025 14:51

Implement EECompilationContextMethod.TryGetOverloadResolutionPriority

cf7c4d5

jjonescz approved these changes Apr 28, 2025

View reviewed changes

jcouv merged commit 73f70f4 into dotnet:main Apr 28, 2025
28 checks passed

jcouv deleted the extensions-orpa branch April 28, 2025 08:54

dotnet-policy-service bot added this to the Next milestone Apr 28, 2025

This was referenced Apr 30, 2025

[Automated] PRs inserted in VS build main-10529.74 #78383

Closed

[Automated] PRs inserted in VS build feature.debugger.main-10601.163 #78410

Closed

dotnet-bot mentioned this pull request May 31, 2025

[Automated] PRs inserted in VS build feature.telemetry.d18OpenTelemetry-10630.96 #78782

Closed

RikkiGibson modified the milestones: Next, 18.0 P1 Aug 19, 2025

Conversation

jcouv commented Apr 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlekseyTs Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs commented Apr 22, 2025

Uh oh!

AlekseyTs Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs commented Apr 24, 2025

Uh oh!

AlekseyTs left a comment

Choose a reason for hiding this comment

Uh oh!

jcouv commented Apr 25, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jcouv commented Apr 19, 2025 •

edited

Loading

AlekseyTs Apr 22, 2025 •

edited

Loading

AlekseyTs Apr 22, 2025 •

edited

Loading

AlekseyTs Apr 22, 2025 •

edited

Loading

AlekseyTs Apr 24, 2025 •

edited

Loading

AlekseyTs Apr 24, 2025 •

edited

Loading

AlekseyTs Apr 24, 2025 •

edited

Loading

AlekseyTs Apr 24, 2025 •

edited

Loading

AlekseyTs Apr 24, 2025 •

edited

Loading

AlekseyTs Apr 24, 2025 •

edited

Loading

AlekseyTs Apr 24, 2025 •

edited

Loading