Skip to content

Commit b349c65

Browse files
jjonesczRikkiGibson
jjonescz
and
RikkiGibson
authored
Partial events and constructors: attributes (#77182)
* Partial events and constructors: attributes * Update pre-existing tests * Fixup expected PEVerify output * Check a flag before materializing syntax nodes * Assert that the definition part of an event doesn't have accessors * Keep pre-existing event attribute target behavior * Improve code * Add WorkItem attribute * Strengthen asserts Co-authored-by: Rikki Gibson <rikkigibson@gmail.com> --------- Co-authored-by: Rikki Gibson <rikkigibson@gmail.com>
1 parent d053d06 commit b349c65

14 files changed

+1008
-66
lines changed

‎src/Compilers/CSharp/Portable/Symbols/MemberSymbolExtensions.cs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,18 @@ internal static bool IsPartialDefinition(this Symbol member)
593593
_ => null,
594594
};
595595
}
596+
597+
internal static Symbol? GetPartialDefinitionPart(this Symbol member)
598+
{
599+
Debug.Assert(member.IsDefinition);
600+
return member switch
601+
{
602+
MethodSymbol method => method.PartialDefinitionPart,
603+
SourcePropertySymbol property => property.PartialDefinitionPart,
604+
SourceEventSymbol ev => ev.PartialDefinitionPart,
605+
_ => null,
606+
};
607+
}
596608
#nullable disable
597609

598610
internal static bool ContainsTupleNames(this Symbol member)

