Extensions: add Name property on embedded ExtensionMarkerAttribute #80456

jcouv · 2025-09-24T21:33:31Z

Addresses part of #78963 ("Add Name property to synthesized attribute")
This shape matches that of the BCL attribute: dotnet/runtime#118179

Relates to test plan #76130

jjonescz · 2025-09-25T08:48:40Z

...CSharp/Portable/Symbols/Synthesized/SynthesizedEmbeddedExtensionMarkerNameAttributeSymbol.cs

+        }
+
+        public override IEnumerable<string> MemberNames
+            => [_nameField.Name, PropertyName, WellKnownMemberNames.InstanceConstructorName];


Consider using only constants:

Suggested change

=> [_nameField.Name, PropertyName, WellKnownMemberNames.InstanceConstructorName];

=> [FieldName, PropertyName, WellKnownMemberNames.InstanceConstructorName];

``` #Resolved

jjonescz · 2025-09-25T08:51:23Z

...CSharp/Portable/Symbols/Synthesized/SynthesizedEmbeddedExtensionMarkerNameAttributeSymbol.cs

+            public override MethodSymbol GetMethod { get; }
+            public override MethodSymbol? SetMethod => null;
+            public override Symbol ContainingSymbol => _backingField.ContainingSymbol;
+            public override Accessibility DeclaredAccessibility => Accessibility.Internal;


Should it be public to match the runtime API? #Resolved

The embedded type is internal (see SynthesizedEmbeddedAttributeSymbolBase.DeclaredAccessibility), so public or internal here are equivalent

I think we should match the accesibility

jjonescz · 2025-09-25T08:59:11Z

...CSharp/Portable/Symbols/Synthesized/SynthesizedEmbeddedExtensionMarkerNameAttributeSymbol.cs

+            public override ImmutableArray<ParameterSymbol> Parameters => [];
+            public override ImmutableArray<MethodSymbol> ExplicitInterfaceImplementations => [];
+            public override ImmutableArray<Location> Locations => [];
+            public override Accessibility DeclaredAccessibility => Accessibility.Internal;


Should this have the same accessibility as the property?

Suggested change

public override Accessibility DeclaredAccessibility => Accessibility.Internal;

public override Accessibility DeclaredAccessibility => AssociatedSymbol.DeclaredAccessibility;

``` #Resolved

It is the same and we already know the value, so doesn't seem useful to delegate

AlekseyTs · 2025-09-25T17:34:55Z

...CSharp/Portable/Symbols/Synthesized/SynthesizedEmbeddedExtensionMarkerNameAttributeSymbol.cs

-                    m => ImmutableArray.Create(SynthesizedParameterSymbol.Create(m, TypeWithAnnotations.Create(systemStringType), 0, RefKind.None, name: "name"))));
+            Debug.Assert(FieldName == GeneratedNames.MakeBackingFieldName(PropertyName));
+
+            _nameField = new SynthesizedFieldSymbol(this, systemStringType, FieldName);


SynthesizedFieldSymbol

isReadOnly: true? #Closed

AlekseyTs · 2025-09-25T17:38:22Z

...CSharp/Portable/Symbols/Synthesized/SynthesizedEmbeddedExtensionMarkerNameAttributeSymbol.cs

