Skip to content

Ensure custom attributes bag of a parameter backing field is properly initialized during "completion" of a primary constructor. #80424

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

Merged
merged 1 commit into from
Sep 24, 2025
Merged

Ensure custom attributes bag of a parameter backing field is properly initialized during "completion" of a primary constructor. #80424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3997,7 +3997,7 @@ spread.ElementPlaceholder is { } elementPlaceholder &&
TypeWithState convertCollection(BoundCollectionExpression node, TypeWithAnnotations targetCollectionType, ArrayBuilder<Func<TypeWithAnnotations, TypeSymbol, TypeWithState>> completions)
{
var strippedTargetCollectionType = targetCollectionType.Type.StrippedType();
Debug.Assert(TypeSymbol.Equals(strippedTargetCollectionType, node.Type, TypeCompareKind.IgnoreNullableModifiersForReferenceTypes));
Debug.Assert(TypeSymbol.Equals(strippedTargetCollectionType, node.Type, TypeCompareKind.AllIgnoreOptions));

// https://github.com/dotnet/roslyn/issues/68786: Use inferInitialObjectState() to set the initial
// state of the instance: see the call to InheritNullableStateOfTrackableStruct() in particular.
Expand Down
18 changes: 9 additions & 9 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberMethodSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -901,15 +901,6 @@ internal override void ForceComplete(SourceLocation? locationOpt, Predicate<Symb
{
case CompletionPart.Attributes:
GetAttributes();

if (this is SynthesizedPrimaryConstructor primaryConstructor)
{
// The constructor is responsible for completion of the backing fields
foreach (var field in primaryConstructor.GetBackingFields())
{
field.GetAttributes();
}
}
break;

case CompletionPart.ReturnTypeAttributes:
Expand All @@ -927,6 +918,15 @@ internal override void ForceComplete(SourceLocation? locationOpt, Predicate<Symb
parameter.ForceComplete(locationOpt, filter: null, cancellationToken);
}

if (this is SynthesizedPrimaryConstructor primaryConstructor)
{
// The constructor is responsible for completion of the backing fields
foreach (var field in primaryConstructor.GetBackingFields())
{
field.GetAttributes();
}
}

state.NotePartComplete(CompletionPart.Parameters);
break;

Expand Down
16 changes: 13 additions & 3 deletions src/Compilers/CSharp/Test/Emit3/Semantics/PrimaryConstructorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3118,7 +3118,7 @@ public class A : System.Attribute
[field: A]
int P1)
{
int M1() => P1;
[A] int M1() => P1;
}

[System.AttributeUsage(System.AttributeTargets.Field, AllowMultiple = true) ]
Expand All @@ -3133,7 +3133,12 @@ public class C : System.Attribute
int M1() => P1;
}
";
var comp = CreateCompilation(source);
var comp = CreateCompilation(source, parseOptions: TestOptions.RegularDefault.WithFeature("run-nullable-analysis", "never"));
Copy link
Member

@jjonescz jjonescz Sep 24, 2025

Choose a reason for hiding this comment

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

Is .WithFeature("run-nullable-analysis", "never") somehow important for the test?

Copy link
Contributor Author

@AlekseyTs AlekseyTs Sep 24, 2025

Choose a reason for hiding this comment

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

Is .WithFeature("run-nullable-analysis", "never") somehow important for the test?

Yes, by default debug build performs nullable analysis even if one is not enabled in source. That requests attributes from symbols. We want to control when attributes are requested and from what symbols.

var test1 = comp.GetTypeByMetadataName("Test1");
var ctor = test1.InstanceConstructors.Where(c => !c.IsDefaultValueTypeConstructor()).Single();

ctor.GetAttributes();
Copy link
Member

@jjonescz jjonescz Sep 24, 2025

Choose a reason for hiding this comment

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

Consider asserting at least number of the attributes returned by this call.

Copy link
Contributor Author

@AlekseyTs AlekseyTs Sep 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Consider asserting at least number of the attributes returned by this call.

This is not interesting. We are requesting the attributes to get symbol into a certain state.


comp.VerifyDiagnostics(
// (8,6): warning CS0657: 'field' is not a valid attribute location for this declaration. Valid attribute locations for this declaration are 'param'. All attributes in this block will be ignored.
// [field: A]
Expand All @@ -3143,7 +3148,12 @@ public class C : System.Attribute
Diagnostic(ErrorCode.ERR_AttributeOnBadSymbolType, "C").WithArguments("C", "field").WithLocation(20, 6)
);

Assert.Empty(comp.GetTypeByMetadataName("Test1").InstanceConstructors.Where(c => !c.IsDefaultValueTypeConstructor()).Single().Parameters[0].GetAttributes());
Assert.Empty(ctor.Parameters[0].GetAttributes());

var field = test1.GetMembers().OfType<FieldSymbol>().Single();
Assert.Equal(ObsoleteAttributeKind.None, field.ObsoleteKind);
Assert.Empty(field.GetAttributes());

Assert.Equal(1, comp.GetTypeByMetadataName("Test2").InstanceConstructors.Where(c => !c.IsDefaultValueTypeConstructor()).Single().Parameters[0].GetAttributes().Count());
}

Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/VisualBasic/Portable/Semantics/Conversions.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3502,7 +3502,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
'From a type T? to a type S?
conv = ClassifyPredefinedConversion(srcUnderlying, dstUnderlying, useSiteInfo)
Debug.Assert((conv And ConversionKind.VarianceConversionAmbiguity) = 0)
Debug.Assert((conv And ConversionKind.DelegateRelaxationLevelMask) = 0 OrElse (conv And ConversionKind.Tuple) <> 0)
Debug.Assert(Not ConversionExists(conv) OrElse (conv And ConversionKind.DelegateRelaxationLevelMask) = 0 OrElse (conv And ConversionKind.Tuple) <> 0)

If IsWideningConversion(conv) Then
'From a type T? to a type S?, where there is a widening conversion from the type T to the type S.
Expand Down Expand Up @@ -3531,7 +3531,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
Return ConversionKind.NarrowingNullable
Else
conv = ClassifyPredefinedConversion(srcUnderlying, destination, useSiteInfo)
Debug.Assert((conv And ConversionKind.DelegateRelaxationLevelMask) = 0 OrElse (conv And ConversionKind.Tuple) <> 0)
Debug.Assert(Not ConversionExists(conv) OrElse (conv And ConversionKind.DelegateRelaxationLevelMask) = 0 OrElse (conv And ConversionKind.Tuple) <> 0)

If ConversionExists(conv) Then
'From a type S? to a type T, where there is a conversion from the type S to the type T.
Expand All @@ -3550,7 +3550,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic

Dim conv = ClassifyPredefinedConversion(source, dstUnderlying, useSiteInfo)
Debug.Assert((conv And ConversionKind.VarianceConversionAmbiguity) = 0)
Debug.Assert((conv And ConversionKind.DelegateRelaxationLevelMask) = 0 OrElse (conv And ConversionKind.Tuple) <> 0)
Debug.Assert(Not ConversionExists(conv) OrElse (conv And ConversionKind.DelegateRelaxationLevelMask) = 0 OrElse (conv And ConversionKind.Tuple) <> 0)

If IsWideningConversion(conv) Then
'From a type T to a type S?, where there is a widening conversion from the type T to the type S.
Expand Down
Loading