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

Support declaration and emit of required members in source #58507

Merged
merged 11 commits into from
Feb 4, 2022
21 changes: 21 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -6941,4 +6941,25 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="WRN_LowerCaseTypeName_Title" xml:space="preserve">
<value>The type name only contains lower-cased ascii characters. Such names may become reserved for the language.</value>
</data>
<data name="ERR_RequiredNameDisallowed" xml:space="preserve">
<value>Types and aliases cannot be named 'required'.</value>
</data>
<data name="IDS_FeatureRequiredMembers" xml:space="preserve">
<value>required members</value>
</data>
<data name="ERR_OverrideMustHaveRequired" xml:space="preserve">
<value>'{0}': cannot remove 'required' from '{1}' when overriding</value>
</data>
<data name="ERR_RequiredMemberCannotBeHidden" xml:space="preserve">
<value>Required member '{0}' cannot be hidden by '{1}'.</value>
</data>
<data name="ERR_RequiredMemberCannotBeLessVisibleThanContainingType" xml:space="preserve">
<value>Required member '{0}' cannot be less visible or have a setter less visible than the containing type '{1}'.</value>
</data>
<data name="ERR_ExplicitRequiredMember" xml:space="preserve">
<value>Do not use 'System.Runtime.CompilerServices.RequiredMemberAttribute'. Use the 'required' keyword on required fields and properties instead.</value>
</data>
<data name="ERR_RequiredMemberMustBeSettable" xml:space="preserve">
<value>Required member '{0}' must be settable.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -750,6 +750,7 @@ private static ImmutableSegmentedDictionary<string, VoidResult> GetNonTypeMember
bool anyMethodHadExtensionSyntax = false;
bool anyMemberHasAttributes = false;
bool anyNonTypeMembers = false;
bool anyRequiredMembers = false;

var memberNameBuilder = s_memberNameBuilderPool.Allocate();

Expand All @@ -769,6 +770,11 @@ private static ImmutableSegmentedDictionary<string, VoidResult> GetNonTypeMember
{
anyMemberHasAttributes = true;
}

if (!anyRequiredMembers && checkPropertyOrFieldMemberForRequiredModifier(member))
{
anyRequiredMembers = true;
}
}

if (anyMethodHadExtensionSyntax)
Expand All @@ -786,7 +792,24 @@ private static ImmutableSegmentedDictionary<string, VoidResult> GetNonTypeMember
declFlags |= SingleTypeDeclaration.TypeDeclarationFlags.HasAnyNontypeMembers;
}

if (anyRequiredMembers)
{
declFlags |= SingleTypeDeclaration.TypeDeclarationFlags.HasRequiredMembers;
}

return ToImmutableAndFree(memberNameBuilder);

static bool checkPropertyOrFieldMemberForRequiredModifier(Syntax.InternalSyntax.CSharpSyntaxNode member)
{
var modifiers = member switch
{
Syntax.InternalSyntax.FieldDeclarationSyntax fieldDeclaration => fieldDeclaration.Modifiers,
Syntax.InternalSyntax.PropertyDeclarationSyntax propertyDeclaration => propertyDeclaration.Modifiers,
_ => default
};

return modifiers.Any((int)SyntaxKind.RequiredKeyword);
}
}

private static bool CheckMethodMemberForExtensionSyntax(Syntax.InternalSyntax.CSharpSyntaxNode member)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ internal enum TypeDeclarationFlags : ushort
HasReturnWithExpression = 1 << 8,

IsSimpleProgram = 1 << 9,

HasRequiredMembers = 1 << 10,
}

internal SingleTypeDeclaration(
Expand Down Expand Up @@ -189,6 +191,8 @@ public bool IsSimpleProgram
}
}

public bool HasRequiredMembers => (_flags & TypeDeclarationFlags.HasRequiredMembers) != 0;

protected override ImmutableArray<SingleNamespaceOrTypeDeclaration> GetNamespaceOrTypeDeclarationChildren()
{
return StaticCast<SingleNamespaceOrTypeDeclaration>.From(_children);
Expand Down
9 changes: 9 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2029,5 +2029,14 @@ internal enum ErrorCode
#endregion

// Note: you will need to re-generate compiler code after adding warnings (eng\generate-compiler-code.cmd)

// PROTOTYPE(req): Move above the comment and condense before merge

ERR_RequiredNameDisallowed = 9500,
ERR_OverrideMustHaveRequired = 9501,
ERR_RequiredMemberCannotBeHidden = 9502,
ERR_RequiredMemberCannotBeLessVisibleThanContainingType = 9503,
ERR_ExplicitRequiredMember = 9504,
ERR_RequiredMemberMustBeSettable = 9505,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ public override bool IsExtern
get { return false; }
}

internal override bool IsRequired => false;

internal override ObsoleteAttributeData ObsoleteAttributeData
{
get { return null; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ internal sealed override bool HasSpecialName
get { return false; }
}

internal override bool HasDeclaredRequiredMembers => false;

public sealed override ImmutableArray<NamedTypeSymbol> GetTypeMembers()
{
return ImmutableArray<NamedTypeSymbol>.Empty;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, r
ImmutableArray.Create(
new TypedConstant(manager.System_Diagnostics_DebuggerBrowsableState, TypedConstantKind.Enum, DebuggerBrowsableState.Never))));
}