+        private sealed class ConstructorSymbol : SynthesizedInstanceConstructor
+        {
+            private readonly ImmutableArray<ParameterSymbol> _parameters;
+            private readonly SynthesizedFieldSymbol _nameField;


private readonly SynthesizedFieldSymbol _nameField;

I think this storage is redundant because the field symbol is available from containing type (through a private field) #Closed

AlekseyTs · 2025-09-25T17:47:23Z

...CSharp/Portable/Symbols/Synthesized/SynthesizedEmbeddedExtensionMarkerNameAttributeSymbol.cs

+                SynthesizedFieldSymbol nameField) :
+                base(containingType)
+            {
+                _parameters = [SynthesizedParameterSymbol.Create(containingType, TypeWithAnnotations.Create(systemStringType), ordinal: 0, RefKind.None, name: "name")];


containingType

The container for parameter looks wrong #Closed

AlekseyTs · 2025-09-25T17:58:28Z

...CSharp/Portable/Symbols/Synthesized/SynthesizedEmbeddedExtensionMarkerNameAttributeSymbol.cs

+        public override IEnumerable<string> MemberNames
+            => [FieldName, PropertyName, WellKnownMemberNames.InstanceConstructorName];
+
+        private sealed class ConstructorSymbol : SynthesizedInstanceConstructor


ConstructorSymbol

Would it make sense to use existing SynthesizedEmbeddedAttributeConstructorWithBodySymbol instead defining new class? #Closed

AlekseyTs · 2025-09-25T18:10:55Z

...CSharp/Portable/Symbols/Synthesized/SynthesizedEmbeddedExtensionMarkerNameAttributeSymbol.cs

+        private sealed partial class NameGetAccessorMethodSymbol : SynthesizedMethodSymbol
+        {
+            private readonly NamePropertySymbol _nameProperty;
+            private readonly SynthesizedFieldSymbol _backingField;


private readonly SynthesizedFieldSymbol _backingField;

This field looks redundant #Closed

AlekseyTs · 2025-09-25T18:20:54Z

...CSharp/Portable/Symbols/Synthesized/SynthesizedEmbeddedExtensionMarkerNameAttributeSymbol.cs

+
+            internal override void GenerateMethodBody(TypeCompilationState compilationState, BindingDiagnosticBag diagnostics)
+            {
+                SyntheticBoundNodeFactory F = new SyntheticBoundNodeFactory(this, this.GetNonNullSyntaxNode(), compilationState, diagnostics);


this.GetNonNullSyntaxNode()

Is there a chance that is is going to produce non-Dummy node? If not, consider using it directly. #Closed

AlekseyTs · 2025-09-25T18:22:11Z

...CSharp/Portable/Symbols/Synthesized/SynthesizedEmbeddedExtensionMarkerNameAttributeSymbol.cs

+            public override ImmutableArray<ParameterSymbol> Parameters => [];
+            public override ImmutableArray<MethodSymbol> ExplicitInterfaceImplementations => [];
+            public override ImmutableArray<Location> Locations => [];
+            public override Accessibility DeclaredAccessibility => Accessibility.Internal;


Accessibility.Internal

I think accessibility should be public #Closed

~~Why?~~ Forget it, it's fine

AlekseyTs · 2025-09-25T18:24:49Z

...CSharp/Portable/Symbols/Synthesized/SynthesizedEmbeddedExtensionMarkerNameAttributeSymbol.cs

+                }
+                catch (SyntheticBoundNodeFactory.MissingPredefinedMember ex)
+                {
+                    diagnostics.Add(ex.Diagnostic);


diagnostics.Add(ex.Diagnostic);

It looks like other similar places also do F.CloseMethod(F.ThrowNull()); #Closed

AlekseyTs · 2025-09-25T18:31:51Z

src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests2.cs

+        IL_000d: ret
    } // end of method ExtensionMarkerAttribute::.ctor
+    // Properties
+    .property instance string Name()


.property instance string Name()

Is there a way to test runtime behavior of this property. Perhaps getting it from an applied attribute through a reflection? #Closed

I see a test below does that.

AlekseyTs · 2025-09-25T18:46:06Z

...CSharp/Portable/Symbols/Synthesized/SynthesizedEmbeddedExtensionMarkerNameAttributeSymbol.cs

+            {
+                FieldName => [_nameField],
+                PropertyName => [_nameProperty],
+                WellKnownMemberNames.InstanceConstructorName => [_constructors[0]],


[_constructors[0]]

_constructors? #Closed

Types don't line up

Types don't line up

There is a CastUp helper that allows to cast to an array of base class without reallocation

AlekseyTs · 2025-09-25T18:46:57Z

Done with review pass (commit 2) #Closed

AlekseyTs · 2025-09-25T20:16:10Z

...CSharp/Portable/Symbols/Synthesized/SynthesizedEmbeddedExtensionMarkerNameAttributeSymbol.cs

+                try
+                {
+                    // return this._backingField;
+                    F.CloseMethod(F.Return(F.Field(F.This(), ((SynthesizedEmbeddedExtensionMarkerAttributeSymbol)_nameProperty.ContainingSymbol)._nameField)));


((SynthesizedEmbeddedExtensionMarkerAttributeSymbol)_nameProperty.ContainingSymbol)._nameField

It looks like this is a "longer" way to get to _nameProperty._backingField, which is going to read that field in order to calculate _nameProperty.ContainingSymbol. #Closed

AlekseyTs · 2025-09-25T20:19:08Z

src/Compilers/CSharp/Test/Emit3/Semantics/ExtensionTests2.cs

+    // Fields
+    .field private initonly string '<Name>k__BackingField'
+    // Methods
+    .method assembly hidebysig specialname 


assembly

I would expect accessibility changed to public #Closed

AlekseyTs · 2025-09-25T20:20:46Z

Done with review pass (commit 3) #Closed

AlekseyTs

LGTM (commit 5)

jcouv self-assigned this Sep 24, 2025

jcouv added Area-Compilers Feature - Extension Everything The extension everything feature labels Sep 24, 2025

jcouv mentioned this pull request Sep 24, 2025

Extensions: follow-ups for metadata #78963

Open

28 tasks

Extensions: add Name property on embedded ExtensionMarkerAttribute

7e16850

jcouv force-pushed the extensions-name branch from b62e77a to 7e16850 Compare September 24, 2025 23:49

jcouv marked this pull request as ready for review September 25, 2025 04:10

jcouv requested a review from a team as a code owner September 25, 2025 04:10

jjonescz reviewed Sep 25, 2025

View reviewed changes

jjonescz approved these changes Sep 25, 2025

View reviewed changes

Address feedback

5ff0615

build-analysis bot mentioned this pull request Sep 25, 2025

The active test run was aborted. Reason: Test host process crashed dotnet/dnceng#451

Closed

3 tasks

AlekseyTs reviewed Sep 25, 2025

View reviewed changes

Julien Couvreur added 2 commits September 25, 2025 12:45

Address feedback

ed6eb15

CastUp

249f934

AlekseyTs reviewed Sep 25, 2025

View reviewed changes

Address feedback

22bb8d2

jjonescz approved these changes Sep 26, 2025

View reviewed changes

AlekseyTs approved these changes Sep 26, 2025

View reviewed changes

jcouv merged commit e477ee2 into dotnet:main Sep 26, 2025
24 checks passed

jcouv deleted the extensions-name branch September 26, 2025 16:21

dotnet-policy-service bot added this to the Next milestone Sep 26, 2025

This was referenced Oct 4, 2025

[Automated] PRs inserted in VS build main-11103.153 #80558

Closed

[Automated] PRs inserted in VS build feature.debugger.main-11103.170 #80560

Closed

	=> [_nameField.Name, PropertyName, WellKnownMemberNames.InstanceConstructorName];
	=> [FieldName, PropertyName, WellKnownMemberNames.InstanceConstructorName];
	``` #Resolved

	public override Accessibility DeclaredAccessibility => Accessibility.Internal;
	public override Accessibility DeclaredAccessibility => AssociatedSymbol.DeclaredAccessibility;
	``` #Resolved

Extensions: add Name property on embedded ExtensionMarkerAttribute #80456

Extensions: add Name property on embedded ExtensionMarkerAttribute #80456

Uh oh!

Conversation

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

Uh oh!

Uh oh!

jjonescz Sep 25, 2025 • edited by jcouv Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jjonescz Sep 25, 2025 • edited by jcouv Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcouv Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

jjonescz Sep 25, 2025 • edited by jcouv Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcouv Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Sep 25, 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 Sep 25, 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 Sep 25, 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 Sep 25, 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 Sep 25, 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 Sep 25, 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 Sep 25, 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!

jcouv Sep 25, 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 Sep 25, 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 Sep 25, 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 Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Sep 25, 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!

jcouv Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Sep 25, 2025

Choose a reason for hiding this comment

jcouv commented Sep 24, 2025 •

edited

Loading

jjonescz Sep 25, 2025 •

edited by jcouv

Loading

jjonescz Sep 25, 2025 •

edited by jcouv

Loading

jjonescz Sep 25, 2025 •

edited by jcouv

Loading

AlekseyTs Sep 25, 2025 •

edited

Loading

AlekseyTs Sep 25, 2025 •

edited

Loading

AlekseyTs Sep 25, 2025 •

edited

Loading

AlekseyTs Sep 25, 2025 •

edited

Loading

AlekseyTs Sep 25, 2025 •

edited

Loading

AlekseyTs Sep 25, 2025 •

edited

Loading

AlekseyTs Sep 25, 2025 •

edited

Loading

jcouv Sep 25, 2025 •

edited

Loading

AlekseyTs Sep 25, 2025 •

edited

Loading

AlekseyTs Sep 25, 2025 •

edited

Loading

AlekseyTs Sep 25, 2025 •

edited

Loading

AlekseyTs commented Sep 25, 2025 •

edited

Loading

AlekseyTs Sep 25, 2025 •

edited

Loading

AlekseyTs Sep 25, 2025 •

edited

Loading

AlekseyTs commented Sep 25, 2025 •

edited

Loading