Error when required members are not set

Implements reading the required members list of a type, and enforces that required members are all set by an initializer on the constructor. Required members must be initialized with values, not with nested object initializers.

Test plan dotnet#57046.
Specification https://github.com/dotnet/csharplang/blob/main/proposals/required-members.md

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

@@ -4967,6 +4967,78 @@ private static void ReportDuplicateObjectMemberInitializers(BoundExpression boun
             }
         }
 
+        private static void CheckRequiredMembersInObjectInitializer(
+            BoundObjectCreationExpression creation,
+            BindingDiagnosticBag diagnostics)
+        {
+            if (!creation.Constructor.ShouldCheckRequiredMembers())
+            {
+                return;
+            }
+
+            if (creation.Constructor.ContainingType.HasRequiredMembersError)
+            {
+                // An error will be reported on the constructor if from source, or a use-site diagnostic will be reported on the use if from metadata.
+                return;
+            }
+
+            var requiredMembers = creation.Constructor.ContainingType.AllRequiredMembers.ToBuilder();
+
+            if (requiredMembers.Count == 0)
+            {
+                return;
+            }
+
+            if (creation.InitializerExpressionOpt == null)
+            {
+                reportMembers();
+                return;
+            }
+
+            foreach (var initializer in creation.InitializerExpressionOpt.Initializers)
+            {
+                if (initializer is not BoundAssignmentOperator { Left: BoundObjectInitializerMember member, Right: { } initializerExpression })
+                {
+                    continue;
+                }
+
+                if (!requiredMembers.TryGetValue(member.MemberSymbol.Name, out var requiredMember))
+                {
+                    continue;
+                }
+
+                if (!member.MemberSymbol.Equals(requiredMember, TypeCompareKind.ConsiderEverything))
+                {
+                    continue;
+                }
+
+                requiredMembers.Remove(member.MemberSymbol.Name);
+
+                if (initializerExpression is BoundObjectInitializerExpressionBase)
+                {
+                    diagnostics.Add(ErrorCode.ERR_RequiredMembersMustBeAssignment, initializerExpression.Syntax.Location, requiredMember);
+                }
+            }
+
+            reportMembers();
+
+            void reportMembers()
+            {
+                Location location = creation.Syntax switch
+                {
+                    ObjectCreationExpressionSyntax { Type: { } type } => type.Location,
+                    BaseObjectCreationExpressionSyntax { NewKeyword: { } newKeyword } => newKeyword.GetLocation(),
+                    _ => creation.Syntax.Location
+                };
+
+                foreach (var (_, member) in requiredMembers)
+                {
+                    // Required member '{0}' must be set in the object initializer.
+                    diagnostics.Add(ErrorCode.ERR_RequiredMemberMustBeSet, location, member);
+                }
+            }
+        }
+
         private BoundCollectionInitializerExpression BindCollectionInitializerExpression(
             InitializerExpressionSyntax initializerSyntax,
             TypeSymbol initializerType,
@@ -5411,7 +5483,7 @@ protected BoundExpression BindClassCreationExpression(
                 }
 
                 boundInitializerOpt = makeBoundInitializerOpt();
-                result = new BoundObjectCreationExpression(
+                var creation = new BoundObjectCreationExpression(
                     node,
                     method,
                     candidateConstructors,
@@ -5427,7 +5499,9 @@ protected BoundExpression BindClassCreationExpression(
                     type,
                     hasError);
 
-                return result;
+                CheckRequiredMembersInObjectInitializer(creation, diagnostics);
+
+                return creation;
             }
 
             LookupResultKind resultKind;

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -7007,6 +7007,18 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
   <data name="ERR_RequiredMemberMustBeSettable" xml:space="preserve">
     <value>Required member '{0}' must be settable.</value>
   </data>
+  <data name="ERR_RequiredMemberMustBeSet" xml:space="preserve">
+    <value>Required member '{0}' must be set in the object initializer.</value>
+  </data>
+  <data name="ERR_RequiredMembersMustBeAssignment" xml:space="preserve">
+    <value>Required member '{0}' must be assigned a value, it cannot use a nested member or collection initializer.</value>
+  </data>
+  <data name="ERR_RequiredMembersInvalid" xml:space="preserve">
+    <value>The required members list for '{0}' is malformed and cannot be interpreted.</value>
+  </data>
+  <data name="ERR_RequiredMembersBaseTypeInvalid" xml:space="preserve">
+    <value>The required members list for the base type '{0}' is malformed and cannot be interpreted. To use this constructor, apply the `SetsRequiredMembers` attribute.</value>
+  </data>
   <data name="ERR_LineContainsDifferentWhitespace" xml:space="preserve">
     <value>Line contains different whitespace than the closing line of the raw string literal: '{0}' versus '{1}'</value>
   </data>

src/Compilers/CSharp/Portable/Errors/ErrorCode.cs

@@ -2058,5 +2058,9 @@ internal enum ErrorCode
         ERR_RequiredMemberCannotBeLessVisibleThanContainingType = 9503,
         ERR_ExplicitRequiredMember = 9504,
         ERR_RequiredMemberMustBeSettable = 9505,
+        ERR_RequiredMemberMustBeSet = 9506,
+        ERR_RequiredMembersMustBeAssignment = 9507,
+        ERR_RequiredMembersInvalid = 9508,
+        ERR_RequiredMembersBaseTypeInvalid = 9509,
     }
 }

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs

@@ -1346,6 +1346,11 @@ internal override UseSiteInfo<AssemblySymbol> GetUseSiteInfo()
                     }
                 }
 
+                if (diagnosticInfo == null && this.ShouldCheckRequiredMembers() && ContainingType.HasRequiredMembersError)
+                {
+                    diagnosticInfo = new CSDiagnosticInfo(ErrorCode.ERR_RequiredMembersInvalid, ContainingType);
+                }
+
                 return InitializeUseSiteDiagnostic(result.AdjustDiagnosticInfo(diagnosticInfo));
             }
 

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

@@ -11,6 +11,8 @@
 using System.Linq;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
+using System.Threading;
+using Microsoft.CodeAnalysis.Collections;
 using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Symbols;
 using Roslyn.Utilities;
@@ -24,6 +26,14 @@ internal abstract partial class NamedTypeSymbol : TypeSymbol, INamedTypeSymbolIn
     {
         private bool _hasNoBaseCycles;
 
+        /// <remarks>
+        /// This field cannot be used to determine whether <see cref="_lazyRequiredMembers"/> has been initialized.
+        /// Ensure that <see cref="_lazyRequiredMembers"/> is checked for defaultness _before_ reading this field, which potentially
+        /// means using an `Interlocked.MemoryBarrier()` to ensure that reading this location is not reordered before the read to <see cref="_lazyRequiredMembers"/>.
+        /// </remarks>
+        private bool _lazyHasRequiredMembersErrorDoNotAccessDirectly = false;
+        private ImmutableSegmentedDictionary<string, Symbol> _lazyRequiredMembers;
+
         // Only the compiler can create NamedTypeSymbols.
         internal NamedTypeSymbol(TupleExtraData tupleData = null)
         {
@@ -499,6 +509,118 @@ internal abstract bool MangleName
         /// </summary>
         internal abstract bool HasDeclaredRequiredMembers { get; }
 
+#nullable enable
+        /// <summary>
+        /// Whether the type encountered an error while trying to build its complete list of required members.
+        /// </summary>
+        internal bool HasRequiredMembersError
+        {
+            get
+            {
+                CalculateRequiredMembersIfRequired();
+                Debug.Assert(!_lazyRequiredMembers.IsDefault);
+                return _lazyHasRequiredMembersErrorDoNotAccessDirectly;
+            }
+        }
+
+        /// <summary>
+        /// The full list of all required members for this type, including from base classes. If <see cref="HasRequiredMembersError"/> is true,
+        /// this returns empty.
+        /// </summary>
+        /// <remarks>
+        /// Do not call this API if all you need are the required members declared on this type. Use <see cref="GetMembers()"/> instead, filtering for
+        /// required members, instead of calling this API.
+        /// </remarks>
+        internal ImmutableSegmentedDictionary<string, Symbol> AllRequiredMembers
+        {
+            get
+            {
+                CalculateRequiredMembersIfRequired();
+                Debug.Assert(!_lazyRequiredMembers.IsDefault);
+                return _lazyRequiredMembers;
+            }
+        }
+
+        private void CalculateRequiredMembersIfRequired()
+        {
+            if (_lazyRequiredMembers.IsDefault)
+            {
+                ImmutableSegmentedDictionary<string, Symbol>.Builder? builder = null;
+                bool success = TryCalculateRequiredMembers(ref builder);
+                var requiredMembers = success
+                    ? builder?.ToImmutable() ?? ImmutableSegmentedDictionary<string, Symbol>.Empty
+                    : ImmutableSegmentedDictionary<string, Symbol>.Empty;
+
+                _lazyHasRequiredMembersErrorDoNotAccessDirectly = !success;
+                // InterlockedInitialize uses `Interlocked.CompareExchange` under the hood, which will introduce a full
+                // memory barrier, ensuring that the _lazyHasRequiredMembersErrorDoNotAccessDirectly write is not reordered
+                // after the initialization of _lazyRequiredMembers.
+                RoslynImmutableInterlocked.InterlockedInitialize(ref _lazyRequiredMembers, requiredMembers);
+            }
+            else
+            {
+                // Ensure that, after this method, _lazyRequiredMembersErrorDoNotAccessDirectly is able to be accessed safely.
+                Interlocked.MemoryBarrier();
+            }
+        }
+
+        /// <summary>
+        /// Attempts to calculate the required members for this type. Returns false if there were errors.
+        /// </summary>
+        private bool TryCalculateRequiredMembers(ref ImmutableSegmentedDictionary<string, Symbol>.Builder? requiredMembersBuilder)
+        {
+            var lazyRequiredMembers = _lazyRequiredMembers;
+            if (!lazyRequiredMembers.IsDefault)
+            {
+                requiredMembersBuilder = lazyRequiredMembers.ToBuilder();
+                // Ensure that this read is not reordered before the read to `IsDefault`.
+                Interlocked.MemoryBarrier();
+                return !_lazyHasRequiredMembersErrorDoNotAccessDirectly;
+            }
+
+            if (BaseTypeNoUseSiteDiagnostics?.TryCalculateRequiredMembers(ref requiredMembersBuilder) == false)
+            {
+                return false;
+            }
+
+            // We need to make sure that members from a base type weren't hidden by members from the current type.
+            if (!HasDeclaredRequiredMembers && requiredMembersBuilder == null)
+            {
+                return true;
+            }
+
+            return addCurrentTypeMembers(ref requiredMembersBuilder);
+
+            bool addCurrentTypeMembers(ref ImmutableSegmentedDictionary<string, Symbol>.Builder? requiredMembersBuilder)
+            {
+                requiredMembersBuilder ??= ImmutableSegmentedDictionary.CreateBuilder<string, Symbol>();
+
+                foreach (var member in GetMembersUnordered())
+                {
+                    if (requiredMembersBuilder.ContainsKey(member.Name))
+                    {
+                        // This is only permitted if the member is an override of a required member from a base type, and is required itself.
+                        if (!member.IsRequired()
+                            || member.GetOverriddenMember() is not { } overriddenMember
+                            || !overriddenMember.Equals(requiredMembersBuilder[member.Name], TypeCompareKind.ConsiderEverything))
+                        {
+                            return false;
+                        }
+                    }
+
+                    if (!member.IsRequired())
+                    {
+                        continue;
+                    }
+
+                    requiredMembersBuilder[member.Name] = member;
+                }
+
+                return true;
+            }
+        }
+#nullable disable
+
         /// <summary>
         /// Get all the members of this symbol.
         /// </summary>

src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs

@@ -2400,6 +2400,20 @@ private void CheckForRequiredMemberAttribute(BindingDiagnosticBag diagnostics)
                 // Ensure that an error is reported if the required constructor isn't present.
                 _ = Binder.GetWellKnownTypeMember(DeclaringCompilation, WellKnownMember.System_Runtime_CompilerServices_RequiredMemberAttribute__ctor, diagnostics, Locations[0]);
             }
+
+            if (BaseTypeNoUseSiteDiagnostics is (not SourceMemberContainerTypeSymbol) and { HasRequiredMembersError: true })
+            {
+                foreach (var member in GetMembersUnordered())
+                {
+                    if (member is not MethodSymbol method || !method.ShouldCheckRequiredMembers())
+                    {
+                        continue;
+                    }
+
+                    // The required members list for the base type '{0}' is malformed and cannot be interpreted. To use this constructor, apply the `SetsRequiredMembers` attribute.
+                    diagnostics.Add(ErrorCode.ERR_RequiredMembersBaseTypeInvalid, method.Locations[0], BaseTypeNoUseSiteDiagnostics);
+                }
+            }
         }
 
         private bool TypeOverridesObjectMethod(string name)

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

@@ -841,5 +841,9 @@ 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 constructor)
+            // PROTOTYPE(req): Check for the SetsRequiredMembersAttribute and return false for that case
+            => constructor.MethodKind == MethodKind.Constructor;
     }
 }