Skip to content

Commit

Permalink
Add support for decoding required members in metadata (#59288)
Browse files Browse the repository at this point in the history
Added support for reading the RequiredMemberAttribute from metadata appropriately and returning the right result for IsRequired and HasDeclaredRequiredMembers for metadata infos.
  • Loading branch information
333fred authored Feb 15, 2022
1 parent 3004d67 commit 4adfb65
Show file tree
Hide file tree
Showing 19 changed files with 337 additions and 115 deletions.
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -6948,7 +6948,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<value>required members</value>
</data>
<data name="ERR_OverrideMustHaveRequired" xml:space="preserve">
<value>'{0}': cannot remove 'required' from '{1}' when overriding</value>
<value>'{0}' must be required because it overrides required member '{1}'</value>
</data>
<data name="ERR_RequiredMemberCannotBeHidden" xml:space="preserve">
<value>Required member '{0}' cannot be hidden by '{1}'.</value>
Expand Down
39 changes: 36 additions & 3 deletions src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,20 @@ internal sealed class PEFieldSymbol : FieldSymbol
private struct PackedFlags
{
// Layout:
// |..............................|vvvvv|
// |............................|rr|vvvvv|
//
// f = FlowAnalysisAnnotations. 5 bits (4 value bits + 1 completion bit).
// r = Required members. 2 bits (1 value bit + 1 completion bit).

private const int HasDisallowNullAttribute = 0x1 << 0;
private const int HasAllowNullAttribute = 0x1 << 1;
private const int HasMaybeNullAttribute = 0x1 << 2;
private const int HasNotNullAttribute = 0x1 << 3;
private const int FlowAnalysisAnnotationsCompletionBit = 0x1 << 4;

private const int HasRequiredMemberAttribute = 0x1 << 5;
private const int RequiredMemberCompletionBit = 0x1 << 6;

private int _bits;

public bool SetFlowAnalysisAnnotations(FlowAnalysisAnnotations value)
Expand Down Expand Up @@ -67,6 +71,24 @@ public bool TryGetFlowAnalysisAnnotations(out FlowAnalysisAnnotations value)
Debug.Assert(value == 0 || result);
return result;
}

public bool SetHasRequiredMemberAttribute(bool isRequired)
{
int bitsToSet = RequiredMemberCompletionBit | (isRequired ? HasRequiredMemberAttribute : 0);
return ThreadSafeFlagOperations.Set(ref _bits, bitsToSet);
}

public bool TryGetHasRequiredMemberAttribute(out bool hasRequiredMemberAttribute)
{
if ((_bits & RequiredMemberCompletionBit) != 0)
{
hasRequiredMemberAttribute = (_bits & HasRequiredMemberAttribute) != 0;
return true;
}

hasRequiredMemberAttribute = false;
return false;
}
}

private readonly FieldDefinitionHandle _handle;
Expand Down Expand Up @@ -588,7 +610,18 @@ internal override ObsoleteAttributeData ObsoleteAttributeData
get { return null; }
}

// PROTOTYPE(req): Implement
internal override bool IsRequired => false;
internal override bool IsRequired
{
get
{
if (!_packedFlags.TryGetHasRequiredMemberAttribute(out bool hasRequiredMemberAttribute))
{
hasRequiredMemberAttribute = ContainingPEModule.Module.HasAttribute(_handle, AttributeDescription.RequiredMemberAttribute);
_packedFlags.SetHasRequiredMemberAttribute(hasRequiredMemberAttribute);
}

return hasRequiredMemberAttribute;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ private class UncommonProperties
internal NamedTypeSymbol lazyComImportCoClassType = ErrorTypeSymbol.UnknownResultType;
internal ThreeState lazyHasEmbeddedAttribute = ThreeState.Unknown;
internal ThreeState lazyHasInterpolatedStringHandlerAttribute = ThreeState.Unknown;
internal ThreeState lazyHasRequiredMembers = ThreeState.Unknown;

#if DEBUG
internal bool IsDefaultValue()
Expand All @@ -157,7 +158,8 @@ internal bool IsDefaultValue()
lazyDefaultMemberName == null &&
(object)lazyComImportCoClassType == (object)ErrorTypeSymbol.UnknownResultType &&
!lazyHasEmbeddedAttribute.HasValue() &&
!lazyHasInterpolatedStringHandlerAttribute.HasValue();
!lazyHasInterpolatedStringHandlerAttribute.HasValue() &&
!lazyHasRequiredMembers.HasValue();
}
#endif
}
Expand Down Expand Up @@ -824,8 +826,26 @@ private static ICollection<string> CreateReadOnlyMemberNames(HashSet<string> nam
}
}

// PROTOTYPE(req): Implement
internal override bool HasDeclaredRequiredMembers => false;
internal override bool HasDeclaredRequiredMembers
{
get
{
var uncommon = GetUncommonProperties();
if (uncommon == s_noUncommonProperties)
{
return false;
}

if (uncommon.lazyHasRequiredMembers.HasValue())
{
return uncommon.lazyHasRequiredMembers.Value();
}

var hasRequiredMemberAttribute = ContainingPEModule.Module.HasAttribute(_handle, AttributeDescription.RequiredMemberAttribute);
uncommon.lazyHasRequiredMembers = hasRequiredMemberAttribute.ToThreeState();
return hasRequiredMemberAttribute;
}
}

