Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Support SetsRequiredMembersAttribute
Browse files Browse the repository at this point in the history
The SetsRequiredMembersAttribute prevents the compiler from checking the required member list of a type when calling that constructor, and suppresses any errors from a base type's list being invalid.

Specification: https://github.com/dotnet/csharplang/blob/main/proposals/required-members.md
Test plan: dotnet#57046
333fred committed Mar 25, 2022

Verified

This commit was signed with the committer’s verified signature. The key has expired.
333fred Fred Silberberg
1 parent f2d1b99 commit 7761d6b
Showing 34 changed files with 995 additions and 441 deletions.
45 changes: 26 additions & 19 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
@@ -5846,25 +5846,7 @@ internal bool TryPerformConstructorOverloadResolution(
}
}

if (suppressUnsupportedRequiredMembersError && useSiteInfo.AccumulatesDiagnostics && useSiteInfo.Diagnostics is { Count: not 0 })
{
diagnostics.AddDependencies(useSiteInfo);
foreach (var diagnostic in useSiteInfo.Diagnostics)
{
// We don't want to report this error here because we'll report ERR_RequiredMembersBaseTypeInvalid. That error is suppressable by the
// user using the `SetsRequiredMembers` attribute on the constructor, so reporting this error would prevent that from working.
if ((ErrorCode)diagnostic.Code == ErrorCode.ERR_RequiredMembersInvalid)
{
continue;
}

diagnostics.ReportUseSiteDiagnostic(diagnostic, errorLocation);
}
}
else
{
diagnostics.Add(errorLocation, useSiteInfo);
}
ReportConstructorUseSiteDiagnostics(errorLocation, diagnostics, suppressUnsupportedRequiredMembersError, useSiteInfo);

if (succeededIgnoringAccessibility)
{
@@ -5921,6 +5903,31 @@ internal bool TryPerformConstructorOverloadResolution(
return succeededConsideringAccessibility;
}

internal static bool ReportConstructorUseSiteDiagnostics(Location errorLocation, BindingDiagnosticBag diagnostics, bool suppressUnsupportedRequiredMembersError, CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
if (suppressUnsupportedRequiredMembersError && useSiteInfo.AccumulatesDiagnostics && useSiteInfo.Diagnostics is { Count: not 0 })
{
diagnostics.AddDependencies(useSiteInfo);
foreach (var diagnostic in useSiteInfo.Diagnostics)
{
// We don't want to report this error here because we'll report ERR_RequiredMembersBaseTypeInvalid. That error is suppressable by the
// user using the `SetsRequiredMembers` attribute on the constructor, so reporting this error would prevent that from working.
if ((ErrorCode)diagnostic.Code == ErrorCode.ERR_RequiredMembersInvalid)
{
continue;
}

diagnostics.ReportUseSiteDiagnostic(diagnostic, errorLocation);
}

return true;
}
else
{
return diagnostics.Add(errorLocation, useSiteInfo);
}
}

private ImmutableArray<MethodSymbol> GetAccessibleConstructorsForOverloadResolution(NamedTypeSymbol type, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
ImmutableArray<MethodSymbol> allInstanceConstructors;
Original file line number Diff line number Diff line change
@@ -195,6 +195,8 @@ internal override void GenerateMethodBody(TypeCompilationState compilationState,
F.CloseMethod(F.ThrowNull());
}
}

protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable;
}

internal abstract partial class MethodToClassRewriter
4 changes: 3 additions & 1 deletion src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
Original file line number Diff line number Diff line change
@@ -2142,7 +2142,9 @@ private static BoundCall GenerateBaseCopyConstructorInitializer(SynthesizedRecor
return null;
}

if (Binder.ReportUseSite(baseConstructor, diagnostics, diagnosticsLocation))
var constructorUseSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.DiscardedDependencies;
constructorUseSiteInfo.Add(baseConstructor.GetUseSiteInfo());
if (Binder.ReportConstructorUseSiteDiagnostics(diagnosticsLocation, diagnostics, suppressUnsupportedRequiredMembersError: constructor.HasSetsRequiredMembers, constructorUseSiteInfo))
{
return null;
}
7 changes: 5 additions & 2 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
Original file line number Diff line number Diff line change
@@ -871,6 +871,8 @@ void makeNotNullMembersMaybeNull()
}
}

return;

