Skip to content

Commit

Permalink
Support SetsRequiredMembersAttribute (#60392)
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: #57046
  • Loading branch information
@333fred
333fred authored Apr 15, 2022
1 parent 77dba9e commit 53ce8d4
Show file tree
Hide file tree
Showing 59 changed files with 1,716 additions and 512 deletions.
52 changes: 33 additions & 19 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4123,6 +4123,13 @@ private BoundExpression BindConstructorInitializerCore(
diagnostics);
}

if (resultMember.HasSetsRequiredMembers && !constructor.HasSetsRequiredMembers)
{
hasErrors = true;
// This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute.
diagnostics.Add(ErrorCode.ERR_ChainingToSetsRequiredMembersRequiresSetsRequiredMembers, errorLocation);
}

return new BoundCall(
nonNullSyntax,
receiver,
Expand Down Expand Up @@ -5854,25 +5861,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 @@ -5929,6 +5918,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
1 change: 0 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Initializers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ internal struct ProcessedFieldInitializers
{
internal ImmutableArray<BoundInitializer> BoundInitializers { get; set; }
internal BoundStatement? LoweredInitializers { get; set; }
internal NullableWalker.VariableState AfterInitializersState;
internal bool HasErrors { get; set; }
internal ImportChain? FirstImportChain { get; set; }
}
Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7121,4 +7121,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_ExpressionTreeContainsUTF8StringLiterals" xml:space="preserve">
<value>An expression tree may not contain UTF8 string conversion or literal.</value>
</data>
<data name="ERR_ChainingToSetsRequiredMembersRequiresSetsRequiredMembers" xml:space="preserve">
<value>This constructor must add 'SetsRequiredMembers' because it chains to a constructor that has that attribute.</value>
</data>
</root>
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Compilation/MethodBodySemanticModel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ protected override BoundNode RewriteNullableBoundNodesWithSnapshots(
out NullableWalker.SnapshotManager snapshotManager,
ref ImmutableDictionary<Symbol, Symbol> remappedSymbols)
{
var afterInitializersState = NullableWalker.GetAfterInitializersState(Compilation, MemberSymbol);
var afterInitializersState = NullableWalker.GetAfterInitializersState(Compilation, MemberSymbol, boundRoot);
return NullableWalker.AnalyzeAndRewrite(Compilation, MemberSymbol, boundRoot, binder, afterInitializersState, diagnostics, createSnapshots, out snapshotManager, ref remappedSymbols);
}

Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Compiler/AnonymousTypeMethodBodySynthesizer.cs
View file
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
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Compiler/MethodBodySynthesizer.Lowered.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,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
87 changes: 55 additions & 32 deletions src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,7 @@ private void CompileNamedType(NamedTypeSymbol containingType)
useConstructorExitWarnings: true,
initialNullableState: null,
getFinalNullableState: false,
baseOrThisInitializer: null,
finalNullableState: out _);
}
}
Expand Down Expand Up @@ -1011,7 +1012,8 @@ private void CompileMethod(
useConstructorExitWarnings: false,
initialNullableState: null,
getFinalNullableState: true,
out processedInitializers.AfterInitializersState);
baseOrThisInitializer: null,
out _);
}

var unusedDiagnostics = DiagnosticBag.GetInstance();
Expand All @@ -1031,28 +1033,12 @@ private void CompileMethod(
processedInitializers.HasErrors = processedInitializers.HasErrors || analyzedInitializers.HasAnyErrors;
}

if (includeInitializersInBody &&
processedInitializers.AfterInitializersState is null &&
ReportNullableDiagnostics)
{
NullableWalker.AnalyzeIfNeeded(
_compilation,
methodSymbol,
// we analyze to produce an AfterInitializersState even if there are no initializers
// because it conveniently allows us to capture all the 'default' states for applicable members
analyzedInitializers ?? GetSynthesizedEmptyBody(methodSymbol),
diagsForCurrentMethod.DiagnosticBag,
useConstructorExitWarnings: false,
initialNullableState: null,
getFinalNullableState: true,
out processedInitializers.AfterInitializersState);
}

