Parameterless struct constructors: Remaining work items

src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs

@@ -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)
             {
@@ -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);
@@ -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 :

src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml

@@ -2174,7 +2174,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 -->

src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs

@@ -2456,7 +2456,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);
 
@@ -2465,7 +2465,7 @@ public override BoundNode BindMethodBody(CSharpSyntaxNode node, BindingDiagnosti
                     return boundNode;
                 }
 
-                boundNode = base.BindMethodBody(node, diagnostics);
+                boundNode = base.BindMethodBody(node, diagnostics, includeInitializersInBody);
 
                 return boundNode;
             }

src/Compilers/CSharp/Portable/Compilation/MethodBodySemanticModel.cs

@@ -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)
             {

src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs

@@ -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();
@@ -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)
         {
@@ -1713,12 +1718,11 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax &&
                     return null;
                 }
 
-                ExecutableCodeBinder bodyBinder = sourceMethod.TryGetBodyBinder();
+                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;
@@ -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)
                                 {
@@ -1778,6 +1788,7 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax &&
                             }
                             else
                             {
+                                Debug.Assert(constructor.Initializer is null);
                                 Debug.Assert(constructor.Locals.IsEmpty);
                             }
                             break;

src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs

@@ -442,9 +442,6 @@ protected ImmutableArray<PendingBranch> Analyze(ref bool badRegion, Optional<TLo
                 PendingBranches.Clear();
                 this.stateChangedAfterUse = false;
                 this.Diagnostics.Clear();
-                // PROTOTYPE: NullableWalker.Scan() will call EnterParameter(methodThisParameter), and
-                // if 'this' is a struct and we have initial state above, then EnterParameter() will overwrite
-                // fields of 'this'. In short, we're overwriting the state from field initializers.
                 returns = this.Scan(ref badRegion);
             }
             while (this.stateChangedAfterUse);

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

@@ -2381,20 +2381,24 @@ private void EnterParameters()
         private void EnterParameter(ParameterSymbol parameter, TypeWithAnnotations parameterType)
         {
             _variables.SetType(parameter, parameterType);
-            int slot = GetOrCreateSlot(parameter);
 
-            Debug.Assert(!IsConditionalState);
-            if (slot > 0)
+            if (parameter.RefKind != RefKind.Out)
             {
-                var state = GetParameterState(parameterType, parameter.FlowAnalysisAnnotations).State;
-                this.State[slot] = state;
-                if (EmptyStructTypeCache.IsTrackableStructType(parameterType.Type))
+                int slot = GetOrCreateSlot(parameter);
+
+                Debug.Assert(!IsConditionalState);
+                if (slot > 0)
                 {
-                    InheritNullableStateOfTrackableStruct(
-                        parameterType.Type,
-                        slot,
-                        valueSlot: -1,
-                        isDefaultValue: parameter.ExplicitDefaultConstantValue?.IsNull == true);
+                    var state = GetParameterState(parameterType, parameter.FlowAnalysisAnnotations).State;
+                    this.State[slot] = state;
+                    if (EmptyStructTypeCache.IsTrackableStructType(parameterType.Type))
+                    {
+                        InheritNullableStateOfTrackableStruct(
+                            parameterType.Type,
+                            slot,
+                            valueSlot: -1,
+                            isDefaultValue: parameter.ExplicitDefaultConstantValue?.IsNull == true);
+                    }
                 }
             }
         }
@@ -3071,7 +3075,6 @@ private void VisitObjectOrDynamicObjectCreation(
                 {
                     var boundObjectCreationExpression = node as BoundObjectCreationExpression;
                     var constructor = boundObjectCreationExpression?.Constructor;
-                    // PROTOTYPE: Test with structs with parameterless constructors.
                     bool isDefaultValueTypeConstructor = constructor?.IsDefaultValueTypeConstructor(requireZeroInit: true) == true;
 
                     if (EmptyStructTypeCache.IsTrackableStructType(type))

src/Compilers/CSharp/Portable/Symbols/MemberSymbolExtensions.cs

@@ -392,22 +392,23 @@ internal static bool HasThisConstructorInitializer(this MethodSymbol method, out
 
         internal static bool IncludeFieldInitializersInBody(this MethodSymbol methodSymbol)
         {
+            // A struct constructor that calls ": this()" will need to include field initializers if the
+            // parameterless constructor is a synthesized default constructor that is not emitted.
+
             return methodSymbol.IsConstructor()
-                && !(methodSymbol.HasThisConstructorInitializer(out var initializerSyntax) && !isDefaultValueTypeConstructor(methodSymbol.ContainingType, initializerSyntax))
+                && !(methodSymbol.HasThisConstructorInitializer(out var initializerSyntax) && !methodSymbol.ContainingType.IsDefaultValueTypeConstructor(initializerSyntax))
                 && !(methodSymbol is SynthesizedRecordCopyCtor) // A record copy constructor is special, regular initializers are not supposed to be executed by it.
                 && !Binder.IsUserDefinedRecordCopyConstructor(methodSymbol);
+        }
 
-            // A struct constructor that calls ": this()" will need to include field initializers if the
-            // parameterless constructor is a synthesized default constructor that is not emitted.
-            static bool isDefaultValueTypeConstructor(NamedTypeSymbol containingType, ConstructorInitializerSyntax initializerSyntax)
+        internal static bool IsDefaultValueTypeConstructor(this NamedTypeSymbol type, ConstructorInitializerSyntax initializerSyntax)
+        {
+            if (initializerSyntax.ArgumentList.Arguments.Count > 0)
             {
-                if (initializerSyntax.ArgumentList.Arguments.Count > 0)
-                {
-                    return false;
-                }
-                var constructor = containingType.InstanceConstructors.SingleOrDefault(m => m.ParameterCount == 0);
-                return constructor?.IsDefaultValueTypeConstructor(requireZeroInit: true) == true;
+                return false;
             }
+            var constructor = type.InstanceConstructors.SingleOrDefault(m => m.ParameterCount == 0);
+            return constructor?.IsDefaultValueTypeConstructor(requireZeroInit: true) == true;
         }
 
         /// <summary>

src/Compilers/CSharp/Test/Semantic/Semantics/NullableContextTests.cs

@@ -961,11 +961,11 @@ struct S
     object F1;
     S(object obj) : this() { }
 }";
-            // PROTOTYPE: NullableWalker.Scan() overwrites initial field state when calling EnterParameter(methodThisParameter).
-            verify(source, expectedAnalyzedKeys: new[] { ".ctor" }/*,
+            verify(source, expectedAnalyzedKeys: new[] { ".ctor" },
                 // (6,5): warning CS8618: Non-nullable field 'F1' must contain a non-null value when exiting constructor. Consider declaring the field as nullable.
                 //     S(object obj) : this() { }
-                Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "S").WithArguments("field", "F1").WithLocation(6, 5)*/);
+                Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "S").WithArguments("field", "F1").WithLocation(6, 5)
+                );
 
             source =
 @"#pragma warning disable 169