internal override bool IsRequired => false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ public override bool IsAbstract
get { return false; }
}

internal override bool IsRequired => false;

internal sealed override ObsoleteAttributeData ObsoleteAttributeData
{
get { return null; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ internal void AdjustLocation(Location location)
}
}

internal override bool HasDeclaredRequiredMembers => false;

public override ImmutableArray<Symbol> GetMembers()
{
return _members;
Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1377,6 +1377,9 @@ private static bool SatisfiesConstructorConstraint(TypeSymbol typeArgument)
private static bool HasPublicParameterlessConstructor(NamedTypeSymbol type, bool synthesizedIfMissing)
{
Debug.Assert(type.TypeKind is TypeKind.Class or TypeKind.Struct);

// PROTOTYPE(req): Adjust for required members

foreach (var constructor in type.InstanceConstructors)
{
if (constructor.ParameterCount == 0)
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/ErrorPropertySymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ public ErrorPropertySymbol(Symbol containingSymbol, TypeSymbol type, string name

public override bool IsExtern { get { return false; } }

internal override bool IsRequired => false;

internal sealed override ObsoleteAttributeData ObsoleteAttributeData { get { return null; } }

public override ImmutableArray<ParameterSymbol> Parameters { get { return ImmutableArray<ParameterSymbol>.Empty; } }
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/ErrorTypeSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ public override IEnumerable<string> MemberNames
}
}

internal sealed override bool HasDeclaredRequiredMembers => false;

/// <summary>
/// Get all the members of this symbol.
/// </summary>
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/FieldSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,11 @@ internal virtual FieldSymbol AsMember(NamedTypeSymbol newOwner)
return newOwner.IsDefinition ? this : new SubstitutedFieldSymbol(newOwner as SubstitutedNamedTypeSymbol, this);
}

/// <summary>
/// Returns true if this field is required to be set in an object initializer on object creation.
/// </summary>
internal abstract bool IsRequired { get; }

#region Use-Site Diagnostics

internal override UseSiteInfo<AssemblySymbol> GetUseSiteInfo()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -587,5 +587,8 @@ internal override ObsoleteAttributeData ObsoleteAttributeData
{
get { return null; }
}

// PROTOTYPE(req): Implement
internal override bool IsRequired => false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,9 @@ private static ICollection<string> CreateReadOnlyMemberNames(HashSet<string> nam
}
}

// PROTOTYPE(req): Implement
internal override bool HasDeclaredRequiredMembers => false;

public override ImmutableArray<Symbol> GetMembers()
{
EnsureAllMembersAreLoaded();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,15 @@ public override bool IsStatic
}
}

internal override bool IsRequired
{
get
{
// PROTOTYPE(req): Implement
return false;
}
}

public override ImmutableArray<ParameterSymbol> Parameters
{
get { return _parameters; }
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,11 @@ internal abstract bool MangleName
/// </summary>
public abstract IEnumerable<string> MemberNames { get; }

/// <summary>
/// True if this type declares any required members. It does not recursively check up the tree for _all_ required members.
/// </summary>
internal abstract bool HasDeclaredRequiredMembers { get; }

/// <summary>
/// Get all the members of this symbol.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ internal NativeIntegerTypeSymbol(NamedTypeSymbol underlyingType) : base(underlyi

public override IEnumerable<string> MemberNames => GetMembers().Select(m => m.Name);

internal override bool HasDeclaredRequiredMembers => false;
333fred marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Certain members from the underlying types are not exposed from the native integer types:
/// constructors other than the default parameterless constructor are not supported;
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/PropertySymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,11 @@ public bool IsWriteOnly
}
}

/// <summary>
/// Returns true if this property is required to be set in an object initializer on object creation.
/// </summary>
internal abstract bool IsRequired { get; }

/// <summary>
/// True if the property itself is excluded from code coverage instrumentation.
/// True for source properties marked with <see cref="AttributeDescription.ExcludeFromCodeCoverageAttribute"/>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ public override IEnumerable<string> MemberNames
}
}

internal override bool HasDeclaredRequiredMembers => _underlyingType.HasDeclaredRequiredMembers;