ImmutableArray<Symbol> getMembersNeedingDefaultInitialState()
{
if (_hasInitialState)
@@ -880,12 +882,13 @@ ImmutableArray<Symbol> getMembersNeedingDefaultInitialState()

// We don't use a default initial state for value type instance constructors without `: this()` because
// any usages of uninitialized fields will get definite assignment errors anyway.
bool hasConstructorInitializer = method.HasThisConstructorInitializer(out _);
if (!hasConstructorInitializer && (!method.ContainingType.IsValueType || method.IsStatic))
if (!method.HasThisConstructorInitializer(out _) && (!method.ContainingType.IsValueType || method.IsStatic))
{
return membersToBeInitialized(method.ContainingType, includeAllMembers: true);
}

// We want to presume all required members of the type are uninitialized, and in addition we want to set all fields to
// default if we can get to this constructor by doing so (ie, : this() in a value type).
return membersToBeInitialized(method.ContainingType, includeAllMembers: method.IncludeFieldInitializersInBody());

static ImmutableArray<Symbol> membersToBeInitialized(NamedTypeSymbol containingType, bool includeAllMembers)
Original file line number Diff line number Diff line change
@@ -255,5 +255,7 @@ internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree l
{
return _stateMachineType.KickoffMethod.CalculateLocalSyntaxOffset(localPosition, localTree);
}

protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable;
}
}
Original file line number Diff line number Diff line change
@@ -231,6 +231,8 @@ internal sealed override int CalculateLocalSyntaxOffset(int localPosition, Synta
{
throw ExceptionUtilities.Unreachable;
}

protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable;
}
}
}
10 changes: 10 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/ErrorMethodSymbol.cs
Original file line number Diff line number Diff line change
@@ -6,6 +6,7 @@

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp.Symbols
@@ -274,5 +275,14 @@ internal override bool GenerateDebugInfo
}

internal sealed override bool IsNullableAnalysisEnabled() => false;

protected override bool HasSetsRequiredMembersImpl
{
get
{
Debug.Assert(MethodKind == MethodKind.Constructor);
return false;
}
}
}
}
Original file line number Diff line number Diff line change
@@ -830,5 +830,6 @@ public override bool IsVararg
internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree localTree) => throw ExceptionUtilities.Unreachable;
internal override IEnumerable<SecurityAttribute> GetSecurityInformation() => throw ExceptionUtilities.Unreachable;
internal sealed override bool IsNullableAnalysisEnabled() => throw ExceptionUtilities.Unreachable;
protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable;
}
}
Original file line number Diff line number Diff line change
@@ -47,7 +47,7 @@ private struct PackedFlags
{
// We currently pack everything into a 32-bit int with the following layout:
//
// |u|t|s|r|q|p|ooo|n|m|l|k|j|i|h|g|f|e|d|c|b|aaaaa|
// |w|v|u|t|s|r|q|p|ooo|n|m|l|k|j|i|h|g|f|e|d|c|b|aaaaa|
//
// a = method kind. 5 bits.
// b = method kind populated. 1 bit.
@@ -72,7 +72,9 @@ private struct PackedFlags
// s = IsInitOnlyBit. 1 bit.
// t = IsInitOnlyPopulatedBit. 1 bit.
// u = IsUnmanagedCallersOnlyAttributePopulated. 1 bit.
// 6 bits remain for future purposes.
// v = IsSetsRequiredMembersBit. 1 bit.
// w = IsSetsRequiredMembersPopulated. 1 bit.
// 4 bits remain for future purposes.

private const int MethodKindOffset = 0;
private const int MethodKindMask = 0x1F;
@@ -98,6 +100,8 @@ private struct PackedFlags
private const int IsInitOnlyBit = 0x1 << 24;
private const int IsInitOnlyPopulatedBit = 0x1 << 25;
private const int IsUnmanagedCallersOnlyAttributePopulatedBit = 0x1 << 26;
private const int HasSetsRequiredMembersBit = 0x1 << 27;
private const int HasSetsRequiredMembersPopulatedBit = 0x1 << 28;

private int _bits;

@@ -134,6 +138,8 @@ public MethodKind MethodKind
public bool IsInitOnly => (_bits & IsInitOnlyBit) != 0;
public bool IsInitOnlyPopulated => (_bits & IsInitOnlyPopulatedBit) != 0;
public bool IsUnmanagedCallersOnlyAttributePopulated => (_bits & IsUnmanagedCallersOnlyAttributePopulatedBit) != 0;
public bool HasSetsRequiredMembers => (_bits & HasSetsRequiredMembersBit) != 0;
public bool HasSetsRequiredMembersPopulated => (_bits & HasSetsRequiredMembersPopulatedBit) != 0;

#if DEBUG
static PackedFlags()
@@ -240,6 +246,14 @@ public void SetIsUnmanagedCallersOnlyAttributePopulated()
{
ThreadSafeFlagOperations.Set(ref _bits, IsUnmanagedCallersOnlyAttributePopulatedBit);
}

public bool InitializeSetsRequiredMembersBit(bool value)
{
int bitsToSet = HasSetsRequiredMembersPopulatedBit;
if (value) bitsToSet |= HasSetsRequiredMembersBit;

return ThreadSafeFlagOperations.Set(ref _bits, bitsToSet);
}
}

