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

Merge features/struct-ctors into main branch #55042

Merged
merged 7 commits into from
Jul 22, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -5326,7 +5326,7 @@ protected BoundExpression BindClassCreationExpression(
ReportDiagnosticsIfObsolete(diagnostics, method, node, hasBaseReceiver: false);
// NOTE: Use-site diagnostics were reported during overload resolution.

ConstantValue constantValueOpt = (initializerSyntaxOpt == null && method.IsDefaultValueTypeConstructor()) ?
ConstantValue constantValueOpt = (initializerSyntaxOpt == null && method.IsDefaultValueTypeConstructor(requireZeroInit: true)) ?
FoldParameterlessValueTypeConstructor(type) :
null;

Expand Down
23 changes: 16 additions & 7 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3306,7 +3306,7 @@ private BindValueKind GetRequiredReturnValueKind(RefKind refKind)
return requiredValueKind;
}

public virtual BoundNode BindMethodBody(CSharpSyntaxNode syntax, BindingDiagnosticBag diagnostics)
public virtual BoundNode BindMethodBody(CSharpSyntaxNode syntax, BindingDiagnosticBag diagnostics, bool includesFieldInitializers = false)
{
switch (syntax)
{
Expand All @@ -3316,7 +3316,7 @@ public virtual BoundNode BindMethodBody(CSharpSyntaxNode syntax, BindingDiagnost
case BaseMethodDeclarationSyntax method:
if (method.Kind() == SyntaxKind.ConstructorDeclaration)
{
return BindConstructorBody((ConstructorDeclarationSyntax)method, diagnostics);
return BindConstructorBody((ConstructorDeclarationSyntax)method, diagnostics, includesFieldInitializers);
}

return BindMethodBody(method, method.Body, method.ExpressionBody, diagnostics);
Expand Down Expand Up @@ -3388,32 +3388,41 @@ internal virtual BoundExpressionStatement BindConstructorInitializer(PrimaryCons
return constructorInitializer;
}

private BoundNode BindConstructorBody(ConstructorDeclarationSyntax constructor, BindingDiagnosticBag diagnostics)
private BoundNode BindConstructorBody(ConstructorDeclarationSyntax constructor, BindingDiagnosticBag diagnostics, bool includesFieldInitializers)
{
if (constructor.Initializer == null && constructor.Body == null && constructor.ExpressionBody == null)
ConstructorInitializerSyntax initializer = constructor.Initializer;
if (initializer == null && constructor.Body == null && constructor.ExpressionBody == null)
{
return null;
}

Binder bodyBinder = this.GetBinder(constructor);
Debug.Assert(bodyBinder != null);

if (constructor.Initializer?.IsKind(SyntaxKind.ThisConstructorInitializer) != true &&
bool thisInitializer = initializer?.IsKind(SyntaxKind.ThisConstructorInitializer) == true;
if (!thisInitializer &&
ContainingType.GetMembersUnordered().OfType<SynthesizedRecordConstructor>().Any())
{
var constructorSymbol = (MethodSymbol)this.ContainingMember();
if (!constructorSymbol.IsStatic &&
!SynthesizedRecordCopyCtor.IsCopyConstructor(constructorSymbol))
{
// Note: we check the constructor initializer of copy constructors elsewhere
Error(diagnostics, ErrorCode.ERR_UnexpectedOrMissingConstructorInitializerInRecord, constructor.Initializer?.ThisOrBaseKeyword ?? constructor.Identifier);
Error(diagnostics, ErrorCode.ERR_UnexpectedOrMissingConstructorInitializerInRecord, initializer?.ThisOrBaseKeyword ?? constructor.Identifier);
}
}

// The `: this()` initializer is ignored when it is a default value type constructor
// and we need to include field initializers into the constructor.
bool skipInitializer = includesFieldInitializers
&& thisInitializer
&& ContainingType.IsDefaultValueTypeConstructor(initializer);

// Using BindStatement to bind block to make sure we are reusing results of partial binding in SemanticModel
return new BoundConstructorMethodBody(constructor,
bodyBinder.GetDeclaredLocalsForScope(constructor),
constructor.Initializer == null ? null : bodyBinder.BindConstructorInitializer(constructor.Initializer, diagnostics),
skipInitializer ? new BoundNoOpStatement(constructor, NoOpStatementFlavor.Default)
: initializer == null ? null : bodyBinder.BindConstructorInitializer(initializer, diagnostics),
constructor.Body == null ? null : (BoundBlock)bodyBinder.BindStatement(constructor.Body, diagnostics),
constructor.ExpressionBody == null ?
null :
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2185,7 +2185,7 @@

<Node Name="BoundConstructorMethodBody" Base="BoundMethodBodyBase">
<Field Name="Locals" Type="ImmutableArray&lt;LocalSymbol&gt;"/>
<Field Name="Initializer" Type="BoundExpressionStatement?"/>
<Field Name="Initializer" Type="BoundStatement?"/>
</Node>

<!-- Node only used during nullability flow analysis to represent an expression with an updated nullability -->
Expand Down
15 changes: 9 additions & 6 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1691,9 +1691,6 @@ If such a class is used as a base class and if the deriving class defines a dest
<data name="ERR_InterfacesCantContainConversionOrEqualityOperators" xml:space="preserve">
<value>Conversion, equality, or inequality operators declared in interfaces must be abstract</value>
</data>
<data name="ERR_StructsCantContainDefaultConstructor" xml:space="preserve">
<value>Structs cannot contain explicit parameterless constructors</value>
</data>
<data name="ERR_EnumsCantContainDefaultConstructor" xml:space="preserve">
<value>Enums cannot contain explicit parameterless constructors</value>
</data>
Expand All @@ -1709,9 +1706,6 @@ If such a class is used as a base class and if the deriving class defines a dest
<data name="ERR_BadTypeReference" xml:space="preserve">
<value>'{0}': cannot reference a type through an expression; try '{1}' instead</value>
</data>
<data name="ERR_FieldInitializerInStruct" xml:space="preserve">
<value>'{0}': cannot have instance property or field initializers in structs</value>
</data>
<data name="ERR_BadDestructorName" xml:space="preserve">
<value>Name of destructor must match name of type</value>
</data>
Expand Down Expand Up @@ -6597,6 +6591,12 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="IDS_FeaturePositionalFieldsInRecords" xml:space="preserve">
<value>positional fields in records</value>
</data>
<data name="IDS_FeatureParameterlessStructConstructors" xml:space="preserve">
<value>parameterless struct constructors</value>
</data>
<data name="IDS_FeatureStructFieldInitializers" xml:space="preserve">
<value>struct field initializers</value>
</data>
<data name="IDS_FeatureVarianceSafetyForStaticInterfaceMembers" xml:space="preserve">
<value>variance safety for static interface members</value>
</data>
Expand Down Expand Up @@ -6736,6 +6736,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_InterpolatedStringHandlerCreationCannotUseDynamic" xml:space="preserve">
<value>An interpolated string handler construction cannot use dynamic. Manually construct an instance of '{0}'.</value>
</data>
<data name="ERR_NonPublicParameterlessStructConstructor" xml:space="preserve">
<value>The parameterless struct constructor must be 'public'.</value>
</data>
<data name="IDS_FeatureStaticAbstractMembersInInterfaces" xml:space="preserve">
<value>static abstract members in interfaces</value>
</data>
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1481,7 +1481,7 @@ private enum CallKind

private void EmitCallExpression(BoundCall call, UseKind useKind)
{
if (call.Method.IsDefaultValueTypeConstructor())
if (call.Method.IsDefaultValueTypeConstructor(requireZeroInit: true))
{
EmitDefaultValueTypeConstructorCallExpression(call);
}
Expand Down Expand Up @@ -1956,7 +1956,7 @@ private void EmitConvertedStackAllocExpression(BoundConvertedStackAllocExpressio
private void EmitObjectCreationExpression(BoundObjectCreationExpression expression, bool used)
{
MethodSymbol constructor = expression.Constructor;
if (constructor.IsDefaultValueTypeConstructor())
if (constructor.IsDefaultValueTypeConstructor(requireZeroInit: true))
{
EmitInitObj(expression.Type, used, expression.Syntax);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2462,7 +2462,7 @@ private BoundNode TryGetBoundNodeFromMap(CSharpSyntaxNode node)
return null;
}

public override BoundNode BindMethodBody(CSharpSyntaxNode node, BindingDiagnosticBag diagnostics)
public override BoundNode BindMethodBody(CSharpSyntaxNode node, BindingDiagnosticBag diagnostics, bool includeInitializersInBody)
{
BoundNode boundNode = TryGetBoundNodeFromMap(node);

Expand All @@ -2471,7 +2471,7 @@ public override BoundNode BindMethodBody(CSharpSyntaxNode node, BindingDiagnosti
return boundNode;
}

boundNode = base.BindMethodBody(node, diagnostics);
boundNode = base.BindMethodBody(node, diagnostics, includeInitializersInBody);

return boundNode;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ internal readonly struct InitialState
{
internal readonly CSharpSyntaxNode Syntax;
internal readonly BoundNode? Body;
internal readonly ExecutableCodeBinder? Binder;
internal readonly Binder? Binder;
internal readonly NullableWalker.SnapshotManager? SnapshotManager;
internal readonly ImmutableDictionary<Symbol, Symbol>? RemappedSymbols;

internal InitialState(
CSharpSyntaxNode syntax,
BoundNode? bodyOpt = null,
ExecutableCodeBinder? binder = null,
Binder? binder = null,
NullableWalker.SnapshotManager? snapshotManager = null,
ImmutableDictionary<Symbol, Symbol>? remappedSymbols = null)
{
Expand Down
33 changes: 22 additions & 11 deletions src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -965,7 +965,7 @@ private void CompileMethod(
}

// no need to emit the default ctor, we are not emitting those
if (methodSymbol.IsDefaultValueTypeConstructor())
if (methodSymbol.IsDefaultValueTypeConstructor(requireZeroInit: true))
{
return;
}
Expand Down Expand Up @@ -1036,7 +1036,11 @@ private void CompileMethod(
getFinalNullableState: true,
out processedInitializers.AfterInitializersState);
}
body = BindMethodBody(methodSymbol, compilationState, diagsForCurrentMethod, processedInitializers.AfterInitializersState, out importChain, out originalBodyNested, out forSemanticModel);

body = BindMethodBody(methodSymbol, compilationState, diagsForCurrentMethod, processedInitializers.AfterInitializersState,
includeInitializersInBody && !processedInitializers.BoundInitializers.IsEmpty,
out importChain, out originalBodyNested, out forSemanticModel);

if (diagsForCurrentMethod.HasAnyErrors() && body != null)
{
body = (BoundBlock)body.WithHasErrors();
Expand Down Expand Up @@ -1194,7 +1198,7 @@ forSemanticModel.Syntax is { } semanticModelSyntax &&
if (!hasErrors && (hasBody || includeNonEmptyInitializersInBody))
{
Debug.Assert(!(methodSymbol.IsImplicitInstanceConstructor && methodSymbol.ParameterCount == 0) ||
!methodSymbol.ContainingType.IsStructType());
!methodSymbol.IsDefaultValueTypeConstructor(requireZeroInit: true));

// Fields must be initialized before constructor initializer (which is the first statement of the analyzed body, if specified),
// so that the initialization occurs before any method overridden by the declaring class can be invoked from the base constructor
Expand Down Expand Up @@ -1673,12 +1677,13 @@ private static void GetStateMachineSlotDebugInfo(
// NOTE: can return null if the method has no body.
internal static BoundBlock BindMethodBody(MethodSymbol method, TypeCompilationState compilationState, BindingDiagnosticBag diagnostics)
{
return BindMethodBody(method, compilationState, diagnostics, nullableInitialState: null, out _, out _, out _);
return BindMethodBody(method, compilationState, diagnostics, nullableInitialState: null, includesFieldInitializers: false, out _, out _, out _);
}

// NOTE: can return null if the method has no body.
private static BoundBlock BindMethodBody(MethodSymbol method, TypeCompilationState compilationState, BindingDiagnosticBag diagnostics,
NullableWalker.VariableState nullableInitialState,
bool includesFieldInitializers,
out ImportChain importChain, out bool originalBodyNested,
out MethodBodySemanticModel.InitialState forSemanticModel)
{
Expand Down Expand Up @@ -1707,18 +1712,17 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax &&
constructorSyntax.Identifier.ValueText);
}

ExecutableCodeBinder bodyBinder = sourceMethod.TryGetBodyBinder();

if (sourceMethod.IsExtern || sourceMethod.IsDefaultValueTypeConstructor())
Debug.Assert(!sourceMethod.IsDefaultValueTypeConstructor(requireZeroInit: false));
if (sourceMethod.IsExtern)
{
return null;
}

Binder bodyBinder = sourceMethod.TryGetBodyBinder();
if (bodyBinder != null)
{
importChain = bodyBinder.ImportChain;

BoundNode methodBody = bodyBinder.BindMethodBody(syntaxNode, diagnostics);
BoundNode methodBody = bodyBinder.BindMethodBody(syntaxNode, diagnostics, includesFieldInitializers);
BoundNode methodBodyForSemanticModel = methodBody;
NullableWalker.SnapshotManager snapshotManager = null;
ImmutableDictionary<Symbol, Symbol> remappedSymbols = null;
Expand Down Expand Up @@ -1760,9 +1764,15 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax &&
var constructor = (BoundConstructorMethodBody)methodBody;
body = constructor.BlockBody ?? constructor.ExpressionBody;

if (constructor.Initializer != null)
if (constructor.Initializer is BoundNoOpStatement)
{
// We have field initializers and `: this()` is a default value type constructor.
Debug.Assert(body is not null);
return body;
}
else if (constructor.Initializer is BoundExpressionStatement expressionStatement)
{
ReportCtorInitializerCycles(method, constructor.Initializer.Expression, compilationState, diagnostics);
ReportCtorInitializerCycles(method, expressionStatement.Expression, compilationState, diagnostics);

if (body == null)
{
Expand All @@ -1778,6 +1788,7 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax &&
}
else
{
Debug.Assert(constructor.Initializer is null);
Debug.Assert(constructor.Locals.IsEmpty);
}
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ internal abstract partial class CSharpAttributeData : Cci.ICustomAttribute

Cci.IMethodReference Cci.ICustomAttribute.Constructor(EmitContext context, bool reportDiagnostics)
{
if (this.AttributeConstructor.IsDefaultValueTypeConstructor())
if (this.AttributeConstructor.IsDefaultValueTypeConstructor(requireZeroInit: true))
{
// Parameter constructors for structs exist in symbol table, but are not emitted.
// Produce an error since we cannot use it (instead of crashing):
// Default parameterless constructors for structs exist in symbol table, but are not emitted.
// Produce an error since we cannot use it (instead of crashing).
// Details: https://github.com/dotnet/roslyn/issues/19394

if (reportDiagnostics)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1143,7 +1143,7 @@ internal Cci.IMethodReference Translate(
BoundArgListOperator optArgList = null,
bool needDeclaration = false)
{
Debug.Assert(!methodSymbol.IsDefaultValueTypeConstructor());
Debug.Assert(!methodSymbol.IsDefaultValueTypeConstructor(requireZeroInit: true));
Debug.Assert(optArgList == null || (methodSymbol.IsVararg && !needDeclaration));

Cci.IMethodReference unexpandedMethodRef = Translate(methodSymbol, syntaxNodeOpt, diagnostics, needDeclaration);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ internal override EmbeddedMethod EmbedMethod(
DiagnosticBag diagnostics)
{
Debug.Assert(method.AdaptedSymbol.IsDefinition);
Debug.Assert(!method.AdaptedMethodSymbol.IsDefaultValueTypeConstructor());
Debug.Assert(!method.AdaptedMethodSymbol.IsDefaultValueTypeConstructor(requireZeroInit: false));

EmbeddedMethod embedded = new EmbeddedMethod(type, method);
EmbeddedMethod cached = EmbeddedMethodsMap.GetOrAdd(method, embedded);
Expand Down
5 changes: 3 additions & 2 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -384,12 +384,12 @@ internal enum ErrorCode
ERR_BadBinaryOperatorSignature = 563,
ERR_BadShiftOperatorSignature = 564,
ERR_InterfacesCantContainConversionOrEqualityOperators = 567,
ERR_StructsCantContainDefaultConstructor = 568,
//ERR_StructsCantContainDefaultConstructor = 568,
ERR_CantOverrideBogusMethod = 569,
ERR_BindToBogus = 570,
ERR_CantCallSpecialMethod = 571,
ERR_BadTypeReference = 572,
ERR_FieldInitializerInStruct = 573,
//ERR_FieldInitializerInStruct = 573,
ERR_BadDestructorName = 574,
ERR_OnlyClassesCanContainDestructors = 575,
ERR_ConflictAliasAndMember = 576,
Expand Down Expand Up @@ -1981,6 +1981,7 @@ internal enum ErrorCode
ERR_FileScopedAndNormalNamespace = 8955,
ERR_FileScopedNamespaceNotBeforeAllMembers = 8956,
ERR_NoImplicitConvTargetTypedConditional = 8957,
ERR_NonPublicParameterlessStructConstructor = 8958,

#endregion

Expand Down
Loading