Skip to content
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.

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

Filter out RequiredMemberAttribute from metadata symbols #63497

Merged
merged 7 commits into from
Aug 25, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -581,20 +581,15 @@ public override ImmutableArray<CSharpAttributeData> GetAttributes()
{
var containingPEModuleSymbol = (PEModuleSymbol)this.ContainingModule;

if (FilterOutDecimalConstantAttribute())
{
// filter out DecimalConstantAttribute
var attributes = containingPEModuleSymbol.GetCustomAttributesForToken(
_handle,
out _,
AttributeDescription.DecimalConstantAttribute);

ImmutableInterlocked.InterlockedInitialize(ref _lazyCustomAttributes, attributes);
}
else
{
containingPEModuleSymbol.LoadCustomAttributes(_handle, ref _lazyCustomAttributes);
}
var attributes = containingPEModuleSymbol.GetCustomAttributesForToken(
_handle,
out _,
FilterOutDecimalConstantAttribute() ? AttributeDescription.DecimalConstantAttribute : default,
out CustomAttributeHandle required,
AttributeDescription.RequiredMemberAttribute);

ImmutableInterlocked.InterlockedInitialize(ref _lazyCustomAttributes, attributes);
_packedFlags.SetHasRequiredMemberAttribute(!required.IsNil);
333fred marked this conversation as resolved.
Show resolved Hide resolved
}
return _lazyCustomAttributes;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -982,7 +982,9 @@ public override ImmutableArray<CSharpAttributeData> GetAttributes()
filteredOutAttribute4: out _,
filterOut4: (checkForRequiredMembers && ObsoleteAttributeData is null) ? AttributeDescription.ObsoleteAttribute : default,
filteredOutAttribute5: out _,
filterOut5: default);
filterOut5: default,
filteredOutAttribute6: out _,
filterOut6: default);

isExtensionMethod = !extensionAttribute.IsNil;
isReadOnly = !isReadOnlyAttribute.IsNil;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,11 +294,20 @@ internal ImmutableArray<CSharpAttributeData> GetCustomAttributesForToken(EntityH
out CustomAttributeHandle filteredOutAttribute1,
AttributeDescription filterOut1)
{
return GetCustomAttributesForToken(token, out filteredOutAttribute1, filterOut1, out _, default, out _, default, out _, default, out _, default);
return GetCustomAttributesForToken(token, out filteredOutAttribute1, filterOut1, out _, default, out _, default, out _, default, out _, default, out _, default);
}

internal ImmutableArray<CSharpAttributeData> GetCustomAttributesForToken(EntityHandle token,
out CustomAttributeHandle filteredOutAttribute1,
AttributeDescription filterOut1,
out CustomAttributeHandle filteredOutAttribute2,
AttributeDescription filterOut2)
{
return GetCustomAttributesForToken(token, out filteredOutAttribute1, filterOut1, out filteredOutAttribute2, filterOut2, out _, default, out _, default, out _, default, out _, default);
}

/// <summary>
/// Returns attributes with up-to four filters applied. For each filter, the last application of the
/// Returns attributes with up-to 6 filters applied. For each filter, the last application of the
/// attribute will be tracked and returned.
/// </summary>
internal ImmutableArray<CSharpAttributeData> GetCustomAttributesForToken(EntityHandle token,
Expand All @@ -311,13 +320,16 @@ internal ImmutableArray<CSharpAttributeData> GetCustomAttributesForToken(EntityH
out CustomAttributeHandle filteredOutAttribute4,
AttributeDescription filterOut4,
out CustomAttributeHandle filteredOutAttribute5,
AttributeDescription filterOut5)
AttributeDescription filterOut5,
out CustomAttributeHandle filteredOutAttribute6,
AttributeDescription filterOut6)
{
filteredOutAttribute1 = default;
filteredOutAttribute2 = default;
filteredOutAttribute3 = default;
filteredOutAttribute4 = default;
filteredOutAttribute5 = default;
filteredOutAttribute6 = default;
ArrayBuilder<CSharpAttributeData> customAttributesBuilder = null;

try
Expand Down Expand Up @@ -357,6 +369,12 @@ internal ImmutableArray<CSharpAttributeData> GetCustomAttributesForToken(EntityH
continue;
}

if (matchesFilter(customAttributeHandle, filterOut6))
{
filteredOutAttribute6 = customAttributeHandle;
continue;
}

if (customAttributesBuilder == null)
{
customAttributesBuilder = ArrayBuilder<CSharpAttributeData>.GetInstance();
Expand Down Expand Up @@ -436,10 +454,7 @@ internal ImmutableArray<CSharpAttributeData> GetCustomAttributesFilterCompilerAt
filteredOutAttribute1: out CustomAttributeHandle extensionAttribute,
filterOut1: AttributeDescription.CaseSensitiveExtensionAttribute,
filteredOutAttribute2: out CustomAttributeHandle isReadOnlyAttribute,
filterOut2: AttributeDescription.IsReadOnlyAttribute,
filteredOutAttribute3: out _, filterOut3: default,
filteredOutAttribute4: out _, filterOut4: default,
filteredOutAttribute5: out _, filterOut5: default);
filterOut2: AttributeDescription.IsReadOnlyAttribute);

foundExtension = !extensionAttribute.IsNil;
foundReadOnly = !isReadOnlyAttribute.IsNil;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -680,9 +680,19 @@ public override ImmutableArray<CSharpAttributeData> GetAttributes()
IsRefLikeType ? AttributeDescription.IsByRefLikeAttribute : default,
out _,
// Filter out [CompilerFeatureRequired]
(IsRefLikeType && DeriveCompilerFeatureRequiredDiagnostic() is null) ? AttributeDescription.CompilerFeatureRequiredAttribute : default);
(IsRefLikeType && DeriveCompilerFeatureRequiredDiagnostic() is null) ? AttributeDescription.CompilerFeatureRequiredAttribute : default,
out CustomAttributeHandle requiredHandle,
// Filter out [RequiredMember]
AttributeDescription.RequiredMemberAttribute);