/// <summary>
@@ -1407,6 +1421,21 @@ internal override ImmutableArray<string> GetAppliedConditionalSymbols()
}
}

protected override bool HasSetsRequiredMembersImpl
{
get
{
Debug.Assert(MethodKind == MethodKind.Constructor);
if (!_packedFlags.HasSetsRequiredMembersPopulated)
{
var result = _containingType.ContainingPEModule.Module.HasAttribute(_handle, AttributeDescription.SetsRequiredMembersAttribute);
_packedFlags.InitializeSetsRequiredMembersBit(result);
}

return _packedFlags.HasSetsRequiredMembers;
}
}

internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree localTree)
{
throw ExceptionUtilities.Unreachable;
7 changes: 7 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs
Original file line number Diff line number Diff line change
@@ -579,6 +579,13 @@ public bool IsConditional
}
}

/// <summary>
/// Returns true if this is a constructor attributed with HasSetsRequiredMembers
/// </summary>
internal bool HasSetsRequiredMembers => MethodKind == MethodKind.Constructor && HasSetsRequiredMembersImpl;

protected abstract bool HasSetsRequiredMembersImpl { get; }

/// <summary>
/// Some method kinds do not participate in overriding/hiding (e.g. constructors).
/// </summary>
Original file line number Diff line number Diff line change
@@ -588,6 +588,8 @@ public override int GetHashCode()
return _reducedFrom.GetHashCode();
}

protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable;

#nullable enable

private sealed class ReducedExtensionMethodParameterSymbol : WrappedParameterSymbol
Original file line number Diff line number Diff line change
@@ -172,6 +172,8 @@ internal override bool IsMetadataFinal

internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree localTree) { throw ExceptionUtilities.Unreachable; }

protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable;

#endregion
}
}
Original file line number Diff line number Diff line change
@@ -246,5 +246,40 @@ internal sealed override int CalculateLocalSyntaxOffset(int position, SyntaxTree
protected abstract CSharpSyntaxNode GetInitializer();

protected abstract bool IsWithinExpressionOrBlockBody(int position, out int offset);

#nullable enable
protected sealed override bool HasSetsRequiredMembersImpl
=> GetEarlyDecodedWellKnownAttributeData()?.HasSetsRequiredMembersAttribute == true;

internal sealed override (CSharpAttributeData?, BoundAttribute?) EarlyDecodeWellKnownAttribute(ref EarlyDecodeWellKnownAttributeArguments<EarlyWellKnownAttributeBinder, NamedTypeSymbol, AttributeSyntax, AttributeLocation> arguments)
{
if (arguments.SymbolPart == AttributeLocation.None)
{
if (CSharpAttributeData.IsTargetEarlyAttribute(arguments.AttributeType, arguments.AttributeSyntax, AttributeDescription.SetsRequiredMembersAttribute))
{
var earlyData = arguments.GetOrCreateData<MethodEarlyWellKnownAttributeData>();
earlyData.HasSetsRequiredMembersAttribute = true;

if (ContainingType.IsWellKnownSetsRequiredMembersAttribute())
{
// Avoid a binding cycle for this scenario.
return (null, null);
}

var (attributeData, boundAttribute) = arguments.Binder.GetAttribute(arguments.AttributeSyntax, arguments.AttributeType, beforeAttributePartBound: null, afterAttributePartBound: null, out bool hasAnyDiagnostics);

if (!hasAnyDiagnostics)
{
return (attributeData, boundAttribute);
}
else
{
return (null, null);
}
}
}

return base.EarlyDecodeWellKnownAttribute(ref arguments);
}
}
}
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@
using System.Collections.Immutable;
using System.Linq;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
@@ -78,5 +79,7 @@ internal void ReportAsyncParameterErrors(BindingDiagnosticBag diagnostics, Locat
static Location getLocation(ParameterSymbol parameter, Location location)
=> parameter.Locations.FirstOrDefault() ?? location;
}

