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 SetsRequiredMembersAttribute #60392

Merged
merged 14 commits into from
Apr 15, 2022
Merged
45 changes: 26 additions & 19 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ internal override bool HasSpecialName
{
get { return true; }
}

protected override bool HasSetsRequiredMembersImpl => false;
}

private sealed partial class AnonymousTypePropertyGetAccessorSymbol : SynthesizedMethodBase
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2142,7 +2142,9 @@ private static BoundCall GenerateBaseCopyConstructorInitializer(SynthesizedRecor
return null;
}

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

return;

ImmutableArray<Symbol> getMembersNeedingDefaultInitialState()
{
if (_hasInitialState)
Expand All @@ -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).
333fred marked this conversation as resolved.
Show resolved Hide resolved
return membersToBeInitialized(method.ContainingType, includeAllMembers: method.IncludeFieldInitializersInBody());

static ImmutableArray<Symbol> membersToBeInitialized(NamedTypeSymbol containingType, bool includeAllMembers)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -231,6 +231,8 @@ internal sealed override int CalculateLocalSyntaxOffset(int localPosition, Synta
{
throw ExceptionUtilities.Unreachable;
}

protected 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
Expand Up @@ -6,6 +6,7 @@

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

namespace Microsoft.CodeAnalysis.CSharp.Symbols
Expand Down Expand Up @@ -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
Expand Up @@ -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
Expand Up @@ -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.
Expand All @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -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)
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
{
if (CSharpAttributeData.IsTargetEarlyAttribute(arguments.AttributeType, arguments.AttributeSyntax, AttributeDescription.SetsRequiredMembersAttribute))
{
var earlyData = arguments.GetOrCreateData<MethodEarlyWellKnownAttributeData>();
earlyData.HasSetsRequiredMembersAttribute = true;

if (ContainingType.IsWellKnownSetsRequiredMembersAttribute())
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
{
// 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)
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
{
return (attributeData, boundAttribute);
}
else
{
return (null, null);
}
}
}

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

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
Expand Down Expand Up @@ -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
Expand Up @@ -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);

Expand Down
3 changes: 1 addition & 2 deletions src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 is { MethodKind: MethodKind.Constructor, HasSetsRequiredMembers: false };
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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));
333fred marked this conversation as resolved.
Show resolved Hide resolved
}
}

internal static MethodSymbol? FindCopyConstructor(NamedTypeSymbol containingType, NamedTypeSymbol within, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
Expand Down Expand Up @@ -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.
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
=> !ContainingType.AllRequiredMembers.IsEmpty || ContainingType.HasRequiredMembersError;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -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
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -265,5 +265,7 @@ internal override ImmutableArray<string> GetAppliedConditionalSymbols()
{
return ImmutableArray<string>.Empty;
}

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