ImmutableInterlocked.InterlockedInitialize(ref uncommon.lazyCustomAttributes, loadedCustomAttributes);

if (!uncommon.lazyHasRequiredMembers.HasValue())
333fred marked this conversation as resolved.
Show resolved Hide resolved
{
uncommon.lazyHasRequiredMembers = (!requiredHandle.IsNil).ToThreeState();
}

Debug.Assert(uncommon.lazyHasRequiredMembers.Value() != requiredHandle.IsNil);
}

return uncommon.lazyCustomAttributes;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1040,6 +1040,8 @@ public override ImmutableArray<CSharpAttributeData> GetAttributes()
out _,
AttributeDescription.ScopedRefAttribute,
out _,
default,
out _,
default);

if (!paramArrayAttribute.IsNil || !constantAttribute.IsNil)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -642,9 +642,12 @@ public override ImmutableArray<CSharpAttributeData> GetAttributes()
ImmutableArray<CSharpAttributeData> attributes = containingPEModuleSymbol.GetCustomAttributesForToken(
_handle,
out _,
this.RefKind == RefKind.RefReadOnly ? AttributeDescription.IsReadOnlyAttribute : default);
this.RefKind == RefKind.RefReadOnly ? AttributeDescription.IsReadOnlyAttribute : default,
out CustomAttributeHandle required,
AttributeDescription.RequiredMemberAttribute);

ImmutableInterlocked.InterlockedInitialize(ref _lazyCustomAttributes, attributes);
_flags.SetHasRequiredMemberAttribute(!required.IsNil);
333fred marked this conversation as resolved.
Show resolved Hide resolved
}
return _lazyCustomAttributes;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,7 @@ private static Action<ModuleSymbol> ValidateRequiredMembersInModule(string[] mem
AssertEx.NotNull(member, $"Member {memberPath} was not found");
Assert.True(member is PropertySymbol or FieldSymbol, $"Unexpected member symbol type {member.Kind}");
Assert.True(member.IsRequired());
if (module is SourceModuleSymbol)
{
Assert.All(member.GetAttributes(), attr => AssertEx.NotEqual("System.Runtime.CompilerServices.RequiredMemberAttribute", attr.AttributeClass.ToTestDisplayString()));
}
else
{
AssertEx.Any(member.GetAttributes(), attr => attr.AttributeClass.ToTestDisplayString() == "System.Runtime.CompilerServices.RequiredMemberAttribute");
}
Assert.All(member.GetAttributes(), attr => AssertEx.NotEqual("System.Runtime.CompilerServices.RequiredMemberAttribute", attr.AttributeClass.ToTestDisplayString()));

requiredTypes.Add((NamedTypeSymbol)member.ContainingType);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System.Collections.Immutable;
using System.Reflection.Metadata;
using System.Text;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE;
Expand All @@ -25,6 +26,50 @@ private RequiredMemberAttributesVisitor(StringBuilder builder) : base(builder)

protected override SymbolDisplayFormat DisplayFormat => SymbolDisplayFormat.TestFormat;

protected override void ReportSymbol(Symbol symbol)
{
EntityHandle handle;
PEModule module;

switch (symbol)
{
case PENamedTypeSymbol namedType:
handle = namedType.Handle;
module = ((PEModuleSymbol)namedType.ContainingModule).Module;
break;

case PEFieldSymbol field:
handle = field.Handle;
module = ((PEModuleSymbol)field.ContainingModule).Module;
break;

case PEPropertySymbol property:
handle = property.Handle;
module = ((PEModuleSymbol)property.ContainingModule).Module;
break;

default:
base.ReportSymbol(symbol);
return;
}

var attribute = module.GetAttributeHandle(handle, AttributeDescription.RequiredMemberAttribute);
333fred marked this conversation as resolved.
Show resolved Hide resolved

if (attribute.IsNil)
{
return;
}

ReportContainingSymbols(symbol);
_builder.Append(GetIndentString(symbol));
_builder.Append("[RequiredMember] ");
_builder.AppendLine(symbol.ToDisplayString(DisplayFormat));
_reported.Add(symbol);

// If attributes aren't filtered out, this will print extra data and cause an error in test assertion.
base.ReportSymbol(symbol);
}

protected override CSharpAttributeData? GetTargetAttribute(ImmutableArray<CSharpAttributeData> attributes)
=> GetAttribute(attributes, "System.Runtime.CompilerServices", "RequiredMemberAttribute");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,11 @@ public class C
public async Task TestRequiredProperty(bool signaturesOnly)
{
var metadataSource = """
public class C { public required int Property { get; set; } }
public class C
{
public required int Property { get; set; }
public required int Field;
}
namespace System.Runtime.CompilerServices
{
public sealed class RequiredMemberAttribute : Attribute { }
Expand All @@ -783,14 +787,12 @@ public CompilerFeatureRequiredAttribute(string featureName)
// {CodeAnalysisResources.InMemoryAssembly}
#endregion

using System.Runtime.CompilerServices;

[RequiredMember]
public class [|C|]
{{
public required int Field;

public C();

[RequiredMember]
public required int Property {{ get; set; }}
}}",
false => $@"#region {FeaturesResources.Assembly} ReferencedAssembly, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null
Expand All @@ -804,6 +806,9 @@ public class [|C|]
[RequiredMember]
public class [|C|]
{{
[RequiredMember]
public int Field;

[RequiredMember]
public int Property {{ get; set; }}

Expand Down