protected override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable;
}
}
Original file line number Diff line number Diff line change
@@ -318,7 +318,7 @@ public override ImmutableArray<CSharpAttributeData> GetReturnTypeAttributes()
}

#nullable enable
internal sealed override (CSharpAttributeData?, BoundAttribute?) EarlyDecodeWellKnownAttribute(ref EarlyDecodeWellKnownAttributeArguments<EarlyWellKnownAttributeBinder, NamedTypeSymbol, AttributeSyntax, AttributeLocation> arguments)
internal override (CSharpAttributeData?, BoundAttribute?) EarlyDecodeWellKnownAttribute(ref EarlyDecodeWellKnownAttributeArguments<EarlyWellKnownAttributeBinder, NamedTypeSymbol, AttributeSyntax, AttributeLocation> arguments)
{
Debug.Assert(arguments.SymbolPart == AttributeLocation.None || arguments.SymbolPart == AttributeLocation.Return);

3 changes: 1 addition & 2 deletions src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs
Original file line number Diff line number Diff line change
@@ -843,7 +843,6 @@ internal static bool HasAsyncMethodBuilderAttribute(this Symbol symbol, [NotNull
internal static bool IsRequired(this Symbol symbol) => symbol is FieldSymbol { IsRequired: true } or PropertySymbol { IsRequired: true };

internal static bool ShouldCheckRequiredMembers(this MethodSymbol method)
// PROTOTYPE(req): Check for the SetsRequiredMembersAttribute and return false for that case
=> method.MethodKind == MethodKind.Constructor;
=> method.MethodKind == MethodKind.Constructor && !method.HasSetsRequiredMembers;
}
}
Original file line number Diff line number Diff line change
@@ -64,6 +64,11 @@ internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, r
var compilation = this.DeclaringCompilation;
AddSynthesizedAttribute(ref attributes, compilation.TrySynthesizeAttribute(WellKnownMember.System_Runtime_CompilerServices_CompilerGeneratedAttribute__ctor));
Debug.Assert(WellKnownMembers.IsSynthesizedAttributeOptional(WellKnownMember.System_Runtime_CompilerServices_CompilerGeneratedAttribute__ctor));

if (HasSetsRequiredMembersImpl)
{
AddSynthesizedAttribute(ref attributes, compilation.TrySynthesizeAttribute(WellKnownMember.System_Diagnostics_CodeAnalysis_SetsRequiredMembersAttribute__ctor));
}
}

internal static MethodSymbol? FindCopyConstructor(NamedTypeSymbol containingType, NamedTypeSymbol within, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
@@ -127,5 +132,9 @@ internal static bool HasCopyConstructorSignature(MethodSymbol member)
method.Parameters[0].Type.Equals(containingType, TypeCompareKind.AllIgnoreOptions) &&
method.Parameters[0].RefKind == RefKind.None;
}

protected sealed override bool HasSetsRequiredMembersImpl
// If the record type has a required members error, then it does have required members of some kind, we emit the SetsRequiredMembers attribute.
=> !ContainingType.AllRequiredMembers.IsEmpty || ContainingType.HasRequiredMembersError;
}
}
Original file line number Diff line number Diff line change
@@ -238,5 +238,7 @@ public override bool IsExtern
{
get { return false; }
}

protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable;
}
}
Original file line number Diff line number Diff line change
@@ -307,6 +307,8 @@ private static BoundCall CreateParameterlessCall(CSharpSyntaxNode syntax, BoundE
{ WasCompilerGenerated = true };
}

protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable;

/// <summary> A synthesized entrypoint that forwards all calls to an async Main Method </summary>
internal sealed class AsyncForwardEntryPoint : SynthesizedEntryPointSymbol
{
Original file line number Diff line number Diff line change
@@ -335,5 +335,7 @@ internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree l
}

internal sealed override bool IsNullableAnalysisEnabled() => false;

protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable;
}
}
Original file line number Diff line number Diff line change
@@ -265,5 +265,7 @@ internal override ImmutableArray<string> GetAppliedConditionalSymbols()
{
return ImmutableArray<string>.Empty;
}

protected sealed override bool HasSetsRequiredMembersImpl => throw ExceptionUtilities.Unreachable;
}
}
Original file line number Diff line number Diff line change
@@ -316,5 +316,6 @@ internal virtual void GenerateMethodBodyStatements(SyntheticBoundNodeFactory fac
// overridden in a derived class to add extra statements to the body of the generated constructor
}

protected override bool HasSetsRequiredMembersImpl => false;
}
}
Loading

0 comments on commit 7761d6b

Please sign in to comment.