Merge pull request #49261 from AlekseyTs/ConstraintCycles_02

 Avoid cycles while binding type parameter constraints

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

@@ -251,10 +251,6 @@ internal bool AreNullableAnnotationsEnabled(SyntaxTree syntaxTree, int position)
         internal bool AreNullableAnnotationsEnabled(SyntaxToken token)
         {
             RoslynDebug.Assert(token.SyntaxTree is object);
-            if ((Flags & BinderFlags.IgnoreNullableContext) != 0)
-            {
-                return false;
-            }
             return AreNullableAnnotationsEnabled(token.SyntaxTree, token.SpanStart);
         }
 

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

@@ -113,9 +113,10 @@ internal enum BinderFlags : uint
         InEEMethodBinder = 1 << 30,
 
         /// <summary>
-        /// Assume '#nullable disable' context.
+        /// Skip binding type arguments (we use <see cref="Symbols.PlaceholderTypeArgumentSymbol"/> instead).
+        /// For example, currently used when type constraints are bound in some scenarios.
         /// </summary>
-        IgnoreNullableContext = 1u << 31,
+        SuppressTypeArgumentBinding = 1u << 31,
 
         // Groups
 

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

@@ -25,9 +25,8 @@ internal ImmutableArray<TypeParameterConstraintClause> BindTypeParameterConstrai
             ImmutableArray<TypeParameterSymbol> typeParameters,
             TypeParameterListSyntax typeParameterList,
             SyntaxList<TypeParameterConstraintClauseSyntax> clauses,
-            bool canIgnoreNullableContext,
-            ref IReadOnlyDictionary<TypeParameterSymbol, bool> isValueTypeOverride,
             DiagnosticBag diagnostics,
+            bool performOnlyCycleSafeValidation,
             bool isForOverride = false)
         {
             Debug.Assert(this.Flags.Includes(BinderFlags.GenericConstraintsClause));
@@ -66,8 +65,7 @@ internal ImmutableArray<TypeParameterConstraintClause> BindTypeParameterConstrai
                     Debug.Assert(ordinal >= 0);
                     Debug.Assert(ordinal < n);
 
-                    (TypeParameterConstraintClause constraintClause, ArrayBuilder<TypeConstraintSyntax>? typeConstraintNodes) =
-                        this.BindTypeParameterConstraints(typeParameterList.Parameters[ordinal], clause, isForOverride, canIgnoreNullableContext: canIgnoreNullableContext, diagnostics);
+                    (TypeParameterConstraintClause constraintClause, ArrayBuilder<TypeConstraintSyntax>? typeConstraintNodes) = this.BindTypeParameterConstraints(typeParameterList.Parameters[ordinal], clause, isForOverride, diagnostics);
                     if (results[ordinal] == null)
                     {
                         results[ordinal] = constraintClause;
@@ -101,9 +99,7 @@ internal ImmutableArray<TypeParameterConstraintClause> BindTypeParameterConstrai
                 }
             }
 