public override ImmutableArray<Symbol> GetMembers()
{
return this.RetargetingTranslator.Retarget(_underlyingType.GetMembers());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ public SignatureOnlyPropertySymbol(

public override bool IsExtern { get { throw ExceptionUtilities.Unreachable; } }

internal override bool IsRequired => throw ExceptionUtilities.Unreachable;

internal override ObsoleteAttributeData ObsoleteAttributeData { get { throw ExceptionUtilities.Unreachable; } }

public override AssemblySymbol ContainingAssembly { get { throw ExceptionUtilities.Unreachable; } }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,14 @@ internal override void DecodeWellKnownAttribute(ref DecodeWellKnownAttributeArgu
MarshalAsAttributeDecoder<FieldWellKnownAttributeData, AttributeSyntax, CSharpAttributeData, AttributeLocation>.Decode(ref arguments, AttributeTargets.Field, MessageProvider.Instance);
}
else if (ReportExplicitUseOfReservedAttributes(in arguments,
ReservedAttributes.DynamicAttribute | ReservedAttributes.IsReadOnlyAttribute | ReservedAttributes.IsUnmanagedAttribute | ReservedAttributes.IsByRefLikeAttribute | ReservedAttributes.TupleElementNamesAttribute | ReservedAttributes.NullableAttribute | ReservedAttributes.NativeIntegerAttribute))
ReservedAttributes.DynamicAttribute
| ReservedAttributes.IsReadOnlyAttribute
| ReservedAttributes.IsUnmanagedAttribute
| ReservedAttributes.IsByRefLikeAttribute
| ReservedAttributes.TupleElementNamesAttribute
| ReservedAttributes.NullableAttribute
| ReservedAttributes.NativeIntegerAttribute
| ReservedAttributes.RequiredMemberAttribute))
{
}
else if (attribute.IsTargetAttribute(this, AttributeDescription.DateTimeConstantAttribute))
Expand Down
28 changes: 24 additions & 4 deletions src/Compilers/CSharp/Portable/Symbols/Source/ModifierUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,13 @@ internal static DeclarationModifiers CheckModifiers(
modifierErrors = true;
}

if ((result & DeclarationModifiers.PrivateProtected) != 0)
{
modifierErrors |= !Binder.CheckFeatureAvailability(errorLocation.SourceTree, MessageID.IDS_FeaturePrivateProtected, diagnostics, errorLocation);
}
modifierErrors |= checkFeature(DeclarationModifiers.PrivateProtected, MessageID.IDS_FeaturePrivateProtected)
| checkFeature(DeclarationModifiers.Required, MessageID.IDS_FeatureRequiredMembers);

return result;

bool checkFeature(DeclarationModifiers modifier, MessageID featureID)
=> ((result & modifier) != 0) && !Binder.CheckFeatureAvailability(errorLocation.SourceTree, featureID, diagnostics, errorLocation);
}

private static void ReportPartialError(Location errorLocation, BindingDiagnosticBag diagnostics, SyntaxTokenList? modifierTokens)
Expand Down Expand Up @@ -281,6 +282,8 @@ internal static string ConvertSingleModifierToSyntaxText(DeclarationModifiers mo
return SyntaxFacts.GetText(SyntaxKind.AsyncKeyword);
case DeclarationModifiers.Ref:
return SyntaxFacts.GetText(SyntaxKind.RefKeyword);
case DeclarationModifiers.Required:
return SyntaxFacts.GetText(SyntaxKind.RequiredKeyword);
default:
throw ExceptionUtilities.UnexpectedValue(modifier);
}
Expand Down Expand Up @@ -328,6 +331,8 @@ private static DeclarationModifiers ToDeclarationModifier(SyntaxKind kind)
return DeclarationModifiers.Volatile;
case SyntaxKind.RefKeyword:
return DeclarationModifiers.Ref;
case SyntaxKind.RequiredKeyword:
return DeclarationModifiers.Required;
default:
throw ExceptionUtilities.UnexpectedValue(kind);
}
Expand Down Expand Up @@ -417,6 +422,21 @@ internal static CSDiagnosticInfo CheckAccessibility(DeclarationModifiers modifie
}
}

if ((modifiers & DeclarationModifiers.Required) != 0)
{
switch (symbol)
{
case FieldSymbol or PropertySymbol when symbol.DeclaredAccessibility < symbol.ContainingType.DeclaredAccessibility:
case PropertySymbol { SetMethod.DeclaredAccessibility: var accessibility } when accessibility < symbol.ContainingType.DeclaredAccessibility:
// Required member '{0}' cannot be less visible or have a setter less visible than the containing type '{1}'.
return new CSDiagnosticInfo(ErrorCode.ERR_RequiredMemberCannotBeLessVisibleThanContainingType, symbol, symbol.ContainingType);
case PropertySymbol { SetMethod: null }:
case FieldSymbol when (modifiers & DeclarationModifiers.ReadOnly) != 0:
333fred marked this conversation as resolved.
Show resolved Hide resolved
// Required member '{0}' must be settable.
return new CSDiagnosticInfo(ErrorCode.ERR_RequiredMemberMustBeSettable, symbol);
}
}

return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
// PROTOTYPE(req): Add obsolete marker to constructors if required members and Obsolete hasn't already been emitted
// PROTOTYPE(req): Add poison type marker to constructors if required members and Obsolete hasn't already been emitted,
// pending framework design review
internal sealed class SourceConstructorSymbol : SourceConstructorSymbolBase
{
private readonly bool _isExpressionBodied;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ internal sealed override bool HasRuntimeSpecialName
return this.Name == WellKnownMemberNames.EnumBackingFieldName;
}
}

internal override bool IsRequired => (Modifiers & DeclarationModifiers.Required) != 0;
}

internal abstract class SourceFieldSymbolWithSyntaxReference : SourceFieldSymbol
Expand Down
Loading