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
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 33 additions & 19 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
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
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
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>
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
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 @@ -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
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
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