‎src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -471,12 +471,7 @@ protected SourceParameterSymbol? PartialImplementationPart
471471
{
472472
get
473473
{
474-
ImmutableArray<ParameterSymbol> implParameters = this.ContainingSymbol switch
475-
{
476-
SourceMemberMethodSymbol { PartialImplementationPart.Parameters: { } parameters } => parameters,
477-
SourcePropertySymbol { PartialImplementationPart.Parameters: { } parameters } => parameters,
478-
_ => default
479-
};
474+
ImmutableArray<ParameterSymbol> implParameters = this.ContainingSymbol.GetPartialImplementationPart()?.GetParameters() ?? default;
480475

481476
if (implParameters.IsDefault)
482477
{
@@ -492,12 +487,7 @@ protected SourceParameterSymbol? PartialDefinitionPart
492487
{
493488
get
494489
{
495-
ImmutableArray<ParameterSymbol> defParameters = this.ContainingSymbol switch
496-
{
497-
SourceMemberMethodSymbol { PartialDefinitionPart.Parameters: { } parameters } => parameters,
498-
SourcePropertySymbol { PartialDefinitionPart.Parameters: { } parameters } => parameters,
499-
_ => default
500-
};
490+
ImmutableArray<ParameterSymbol> defParameters = this.ContainingSymbol.GetPartialDefinitionPart()?.GetParameters() ?? default;
501491

502492
if (defParameters.IsDefault)
503493
{

‎src/Compilers/CSharp/Portable/Symbols/Source/SourceConstructorSymbol.cs‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,11 +192,39 @@ private void CheckModifiers(MethodKind methodKind, bool hasBody, Location locati
192192
}
193193
}
194194

195+
#nullable enable
195196
internal override OneOrMany<SyntaxList<AttributeListSyntax>> GetAttributeDeclarations()
196197
{
197-
return OneOrMany.Create(((ConstructorDeclarationSyntax)this.SyntaxNode).AttributeLists);
198+
// Attributes on partial constructors are owned by the definition part.
199+
// If this symbol has a non-null PartialDefinitionPart, we should have accessed this method through that definition symbol instead.
200+
Debug.Assert(PartialDefinitionPart is null);
201+
202+
if (SourcePartialImplementationPart is { } implementationPart)
203+
{
204+
return OneOrMany.Create(
205+
this.AttributeDeclarationSyntaxList,
206+
implementationPart.AttributeDeclarationSyntaxList);
207+
}
208+
209+
return OneOrMany.Create(this.AttributeDeclarationSyntaxList);
210+
}
211+
212+
private SyntaxList<AttributeListSyntax> AttributeDeclarationSyntaxList
213+
{
214+
get
215+
{
216+
if (this.ContainingType is SourceMemberContainerTypeSymbol { AnyMemberHasAttributes: true })
217+
{
218+
return this.GetSyntax().AttributeLists;
219+
}
220+
221+
return default;
222+
}
198223
}
199224

225+
protected override SourceMemberMethodSymbol? BoundAttributesSource => SourcePartialDefinitionPart;
226+
#nullable disable
227+
200228
internal override bool IsNullableAnalysisEnabled()
201229
=> flags.HasThisInitializer
202230
? flags.IsNullableAnalysisEnabled

‎src/Compilers/CSharp/Portable/Symbols/Source/SourceCustomEventAccessorSymbol.cs‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,32 @@ public override Accessibility DeclaredAccessibility
7777
}
7878
}
7979

80+
#nullable enable
81+
protected override SourceMemberMethodSymbol? BoundAttributesSource => (SourceMemberMethodSymbol?)PartialDefinitionPart;
82+
8083
internal override OneOrMany<SyntaxList<AttributeListSyntax>> GetAttributeDeclarations()
8184
{
82-
return OneOrMany.Create(GetSyntax().AttributeLists);
85+
Debug.Assert(PartialImplementationPart is null);
86+
87+
// If this is a partial event, the corresponding partial definition cannot have any accessor attributes
88+
// (there are no explicit accessors in source on the definition part - it has a field-like syntax).
89+
Debug.Assert(PartialDefinitionPart is null
90+
or SourceEventAccessorSymbol { AssociatedEvent.MemberSyntax: EventFieldDeclarationSyntax });
91+
92+
return OneOrMany.Create(this.AttributeDeclarationSyntaxList);
93+
}
94+
95+
internal SyntaxList<AttributeListSyntax> AttributeDeclarationSyntaxList
96+
{
97+
get
98+
{
99+
if (this.AssociatedEvent.containingType.AnyMemberHasAttributes)
100+
{
101+
return this.GetSyntax().AttributeLists;
102+
}
103+
104+
return default;
105+
}
83106
}
84107

85108
public override bool IsImplicitlyDeclared

‎src/Compilers/CSharp/Portable/Symbols/Source/SourceEventAccessorSymbol.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,8 @@ internal sealed override int TryGetOverloadResolutionPriority()
235235
}
236236

237237
#nullable enable
238+
protected abstract override SourceMemberMethodSymbol? BoundAttributesSource { get; }
239+
238240
public sealed override MethodSymbol? PartialImplementationPart => _event is { IsPartialDefinition: true, OtherPartOfPartial: { } other }
239241
? (MethodKind == MethodKind.EventAdd ? other.AddMethod : other.RemoveMethod)
240242
: null;

‎src/Compilers/CSharp/Portable/Symbols/Source/SourceEventSymbol.cs‎

Lines changed: 57 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -141,25 +141,34 @@ internal SyntaxList<AttributeListSyntax> AttributeDeclarationSyntaxList
141141
{
142142
get
143143
{
144-
if (this.containingType.AnyMemberHasAttributes)
144+
if (this.containingType.AnyMemberHasAttributes && MemberSyntax is { } memberSyntax)
145145
{
146-
var syntax = this.CSharpSyntaxNode;
147-
if (syntax != null)
146+
return memberSyntax.AttributeLists;
147+
}
148+
149+
return default;
150+
}
151+
}
152+
153+
internal MemberDeclarationSyntax? MemberSyntax
154+
{
155+
get
156+
{
157+
if (this.CSharpSyntaxNode is { } syntax)
158+
{
159+
switch (syntax.Kind())
148160
{
149-
switch (syntax.Kind())
150-
{
151-
case SyntaxKind.EventDeclaration:
152-
return ((EventDeclarationSyntax)syntax).AttributeLists;
153-
case SyntaxKind.VariableDeclarator:
154-
Debug.Assert(syntax.Parent!.Parent is object);
155-
return ((EventFieldDeclarationSyntax)syntax.Parent.Parent).AttributeLists;
156-
default:
157-
throw ExceptionUtilities.UnexpectedValue(syntax.Kind());
158-
}
161+
case SyntaxKind.EventDeclaration:
162+
return (EventDeclarationSyntax)syntax;
163+
case SyntaxKind.VariableDeclarator:
164+
Debug.Assert(syntax.Parent?.Parent is not null);
165+
return (EventFieldDeclarationSyntax)syntax.Parent.Parent;
166+
default:
167+
throw ExceptionUtilities.UnexpectedValue(syntax.Kind());
159168
}
160169
}
161170

162-
return default;
171+
return null;
163172
}
164173
}
165174

@@ -194,8 +203,26 @@ protected abstract AttributeLocation AllowedAttributeLocations
194203
/// </remarks>
195204
private CustomAttributesBag<CSharpAttributeData> GetAttributesBag()
196205
{
197-
if ((_lazyCustomAttributesBag == null || !_lazyCustomAttributesBag.IsSealed) &&
198-
LoadAndValidateAttributes(OneOrMany.Create(this.AttributeDeclarationSyntaxList), ref _lazyCustomAttributesBag))
206+
var bag = _lazyCustomAttributesBag;
207+
if (bag != null && bag.IsSealed)
208+
{
209+
return bag;
210+
}
211+
212+
bool bagCreatedOnThisThread;
213+
214+
if (SourcePartialDefinitionPart is { } definitionPart)
215+
{
216+
Debug.Assert(!ReferenceEquals(definitionPart, this));
217+
bag = definitionPart.GetAttributesBag();
218+
bagCreatedOnThisThread = Interlocked.CompareExchange(ref _lazyCustomAttributesBag, bag, null) == null;
219+
}
220+
else
221+
{
222+
bagCreatedOnThisThread = LoadAndValidateAttributes(this.GetAttributeDeclarations(), ref _lazyCustomAttributesBag);
223+
}
224+
225+
if (bagCreatedOnThisThread)
199226
{
200227
DeclaringCompilation.SymbolDeclaredEvent(this);
201228
var wasCompletedThisThread = _state.NotePartComplete(CompletionPart.Attributes);
@@ -206,6 +233,20 @@ private CustomAttributesBag<CSharpAttributeData> GetAttributesBag()
206233
return _lazyCustomAttributesBag;
207234
}
208235

236+
private OneOrMany<SyntaxList<AttributeListSyntax>> GetAttributeDeclarations()
237+
{
238+
// Attributes on partial events are owned by the definition part.
239+
// If this symbol has a non-null PartialDefinitionPart, we should have accessed this method through that definition symbol instead.
240+
Debug.Assert(PartialDefinitionPart is null);
241+
242+
if (SourcePartialImplementationPart is { } implementationPart)
243+
{
244+
return OneOrMany.Create(this.AttributeDeclarationSyntaxList, implementationPart.AttributeDeclarationSyntaxList);
245+
}
246+
247+
return OneOrMany.Create(this.AttributeDeclarationSyntaxList);
248+
}
249+
209250
/// <summary>
210251
/// Gets the attributes applied on this symbol.
211252
/// Returns an empty array if there are no attributes.

‎src/Compilers/CSharp/Portable/Symbols/Source/SourceFieldLikeEventSymbol.cs‎

Lines changed: 115 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,12 @@
55
using System;
66
using System.Collections.Immutable;
77
using System.Diagnostics;
8+
using System.Reflection;
89
using System.Threading;
10+
using Microsoft.CodeAnalysis.CSharp.Emit;
911
using Microsoft.CodeAnalysis.CSharp.Syntax;
12+
using Microsoft.CodeAnalysis.PooledObjects;
13+
using Roslyn.Utilities;
1014

1115
namespace Microsoft.CodeAnalysis.CSharp.Symbols
1216
{
@@ -175,9 +179,19 @@ protected override AttributeLocation AllowedAttributeLocations
175179
{
176180
get
177181
{
178-
return (object?)AssociatedEventField != null ?
179-
AttributeLocation.Event | AttributeLocation.Method | AttributeLocation.Field :
180-
AttributeLocation.Event | AttributeLocation.Method;
182+
var result = AttributeLocation.Event;
183+
184+
if (!IsPartial || IsExtern)
185+
{
186+
result |= AttributeLocation.Method;
187+
}
188+
189+
if (AssociatedEventField is not null)
190+
{
191+
result |= AttributeLocation.Field;
192+
}
193+
194+
return result;
181195
}
182196
}
183197

@@ -239,6 +253,104 @@ internal SourceEventDefinitionAccessorSymbol(
239253
{
240254
return null;
241255
}
256+
257+
protected override SourceMemberMethodSymbol? BoundAttributesSource
258+
{
259+
get
260+
{
261+
return IsExtern && this.MethodKind == MethodKind.EventAdd
262+
? (SourceMemberMethodSymbol?)this.AssociatedEvent.RemoveMethod
263+
: null;
264+
}
265+
}
266+
267+
protected override IAttributeTargetSymbol AttributeOwner
268+
{
269+
get
270+
{
271+
Debug.Assert(IsPartialDefinition);
272+
273+
switch (PartialImplementationPart)
274+
{
275+
case SourceCustomEventAccessorSymbol:
276+
return this;
277+
278+
case SynthesizedEventAccessorSymbol:
279+
Debug.Assert(IsExtern);
280+
return AssociatedEvent;
281+
282+
case null:
283+
// Might happen in error scenarios.
284+
return this;
285+
286+
default:
287+
Debug.Assert(false);
288+
return this;
289+
}
290+
}
291+
}
292+
293+
internal override OneOrMany<SyntaxList<AttributeListSyntax>> GetAttributeDeclarations()
294+
{
295+
Debug.Assert(IsPartialDefinition);
296+
297+
switch (PartialImplementationPart)
298+
{
299+
case SourceCustomEventAccessorSymbol customImplementationPart:
300+
return OneOrMany.Create(customImplementationPart.AttributeDeclarationSyntaxList);
301+
302+
case SynthesizedEventAccessorSymbol synthesizedImplementationPart:
303+
Debug.Assert(IsExtern);
304+
return OneOrMany.Create(
305+
AssociatedEvent.AttributeDeclarationSyntaxList,
306+
synthesizedImplementationPart.AssociatedEvent.AttributeDeclarationSyntaxList);
307+
308+
case null:
309+
// Might happen in error scenarios.
310+
return OneOrMany<SyntaxList<AttributeListSyntax>>.Empty;
311+
312+
default:
313+
Debug.Assert(false);
314+
return OneOrMany<SyntaxList<AttributeListSyntax>>.Empty;
315+
}
316+
}
317+
318+
internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, ref ArrayBuilder<CSharpAttributeData> attributes)
319+
{
320+
Debug.Assert(IsPartialDefinition);
321+
322+
if (PartialImplementationPart is { } implementationPart)
323+
{
324+
implementationPart.AddSynthesizedAttributes(moduleBuilder, ref attributes);
325+
}
326+
else
327+
{
328+
// This could happen in error scenarios (when the implementation part of a partial event is missing),
329+
// but then we should not get to the emit stage and call this method.
330+
Debug.Assert(false);
331+
332+
base.AddSynthesizedAttributes(moduleBuilder, ref attributes);
333+
}
334+
}
335+
336+
internal override MethodImplAttributes ImplementationAttributes
337+
{
338+
get
339+
{
340+
Debug.Assert(IsPartialDefinition);
341+
342+
if (PartialImplementationPart is { } implementationPart)
343+
{
344+
return implementationPart.ImplementationAttributes;
345+
}
346+
347+
// This could happen in error scenarios (when the implementation part of a partial event is missing),
348+
// but then we should not get to the emit stage and call this property.
349+
Debug.Assert(false);
350+
351+
return base.ImplementationAttributes;
352+
}
353+
}
242354
}
243355
}
244356
}

‎src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1743,19 +1743,7 @@ internal void AssertMemberExposure(Symbol member, bool forDiagnostics = false)
17431743

17441744
static bool isMemberInCompleteMemberList(MembersAndInitializers? membersAndInitializers, Symbol member)
17451745
{
1746-
switch (member)
1747-
{
1748-
case MethodSymbol method:
1749-
member = method.PartialDefinitionPart ?? method;
1750-
break;
1751-
case PropertySymbol property:
1752-
member = property.PartialDefinitionPart ?? property;
1753-
break;
1754-
case EventSymbol ev:
1755-
member = ev.PartialDefinitionPart ?? ev;
1756-
break;
1757-
}
1758-
1746+
member = member.GetPartialDefinitionPart() ?? member;
17591747
return membersAndInitializers?.NonTypeMembers.Contains(m => m == (object)member) == true;
17601748
}
17611749
}

0 commit comments

Comments
 (0)