-            TypeParameterConstraintClause.AdjustConstraintTypes(containingSymbol, typeParameters, results, ref isValueTypeOverride);
-
-            RemoveInvalidConstraints(typeParameters, results!, syntaxNodes, diagnostics);
+            RemoveInvalidConstraints(typeParameters, results!, syntaxNodes, performOnlyCycleSafeValidation, diagnostics);
 
             foreach (var typeConstraintsSyntaxes in syntaxNodes)
             {
@@ -118,8 +114,7 @@ internal ImmutableArray<TypeParameterConstraintClause> BindTypeParameterConstrai
         /// <summary>
         /// Bind and return a single type parameter constraint clause along with syntax nodes corresponding to type constraints.
         /// </summary>
-        private (TypeParameterConstraintClause, ArrayBuilder<TypeConstraintSyntax>?) BindTypeParameterConstraints(
-            TypeParameterSyntax typeParameterSyntax, TypeParameterConstraintClauseSyntax constraintClauseSyntax, bool isForOverride, bool canIgnoreNullableContext, DiagnosticBag diagnostics)
+        private (TypeParameterConstraintClause, ArrayBuilder<TypeConstraintSyntax>?) BindTypeParameterConstraints(TypeParameterSyntax typeParameterSyntax, TypeParameterConstraintClauseSyntax constraintClauseSyntax, bool isForOverride, DiagnosticBag diagnostics)
         {
             var constraints = TypeParameterConstraintKind.None;
             ArrayBuilder<TypeWithAnnotations>? constraintTypes = null;
@@ -309,7 +304,7 @@ internal ImmutableArray<TypeParameterConstraintClause> BindTypeParameterConstrai
             Debug.Assert(!isForOverride ||
                          (constraints & (TypeParameterConstraintKind.ReferenceType | TypeParameterConstraintKind.ValueType)) != (TypeParameterConstraintKind.ReferenceType | TypeParameterConstraintKind.ValueType));
 
-            return (TypeParameterConstraintClause.Create(constraints, constraintTypes?.ToImmutableAndFree() ?? ImmutableArray<TypeWithAnnotations>.Empty, canIgnoreNullableContext), syntaxBuilder);
+            return (TypeParameterConstraintClause.Create(constraints, constraintTypes?.ToImmutableAndFree() ?? ImmutableArray<TypeWithAnnotations>.Empty), syntaxBuilder);
 
             static void reportOverrideWithConstraints(ref bool reportedOverrideWithConstraints, TypeParameterConstraintSyntax syntax, DiagnosticBag diagnostics)
             {
@@ -350,21 +345,23 @@ private static void RemoveInvalidConstraints(
             ImmutableArray<TypeParameterSymbol> typeParameters,
             ArrayBuilder<TypeParameterConstraintClause> constraintClauses,
             ArrayBuilder<ArrayBuilder<TypeConstraintSyntax>?> syntaxNodes,
+            bool performOnlyCycleSafeValidation,
             DiagnosticBag diagnostics)
         {
             Debug.Assert(typeParameters.Length > 0);
             Debug.Assert(typeParameters.Length == constraintClauses.Count);
             int n = typeParameters.Length;
             for (int i = 0; i < n; i++)
             {
-                constraintClauses[i] = RemoveInvalidConstraints(typeParameters[i], constraintClauses[i], syntaxNodes[i], diagnostics);
+                constraintClauses[i] = RemoveInvalidConstraints(typeParameters[i], constraintClauses[i], syntaxNodes[i], performOnlyCycleSafeValidation, diagnostics);
             }
         }
 
         private static TypeParameterConstraintClause RemoveInvalidConstraints(
             TypeParameterSymbol typeParameter,
             TypeParameterConstraintClause constraintClause,
             ArrayBuilder<TypeConstraintSyntax>? syntaxNodesOpt,
+            bool performOnlyCycleSafeValidation,
             DiagnosticBag diagnostics)
         {
             if (syntaxNodesOpt != null)
@@ -383,16 +380,20 @@ private static TypeParameterConstraintClause RemoveInvalidConstraints(
                     // since, in general, it may be difficult to support all invalid types.
                     // In the future, we may want to include some invalid types
                     // though so the public binding API has the most information.
-                    if (IsValidConstraint(typeParameter.Name, syntax, constraintType, constraintClause.Constraints, constraintTypeBuilder, diagnostics))
+                    if (IsValidConstraint(typeParameter, syntax, constraintType, constraintClause.Constraints, constraintTypeBuilder, performOnlyCycleSafeValidation, diagnostics))
                     {
-                        CheckConstraintTypeVisibility(containingSymbol, syntax.Location, constraintType, diagnostics);
+                        if (!performOnlyCycleSafeValidation)
+                        {
+                            CheckConstraintTypeVisibility(containingSymbol, syntax.Location, constraintType, diagnostics);
+                        }
+
                         constraintTypeBuilder.Add(constraintType);
                     }
                 }
 
                 if (constraintTypeBuilder.Count < n)
                 {
-                    return TypeParameterConstraintClause.Create(constraintClause.Constraints, constraintTypeBuilder.ToImmutableAndFree(), constraintClause.IgnoresNullableContext);
+                    return TypeParameterConstraintClause.Create(constraintClause.Constraints, constraintTypeBuilder.ToImmutableAndFree());
                 }
 
                 constraintTypeBuilder.Free();
@@ -421,26 +422,28 @@ private static void CheckConstraintTypeVisibility(
         /// returns false and generates a diagnostic.
         /// </summary>
         private static bool IsValidConstraint(
-            string typeParameterName,
+            TypeParameterSymbol typeParameter,
             TypeConstraintSyntax syntax,
             TypeWithAnnotations type,
             TypeParameterConstraintKind constraints,
             ArrayBuilder<TypeWithAnnotations> constraintTypes,
+            bool performOnlyCycleSafeValidation,
             DiagnosticBag diagnostics)
         {
-            if (!isValidConstraintType(syntax, type, diagnostics))
+            if (!isValidConstraintType(typeParameter, syntax, type, performOnlyCycleSafeValidation, diagnostics))
             {
                 return false;
             }
 
-            if (constraintTypes.Contains(c => type.Equals(c, TypeCompareKind.AllIgnoreOptions)))
+            if (!performOnlyCycleSafeValidation && constraintTypes.Contains(c => type.Equals(c, TypeCompareKind.AllIgnoreOptions)))
             {
                 // "Duplicate constraint '{0}' for type parameter '{1}'"
-                Error(diagnostics, ErrorCode.ERR_DuplicateBound, syntax, type.Type.SetUnknownNullabilityForReferenceTypes(), typeParameterName);
+                Error(diagnostics, ErrorCode.ERR_DuplicateBound, syntax, type.Type.SetUnknownNullabilityForReferenceTypes(), typeParameter.Name);
                 return false;
             }
 
-            if (type.TypeKind == TypeKind.Class)
+            if (!type.DefaultType.IsTypeParameter() && // Doing an explicit check for type parameter on unresolved type to avoid cycles while calculating TypeKind. An unresolved type parameter cannot resolve to a class.   
+                type.TypeKind == TypeKind.Class)
             {
                 // If there is already a struct or class constraint (class constraint could be
                 // 'class' or explicit type), report an error and drop this class. If we don't
@@ -489,8 +492,14 @@ private static bool IsValidConstraint(
 
             // Returns true if the type is a valid constraint type.
             // Otherwise returns false and generates a diagnostic.
-            static bool isValidConstraintType(TypeConstraintSyntax syntax, TypeWithAnnotations typeWithAnnotations, DiagnosticBag diagnostics)
+            static bool isValidConstraintType(TypeParameterSymbol typeParameter, TypeConstraintSyntax syntax, TypeWithAnnotations typeWithAnnotations, bool performOnlyCycleSafeValidation, DiagnosticBag diagnostics)
             {
+                if (typeWithAnnotations.NullableAnnotation == NullableAnnotation.Annotated && performOnlyCycleSafeValidation &&
+                    typeWithAnnotations.DefaultType is TypeParameterSymbol typeParameterInConstraint && typeParameterInConstraint.ContainingSymbol == (object)typeParameter.ContainingSymbol)
+                {
+                    return true;
+                }
+
                 TypeSymbol type = typeWithAnnotations.Type;
 
                 switch (type.SpecialType)

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

@@ -1154,6 +1154,10 @@ private TypeWithAnnotations BindGenericSimpleNamespaceOrTypeOrAliasSymbol(
                     resultType = unconstructedType.AsUnboundGenericType();
                 }
             }
+            else if ((Flags & BinderFlags.SuppressTypeArgumentBinding) != 0)
+            {
+                resultType = unconstructedType.Construct(PlaceholderTypeArgumentSymbol.CreateTypeArguments(unconstructedType.TypeParameters));
+            }
             else
             {
                 // It's not an unbound type expression, so we must have type arguments, and we have a

src/Compilers/CSharp/Portable/Binder/Semantics/AccessCheck.cs

@@ -236,7 +236,7 @@ private static bool IsNamedTypeAccessible(NamedTypeSymbol type, Symbol within, r
                 {
                     // type parameters are always accessible, so don't check those (so common it's
                     // worth optimizing this).
-                    if (typeArg.DefaultType.TypeKind != TypeKind.TypeParameter && !IsSymbolAccessibleCore(typeArg.Type, within, null, out unused, compilation, ref useSiteDiagnostics, basesBeingResolved))
+                    if (typeArg.Type.Kind != SymbolKind.TypeParameter && !IsSymbolAccessibleCore(typeArg.Type, within, null, out unused, compilation, ref useSiteDiagnostics, basesBeingResolved))
                     {
                         return false;
                     }

src/Compilers/CSharp/Portable/Lowering/SynthesizedMethodBaseSymbol.cs

@@ -88,8 +88,11 @@ public sealed override ImmutableArray<TypeParameterSymbol> TypeParameters
             get { return _typeParameters; }
         }
 
-        public sealed override ImmutableArray<TypeParameterConstraintClause> GetTypeParameterConstraintClauses(bool canIgnoreNullableContext)
-            => ImmutableArray<TypeParameterConstraintClause>.Empty;
+        public sealed override ImmutableArray<ImmutableArray<TypeWithAnnotations>> GetTypeParameterConstraintTypes()
+            => ImmutableArray<ImmutableArray<TypeWithAnnotations>>.Empty;
+
+        public sealed override ImmutableArray<TypeParameterConstraintKind> GetTypeParameterConstraintKinds()
+            => ImmutableArray<TypeParameterConstraintKind>.Empty;
 
         internal override int ParameterCount
         {

src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/PublicSymbols/AnonymousType.TypePublicSymbol.cs

@@ -330,10 +330,8 @@ internal override ImmutableArray<NamedTypeSymbol> GetDeclaredInterfaces(ConsList
 
             internal override bool IsRecord => false;
 
-            internal override bool Equals(TypeSymbol t2, TypeCompareKind comparison, IReadOnlyDictionary<TypeParameterSymbol, bool> isValueTypeOverrideOpt = null)
+            internal override bool Equals(TypeSymbol t2, TypeCompareKind comparison)
             {
-                Debug.Assert(isValueTypeOverrideOpt == null);
-
                 if (ReferenceEquals(this, t2))
                 {
                     return true;

src/Compilers/CSharp/Portable/Symbols/AnonymousTypes/SynthesizedSymbols/AnonymousType.TypeParameterSymbol.cs

@@ -72,6 +72,11 @@ public override bool HasReferenceTypeConstraint
                 get { return false; }
             }
 
+            public override bool IsReferenceTypeFromConstraintTypes
+            {
+                get { return false; }
+            }
+
             internal override bool? ReferenceTypeConstraintIsNullable
             {
                 get { return false; }
@@ -86,6 +91,11 @@ public override bool HasValueTypeConstraint
                 get { return false; }
             }
 
+            public override bool IsValueTypeFromConstraintTypes
+            {
+                get { return false; }
+            }
+
             public override bool HasUnmanagedTypeConstraint
             {
                 get { return false; }
@@ -101,11 +111,11 @@ public override VarianceKind Variance
                 get { return VarianceKind.None; }
             }
 
-            internal override void EnsureAllConstraintsAreResolved(bool canIgnoreNullableContext)
+            internal override void EnsureAllConstraintsAreResolved()
             {
             }
 
-            internal override ImmutableArray<TypeWithAnnotations> GetConstraintTypes(ConsList<TypeParameterSymbol> inProgress, bool canIgnoreNullableContext)
+            internal override ImmutableArray<TypeWithAnnotations> GetConstraintTypes(ConsList<TypeParameterSymbol> inProgress)
             {
                 return ImmutableArray<TypeWithAnnotations>.Empty;
             }

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

@@ -337,20 +337,20 @@ public override TResult Accept<TResult>(CSharpSymbolVisitor<TResult> visitor)
             return visitor.VisitArrayType(this);
         }
 
-        internal override bool Equals(TypeSymbol? t2, TypeCompareKind comparison, IReadOnlyDictionary<TypeParameterSymbol, bool>? isValueTypeOverrideOpt = null)
+        internal override bool Equals(TypeSymbol? t2, TypeCompareKind comparison)
         {
-            return this.Equals(t2 as ArrayTypeSymbol, comparison, isValueTypeOverrideOpt);
+            return this.Equals(t2 as ArrayTypeSymbol, comparison);
         }
 
-        private bool Equals(ArrayTypeSymbol? other, TypeCompareKind comparison, IReadOnlyDictionary<TypeParameterSymbol, bool>? isValueTypeOverrideOpt)
+        private bool Equals(ArrayTypeSymbol? other, TypeCompareKind comparison)
         {
             if (ReferenceEquals(this, other))
             {
                 return true;
             }
 
             if ((object?)other == null || !other.HasSameShapeAs(this) ||
-                !other.ElementTypeWithAnnotations.Equals(ElementTypeWithAnnotations, comparison, isValueTypeOverrideOpt))
+                !other.ElementTypeWithAnnotations.Equals(ElementTypeWithAnnotations, comparison))
             {
                 return false;
             }