public override ImmutableArray<Symbol> GetMembers()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
using Microsoft.CodeAnalysis.CSharp.DocumentationComments;
using Microsoft.CodeAnalysis.CSharp.Emit;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE
{
Expand Down Expand Up @@ -47,14 +48,53 @@ internal class PEPropertySymbol
private const int UnsetAccessibility = -1;
private int _declaredAccessibility = UnsetAccessibility;

private readonly Flags _flags;
private PackedFlags _flags;

[Flags]
private enum Flags : byte
private struct PackedFlags
{
IsSpecialName = 1,
IsRuntimeSpecialName = 2,
CallMethodsDirectly = 4
// Layout:
// |...........................|rr|c|n|s|
//
// s = special name flag. 1 bit
// n = runtime special name flag. 1 bit
// c = call methods directly flag. 1 bit
// r = Required member. 2 bits (1 bit for value + 1 completion bit).
private const int IsSpecialNameFlag = 1 << 0;
private const int IsRuntimeSpecialNameFlag = 1 << 1;
private const int CallMethodsDirectlyFlag = 1 << 2;
private const int HasRequiredMemberAttribute = 1 << 4;
private const int RequiredMemberCompletionBit = 1 << 5;

private int _bits;

public PackedFlags(bool isSpecialName, bool isRuntimeSpecialName, bool callMethodsDirectly)
{
_bits = (isSpecialName ? IsSpecialNameFlag : 0)
| (isRuntimeSpecialName ? IsRuntimeSpecialNameFlag : 0)
| (callMethodsDirectly ? CallMethodsDirectlyFlag : 0);
}

public void SetHasRequiredMemberAttribute(bool isRequired)
{
var bitsToSet = (isRequired ? HasRequiredMemberAttribute : 0) | RequiredMemberCompletionBit;
ThreadSafeFlagOperations.Set(ref _bits, bitsToSet);
}

public bool TryGetHasRequiredMemberAttribute(out bool hasRequiredMemberAttribute)
{
if ((_bits & RequiredMemberCompletionBit) != 0)
{
hasRequiredMemberAttribute = (_bits & HasRequiredMemberAttribute) != 0;
return true;
}

hasRequiredMemberAttribute = false;
return false;
}

public bool IsSpecialName => (_bits & IsSpecialNameFlag) != 0;
public bool IsRuntimeSpecialName => (_bits & IsRuntimeSpecialNameFlag) != 0;
public bool CallMethodsDirectly => (_bits & CallMethodsDirectlyFlag) != 0;
}

internal static PEPropertySymbol Create(
Expand Down Expand Up @@ -204,20 +244,10 @@ private PEPropertySymbol(
}
}

if (callMethodsDirectly)
{
_flags |= Flags.CallMethodsDirectly;
}

if ((mdFlags & PropertyAttributes.SpecialName) != 0)
{
_flags |= Flags.IsSpecialName;
}

if ((mdFlags & PropertyAttributes.RTSpecialName) != 0)
{
_flags |= Flags.IsRuntimeSpecialName;
}
_flags = new PackedFlags(
isSpecialName: (mdFlags & PropertyAttributes.SpecialName) != 0,
isRuntimeSpecialName: (mdFlags & PropertyAttributes.RTSpecialName) != 0,
callMethodsDirectly);

static bool anyUnexpectedRequiredModifiers(ParamInfo<TypeSymbol>[] propertyParams)
{
Expand Down Expand Up @@ -277,7 +307,7 @@ public override string Name

internal override bool HasSpecialName
{
get { return (_flags & Flags.IsSpecialName) != 0; }
get { return _flags.IsSpecialName; }
}

public override string MetadataName
Expand Down Expand Up @@ -466,8 +496,14 @@ internal override bool IsRequired
{
get
{
// PROTOTYPE(req): Implement
return false;
if (!_flags.TryGetHasRequiredMemberAttribute(out bool hasRequiredMemberAttribute))
{
var containingPEModuleSymbol = (PEModuleSymbol)this.ContainingModule;
hasRequiredMemberAttribute = containingPEModuleSymbol.Module.HasAttribute(_handle, AttributeDescription.RequiredMemberAttribute);
_flags.SetHasRequiredMemberAttribute(hasRequiredMemberAttribute);
}

return hasRequiredMemberAttribute;
}
}

Expand Down Expand Up @@ -632,7 +668,7 @@ public override ImmutableArray<PropertySymbol> ExplicitInterfaceImplementations

internal override bool MustCallMethodsDirectly
{
get { return (_flags & Flags.CallMethodsDirectly) != 0; }
get { return _flags.CallMethodsDirectly; }
}

private static bool DoSignaturesMatch(
Expand Down Expand Up @@ -767,7 +803,7 @@ internal override bool HasRuntimeSpecialName
{
get
{
return (_flags & Flags.IsRuntimeSpecialName) != 0;
return _flags.IsRuntimeSpecialName;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -908,7 +908,7 @@ void checkSingleOverriddenMember(Symbol overridingMember, Symbol overriddenMembe
}
else if (overriddenMember is PropertySymbol { IsRequired: true } && overridingMember is PropertySymbol { IsRequired: false })
{
// '{0}': cannot remove 'required' from '{1}' when overriding
// '{0}' must be required because it overrides required member '{1}'
diagnostics.Add(ErrorCode.ERR_OverrideMustHaveRequired, overridingMemberLocation, overridingMember, overriddenMember);
}
else
Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 4adfb65

Please sign in to comment.