body = BindMethodBody(
methodSymbol,
compilationState,
diagsForCurrentMethod,
processedInitializers.AfterInitializersState,
includeInitializersInBody,
analyzedInitializers,
ReportNullableDiagnostics,
out importChain,
out originalBodyNested,
Expand Down Expand Up @@ -1711,19 +1697,31 @@ private static void GetStateMachineSlotDebugInfo(
}

// NOTE: can return null if the method has no body.
internal static BoundBlock BindMethodBody(MethodSymbol method, TypeCompilationState compilationState, BindingDiagnosticBag diagnostics)
#nullable enable
internal static BoundBlock? BindMethodBody(MethodSymbol method, TypeCompilationState compilationState, BindingDiagnosticBag diagnostics)
{
return BindMethodBody(method, compilationState, diagnostics, nullableInitialState: null, reportNullableDiagnostics: true, out _, out _, out _, out _);
return BindMethodBody(
method,
compilationState,
diagnostics,
includeInitializersInBody: false,
initializersBody: null,
reportNullableDiagnostics: true,
importChain: out _,
originalBodyNested: out _,
prependedDefaultValueTypeConstructorInitializer: out _,
forSemanticModel: out _);
}

// NOTE: can return null if the method has no body.
private static BoundBlock BindMethodBody(
private static BoundBlock? BindMethodBody(
MethodSymbol method,
TypeCompilationState compilationState,
BindingDiagnosticBag diagnostics,
NullableWalker.VariableState nullableInitialState,
bool includeInitializersInBody,
BoundNode? initializersBody,
bool reportNullableDiagnostics,
out ImportChain importChain,
out ImportChain? importChain,
out bool originalBodyNested,
out bool prependedDefaultValueTypeConstructorInitializer,
out MethodBodySemanticModel.InitialState forSemanticModel)
Expand All @@ -1733,11 +1731,15 @@ private static BoundBlock BindMethodBody(
importChain = null;
forSemanticModel = default;

BoundBlock body;
BoundBlock? body;
NullableWalker.VariableState? nullableInitialState = null;

initializersBody ??= GetSynthesizedEmptyBody(method);

if (method is SynthesizedRecordConstructor recordStructPrimaryCtor && method.ContainingType.IsRecordStruct)
{
body = BoundBlock.SynthesizedNoLocals(recordStructPrimaryCtor.GetSyntax());
nullableInitialState = getInitializerState(body);
}
else if (method is SourceMemberMethodSymbol sourceMethod)
{
Expand Down Expand Up @@ -1766,12 +1768,15 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax &&
importChain = bodyBinder.ImportChain;
BoundNode methodBody = bodyBinder.BindMethodBody(syntaxNode, diagnostics);
BoundNode methodBodyForSemanticModel = methodBody;
NullableWalker.SnapshotManager snapshotManager = null;
ImmutableDictionary<Symbol, Symbol> remappedSymbols = null;
NullableWalker.SnapshotManager? snapshotManager = null;
ImmutableDictionary<Symbol, Symbol>? remappedSymbols = null;
var compilation = bodyBinder.Compilation;

nullableInitialState = getInitializerState(methodBody);

if (reportNullableDiagnostics)
{
Debug.Assert(diagnostics.DiagnosticBag != null);
if (compilation.IsNullableAnalysisEnabledIn(method))
{
var isSufficientLangVersion = compilation.LanguageVersion >= MessageID.IDS_FeatureNullableReferenceTypes.RequiredVersion();
Expand Down Expand Up @@ -1799,17 +1804,17 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax &&
useConstructorExitWarnings: true,
nullableInitialState,
getFinalNullableState: false,
baseOrThisInitializer: null,
finalNullableState: out _);
}
}

forSemanticModel = new MethodBodySemanticModel.InitialState(syntaxNode, methodBodyForSemanticModel, bodyBinder, snapshotManager, remappedSymbols);

switch (methodBody.Kind)
{
case BoundKind.ConstructorMethodBody:
var constructor = (BoundConstructorMethodBody)methodBody;
body = constructor.BlockBody ?? constructor.ExpressionBody;
body = constructor.BlockBody ?? constructor.ExpressionBody!;

if (constructor.Initializer is BoundExpressionStatement expressionStatement)
{
Expand Down Expand Up @@ -1838,7 +1843,8 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax &&

case BoundKind.NonConstructorMethodBody:
var nonConstructor = (BoundNonConstructorMethodBody)methodBody;
body = nonConstructor.BlockBody ?? nonConstructor.ExpressionBody;
body = nonConstructor.BlockBody ?? nonConstructor.ExpressionBody!;
Debug.Assert(body != null);
break;

case BoundKind.Block:
Expand All @@ -1851,7 +1857,7 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax &&
else
{
var property = sourceMethod.AssociatedSymbol as SourcePropertySymbolBase;
if ((object)property != null && property.IsAutoPropertyWithGetAccessor)
if (property is not null && property.IsAutoPropertyWithGetAccessor)
{
return MethodBodySynthesizer.ConstructAutoPropertyAccessorBody(sourceMethod);
}
Expand All @@ -1868,15 +1874,18 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax &&
var stmts = ArrayBuilder<BoundStatement>.GetInstance();
ctor.GenerateMethodBodyStatements(factory, stmts, diagnostics);
body = BoundBlock.SynthesizedNoLocals(node, stmts.ToImmutableAndFree());
nullableInitialState = getInitializerState(body);
}
else
{
// synthesized methods should return their bound bodies
body = null;
nullableInitialState = getInitializerState(null);
}

if (reportNullableDiagnostics && method.IsConstructor() && method.IsImplicitlyDeclared && nullableInitialState is object)
{
Debug.Assert(diagnostics.AccumulatesDiagnostics);
NullableWalker.AnalyzeIfNeeded(
compilationState.Compilation,
method,
Expand All @@ -1885,6 +1894,7 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax &&
useConstructorExitWarnings: true,
nullableInitialState,
getFinalNullableState: false,
baseOrThisInitializer: null,
finalNullableState: out _);
}

Expand Down Expand Up @@ -1916,7 +1926,18 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax &&
}

return BoundBlock.SynthesizedNoLocals(method.GetNonNullSyntaxNode(), statements);

NullableWalker.VariableState? getInitializerState(BoundNode? body)
{
if (reportNullableDiagnostics && includeInitializersInBody)
{
return NullableWalker.GetAfterInitializersState(compilationState.Compilation, method, initializersBody, body, diagnostics);
}

return null;
}
}
#nullable disable

private static BoundBlock GetSynthesizedEmptyBody(Symbol symbol)
{
Expand Down Expand Up @@ -2148,7 +2169,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
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2085,5 +2085,6 @@ internal enum ErrorCode
ERR_RequiredMembersMustBeAssignedValue = 9507,
ERR_RequiredMembersInvalid = 9508,
ERR_RequiredMembersBaseTypeInvalid = 9509,
ERR_ChainingToSetsRequiredMembersRequiresSetsRequiredMembers = 9510,
}
}
Loading

0 comments on commit 53ce8d4

Please sign in to comment.