Fix crash in semantic model when using unmanaged constraint (#37022)

The semantic model often creates ReducedExtensionMethods for the public API,
but the creation of ReducedExtensionMethod does constraint checking, which
requires a compilation, which isn't always available. This change removes constraint
checking from all creation of ReducedExtensionMethod and adds asserts that we
always have a compilation during constraint checking.

Fixes #37021

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

@@ -6286,20 +6286,9 @@ private static void CombineExtensionMethodArguments(BoundExpression receiver, An
                 // Arguments will be null if the caller is resolving to the first method group that can accept
                 // that receiver, regardless of arguments, when the signature cannot
                 // be inferred. (In the case of nameof(o.M) or the error case of o.M = null; for instance.)
-                if (analyzedArguments == null)
+                if (analyzedArguments == null && methodGroup.Methods.Count != 0)
                 {
-                    if (expression == EnclosingNameofArgument)
-                    {
-                        for (int i = methodGroup.Methods.Count - 1; i >= 0; i--)
-                        {
-                            if ((object)methodGroup.Methods[i].ReduceExtensionMethod(left.Type) == null) methodGroup.Methods.RemoveAt(i);
-                        }
-                    }
-
-                    if (methodGroup.Methods.Count != 0)
-                    {
-                        return new MethodGroupResolution(methodGroup, diagnostics.ToReadOnlyAndFree());
-                    }
+                    return new MethodGroupResolution(methodGroup, diagnostics.ToReadOnlyAndFree());
                 }
 
                 if (methodGroup.Methods.Count == 0)

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

@@ -511,6 +511,8 @@ void populateDiagnosticsAndClear(ArrayBuilder<TypeParameterDiagnosticInfo> build
         {
             Debug.Assert(!type.IsTupleType);
             Debug.Assert(typeArgumentsSyntax.Count == 0 /*omitted*/ || typeArgumentsSyntax.Count == type.Arity);
+            Debug.Assert(currentCompilation is object);
+
             if (!RequiresChecking(type))
             {
                 return true;
@@ -551,6 +553,8 @@ void populateDiagnosticsAndClear(ArrayBuilder<TypeParameterDiagnosticInfo> build
             Location location,
             DiagnosticBag diagnostics)
         {
+            Debug.Assert(currentCompilation is object);
+
             // We do not report element locations in method parameters and return types
             // so we will simply unwrap the type if it was a tuple. We are relying on
             // TypeSymbolExtensions.VisitType to dig into the "Rest" tuple so that they
@@ -817,6 +821,7 @@ private static bool HasDuplicateInterfaces(NamedTypeSymbol type, ConsList<TypeSy
             HashSet<TypeParameterSymbol> ignoreTypeConstraintsDependentOnTypeParametersOpt)
         {
             Debug.Assert(substitution != null);
+            Debug.Assert(currentCompilation is object);
 
             // The type parameters must be original definitions of type parameters from the containing symbol.
             Debug.Assert(ReferenceEquals(typeParameter.ContainingSymbol, containingSymbol.OriginalDefinition));

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

@@ -688,10 +688,7 @@ public MethodSymbol ReduceExtensionMethod(TypeSymbol receiverType)
                 return null;
             }
 
-            // To give optimal diagnostics, we should really pass the "current" compilation.
-            // However, this is never used in batch scenarios, so it doesn't matter
-            // (modulo future changes to the API).
-            return ReducedExtensionMethodSymbol.Create(this, receiverType, compilation: null);
+            return ReducedExtensionMethodSymbol.Create(this, receiverType);
         }
 
         /// <summary>

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

@@ -17,144 +17,6 @@ public static bool IsParams(this MethodSymbol method)
             return method.ParameterCount != 0 && method.Parameters[method.ParameterCount - 1].IsParams;
         }
 
-        /// <summary>
-        /// If the extension method is applicable based on the "this" argument type, return
-        /// the method constructed with the inferred type arguments. If the method is not an
-        /// unconstructed generic method, type inference is skipped. If the method is not
-        /// applicable, or if constraints when inferring type parameters from the "this" type
-        /// are not satisfied, the return value is null.
-        /// </summary>
-        public static MethodSymbol InferExtensionMethodTypeArguments(this MethodSymbol method, TypeSymbol thisType, Compilation compilation, ref HashSet<DiagnosticInfo> useSiteDiagnostics)
-        {
-            Debug.Assert(method.IsExtensionMethod);
-            Debug.Assert((object)thisType != null);
-
-            if (!method.IsGenericMethod || method != method.ConstructedFrom)
-            {
-                return method;
-            }
-
-            // We never resolve extension methods on a dynamic receiver.
-            if (thisType.IsDynamic())
-            {
-                return null;
-            }
-
-            var containingAssembly = method.ContainingAssembly;
-            var errorNamespace = containingAssembly.GlobalNamespace;
-            var conversions = new TypeConversions(containingAssembly.CorLibrary);
-
-            // There is absolutely no plausible syntax/tree that we could use for these
-            // synthesized literals.  We could be speculatively binding a call to a PE method.
-            var syntaxTree = CSharpSyntaxTree.Dummy;
-            var syntax = (CSharpSyntaxNode)syntaxTree.GetRoot();
-
-            // Create an argument value for the "this" argument of specific type,
-            // and pass the same bad argument value for all other arguments.
-            var thisArgumentValue = new BoundLiteral(syntax, ConstantValue.Bad, thisType) { WasCompilerGenerated = true };
-            var otherArgumentType = new ExtendedErrorTypeSymbol(errorNamespace, name: string.Empty, arity: 0, errorInfo: null, unreported: false);
-            var otherArgumentValue = new BoundLiteral(syntax, ConstantValue.Bad, otherArgumentType) { WasCompilerGenerated = true };
-
-            var paramCount = method.ParameterCount;
-            var arguments = new BoundExpression[paramCount];
-
-            for (int i = 0; i < paramCount; i++)
-            {
-                var argument = (i == 0) ? thisArgumentValue : otherArgumentValue;
-                arguments[i] = argument;
-            }
-
-            var typeArgs = MethodTypeInferrer.InferTypeArgumentsFromFirstArgument(
-                conversions,
-                method,
-                arguments.AsImmutable(),
-                useSiteDiagnostics: ref useSiteDiagnostics);
-
-            if (typeArgs.IsDefault)
-            {
-                return null;
-            }
-
-            int firstNullInTypeArgs = -1;
-
-            // For the purpose of constraint checks we use error type symbol in place of type arguments that we couldn't infer from the first argument.
-            // This prevents constraint checking from failing for corresponding type parameters. 
-            var notInferredTypeParameters = PooledHashSet<TypeParameterSymbol>.GetInstance();
-            var typeParams = method.TypeParameters;
-            var typeArgsForConstraintsCheck = typeArgs;
-            for (int i = 0; i < typeArgsForConstraintsCheck.Length; i++)
-            {
-                if (!typeArgsForConstraintsCheck[i].HasType)
-                {
-                    firstNullInTypeArgs = i;
-                    var builder = ArrayBuilder<TypeWithAnnotations>.GetInstance();
-                    builder.AddRange(typeArgsForConstraintsCheck, firstNullInTypeArgs);
-
-                    for (; i < typeArgsForConstraintsCheck.Length; i++)
-                    {
-                        var typeArg = typeArgsForConstraintsCheck[i];
-                        if (!typeArg.HasType)
-                        {
-                            notInferredTypeParameters.Add(typeParams[i]);
-                            builder.Add(TypeWithAnnotations.Create(ErrorTypeSymbol.UnknownResultType));
-                        }
-                        else
-                        {
-                            builder.Add(typeArg);
-                        }
-                    }
-
-                    typeArgsForConstraintsCheck = builder.ToImmutableAndFree();
-                    break;
-                }
-            }
-
-            // Check constraints.
-            var diagnosticsBuilder = ArrayBuilder<TypeParameterDiagnosticInfo>.GetInstance();
-            var substitution = new TypeMap(typeParams, typeArgsForConstraintsCheck);
-            ArrayBuilder<TypeParameterDiagnosticInfo> useSiteDiagnosticsBuilder = null;
-            var success = method.CheckConstraints(conversions, includeNullability: false, substitution, typeParams, typeArgsForConstraintsCheck, compilation, diagnosticsBuilder, nullabilityDiagnosticsBuilderOpt: null, ref useSiteDiagnosticsBuilder,
-                                                  ignoreTypeConstraintsDependentOnTypeParametersOpt: notInferredTypeParameters.Count > 0 ? notInferredTypeParameters : null);
-            diagnosticsBuilder.Free();
-            notInferredTypeParameters.Free();
-
-            if (useSiteDiagnosticsBuilder != null && useSiteDiagnosticsBuilder.Count > 0)
-            {
-                if (useSiteDiagnostics == null)
-                {
-                    useSiteDiagnostics = new HashSet<DiagnosticInfo>();
-                }
-
-                foreach (var diag in useSiteDiagnosticsBuilder)
-                {
-                    useSiteDiagnostics.Add(diag.DiagnosticInfo);
-                }
-            }
-
-            if (!success)
-            {
-                return null;
-            }
-
-            // For the purpose of construction we use original type parameters in place of type arguments that we couldn't infer from the first argument.
-            var typeArgsForConstruct = typeArgs;
-            if (firstNullInTypeArgs != -1)
-            {
-                var builder = ArrayBuilder<TypeWithAnnotations>.GetInstance();
-                builder.AddRange(typeArgs, firstNullInTypeArgs);
-
-                for (int i = firstNullInTypeArgs; i < typeArgsForConstruct.Length; i++)
-                {
-                    var typeArgForConstruct = typeArgsForConstruct[i];
-                    builder.Add(typeArgForConstruct.HasType ? typeArgForConstruct : TypeWithAnnotations.Create(typeParams[i]));
-                }
-
-                typeArgsForConstruct = builder.ToImmutableAndFree();
-            }
-
-            return method.Construct(typeArgsForConstruct);
-        }
-
         internal static bool IsSynthesizedLambda(this MethodSymbol method)
         {
             Debug.Assert((object)method != null);

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

@@ -4,9 +4,11 @@
 using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Globalization;
+using System.Linq;
 using System.Threading;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Text;
 using Roslyn.Utilities;
 
@@ -29,7 +31,7 @@ internal sealed class ReducedExtensionMethodSymbol : MethodSymbol
         /// is applicable, and satisfies type parameter constraints, based on the
         /// "this" argument type. Otherwise, returns null.
         /// </summary>
-        public static MethodSymbol Create(MethodSymbol method, TypeSymbol receiverType, Compilation compilation)
+        public static MethodSymbol Create(MethodSymbol method, TypeSymbol receiverType)
         {
             Debug.Assert(method.IsExtensionMethod && method.MethodKind != MethodKind.ReducedExtension);
             Debug.Assert(method.ParameterCount > 0);
@@ -37,7 +39,7 @@ public static MethodSymbol Create(MethodSymbol method, TypeSymbol receiverType,
 
             HashSet<DiagnosticInfo> useSiteDiagnostics = null;
 
-            method = method.InferExtensionMethodTypeArguments(receiverType, compilation, ref useSiteDiagnostics);
+            method = InferExtensionMethodTypeArguments(method, receiverType, ref useSiteDiagnostics);
             if ((object)method == null)
             {
                 return null;
@@ -96,6 +98,77 @@ private ReducedExtensionMethodSymbol(MethodSymbol reducedFrom)
             _typeArguments = _typeMap.SubstituteTypes(reducedFrom.TypeArgumentsWithAnnotations);
         }
 
+        /// <summary>
+        /// If the extension method is applicable based on the "this" argument type, return
+        /// the method constructed with the inferred type arguments. If the method is not an
+        /// unconstructed generic method, type inference is skipped. If the method is not
+        /// applicable, or if constraints when inferring type parameters from the "this" type
+        /// are not satisfied, the return value is null.
+        /// </summary>
+        private static MethodSymbol InferExtensionMethodTypeArguments(MethodSymbol method, TypeSymbol thisType, ref HashSet<DiagnosticInfo> useSiteDiagnostics)
+        {
+            Debug.Assert(method.IsExtensionMethod);
+            Debug.Assert((object)thisType != null);
+
+            if (!method.IsGenericMethod || method != method.ConstructedFrom)
+            {
+                return method;
+            }
+
+            // We never resolve extension methods on a dynamic receiver.
+            if (thisType.IsDynamic())
+            {
+                return null;
+            }
+
+            var containingAssembly = method.ContainingAssembly;
+            var errorNamespace = containingAssembly.GlobalNamespace;
+            var conversions = new TypeConversions(containingAssembly.CorLibrary);
+
+            // There is absolutely no plausible syntax/tree that we could use for these
+            // synthesized literals.  We could be speculatively binding a call to a PE method.
+            var syntaxTree = CSharpSyntaxTree.Dummy;
+            var syntax = (CSharpSyntaxNode)syntaxTree.GetRoot();
+
+            // Create an argument value for the "this" argument of specific type,
+            // and pass the same bad argument value for all other arguments.
+            var thisArgumentValue = new BoundLiteral(syntax, ConstantValue.Bad, thisType) { WasCompilerGenerated = true };
+            var otherArgumentType = new ExtendedErrorTypeSymbol(errorNamespace, name: string.Empty, arity: 0, errorInfo: null, unreported: false);
+            var otherArgumentValue = new BoundLiteral(syntax, ConstantValue.Bad, otherArgumentType) { WasCompilerGenerated = true };
+
+            var paramCount = method.ParameterCount;
+            var arguments = new BoundExpression[paramCount];
+
+            for (int i = 0; i < paramCount; i++)
+            {
+                var argument = (i == 0) ? thisArgumentValue : otherArgumentValue;
+                arguments[i] = argument;
+            }
+
+            var typeArgs = MethodTypeInferrer.InferTypeArgumentsFromFirstArgument(
+                conversions,
+                method,
+                arguments.AsImmutable(),
+                useSiteDiagnostics: ref useSiteDiagnostics);
+
+            if (typeArgs.IsDefault)
+            {
+                return null;
+            }
+
+            // For the purpose of construction we use original type parameters in place of type arguments that we couldn't infer from the first argument.
+            ImmutableArray<TypeWithAnnotations> typeArgsForConstruct = typeArgs;
+            if (typeArgs.Any(t => !t.HasType))
+            {
+                typeArgsForConstruct = typeArgs.ZipAsArray(
+                    method.TypeParameters,
+                    (t, tp) => t.HasType ? t : TypeWithAnnotations.Create(tp));
+            }
+
+            return method.Construct(typeArgsForConstruct);
+        }
+
+
         internal override MethodSymbol CallsiteReducedFromMethod
         {
             get { return _reducedFrom.ConstructIfGeneric(_typeArguments); }

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

@@ -842,9 +842,9 @@ public static void Main(string[] args)
 }";
             var compilation = CreateCompilationWithMscorlib40AndSystemCore(source);
             compilation.VerifyDiagnostics(
-                // (14,22): error CS1061: 'A' does not contain a definition for 'Extension' and no extension method 'Extension' accepting a first argument of type 'A' could be found (are you missing a using directive or an assembly reference?)
+                // (14,20): error CS8093: Extension method groups are not allowed as an argument to 'nameof'.
                 //         Use(nameof(a.Extension));
-                Diagnostic(ErrorCode.ERR_NoSuchMemberOrExtension, "Extension").WithArguments("A", "Extension").WithLocation(14, 22)
+                Diagnostic(ErrorCode.ERR_NameofExtensionMethod, "a.Extension").WithLocation(14, 20)
                 );
             var tree = compilation.SyntaxTrees[0];
             var model = compilation.GetSemanticModel(tree);
@@ -996,9 +996,9 @@ public static void Main(string[] args)
 ";
             var compilation = CreateCompilationWithMscorlib40AndSystemCore(source);
             compilation.VerifyDiagnostics(
-                // (16,22): error CS1061: 'A' does not contain a definition for 'Extension' and no extension method 'Extension' accepting a first argument of type 'A' could be found (are you missing a using directive or an assembly reference?)
+                // (16,20): error CS8093: Extension method groups are not allowed as an argument to 'nameof'.
                 //         Use(nameof(a.Extension));
-                Diagnostic(ErrorCode.ERR_NoSuchMemberOrExtension, "Extension").WithArguments("A", "Extension").WithLocation(16, 22)
+                Diagnostic(ErrorCode.ERR_NameofExtensionMethod, "a.Extension").WithLocation(16, 20)
                 );
         }
 

src/Compilers/CSharp/Test/Symbol/Compilation/SemanticModelAPITests.cs

@@ -16,6 +16,61 @@ namespace Microsoft.CodeAnalysis.CSharp.UnitTests
 {
     public class SemanticTests : CSharpTestBase
     {
+        [Fact]
+        public void UnmanagedConstraintOnExtensionMethod()
+        {
+            var src = @"
+public static class Ext
+{
+    public static T GenericExtension<T>(ref this T self) where T : unmanaged
+    {
+        return self;
+    }
+}
+";
+            var src2 = @"
+public class Program
+{
+    public struct S<T> where T : unmanaged
+    {
+        public T t;
+    }
+
+    static void M<T>(S<T> s) where T : unmanaged
+    {
+        s.GenericExtension();
+    }
+}";
+            var comp1 = CreateCompilation(src, parseOptions: TestOptions.Regular8);
+
+            var comp2 = CreateCompilation(src2, parseOptions: TestOptions.Regular8,
+                references: new[] { comp1.ToMetadataReference() });
+            comp2.VerifyDiagnostics();
+            Assert.NotNull(checkSymbolInfo(comp2).Symbol);
+
+            comp2 = CreateCompilation(src2, parseOptions: TestOptions.Regular7_3,
+                references: new[] { comp1.ToMetadataReference() });
+            comp2.VerifyDiagnostics(
+                // (11,11): error CS8652: The feature 'unmanaged constructed types' is currently in Preview and *unsupported*. To use Preview features, use the 'preview' language version.
+                //         s.GenericExtension();
+                Diagnostic(ErrorCode.ERR_FeatureInPreview, "GenericExtension").WithArguments("unmanaged constructed types").WithLocation(11, 11));
+
+            var info = checkSymbolInfo(comp2);
+            Assert.Null(info.Symbol);
+            Assert.Equal(CandidateReason.OverloadResolutionFailure, info.CandidateReason);
+
+            static SymbolInfo checkSymbolInfo(CSharpCompilation comp)
+            {
+                SyntaxTree tree = comp.SyntaxTrees[0];
+                var model = comp.GetSemanticModel(tree);
+                SyntaxNode root = tree.GetRoot();
+                var invoke = root.DescendantNodes().OfType<MemberAccessExpressionSyntax>()
+                    .Where(e => e.Name.ToString() == "GenericExtension").First();
+
+                return model.GetSymbolInfo(invoke);
+            }
+        }
+
         [Fact]
         public void PatternIndexAndRangeIndexers()
         {

src/Compilers/CSharp/Test/Symbol/Compilation/SemanticModelGetDeclaredSymbolAPITests.cs

@@ -1938,7 +1938,7 @@ static void M(A a, B b)
 
             // Type not satisfying constraint.
             symbols = model.LookupSymbols(position, container: method.Parameters[1].Type, name: "F", includeReducedExtensionMethods: true);
-            CheckSymbolsUnordered(symbols);
+            CheckSymbolsUnordered(symbols, "void B.F<B>()");
 
             // Same tests as above but with position outside of
             // static class defining extension methods.
@@ -1978,7 +1978,7 @@ static class E
 
             // Type not satisfying constraint.
             symbols = model.LookupSymbols(position, container: method.Parameters[1].Type, name: "F", includeReducedExtensionMethods: true);
-            CheckSymbols(symbols);
+            CheckSymbolsUnordered(symbols, "void B.F<B>()");
         }
 
         [Fact]

src/Compilers/CSharp/Test/Symbol/Symbols/ExtensionMethodTests.cs

@@ -3831,8 +3831,9 @@ public class BaseClass<TMember>
             var baseClass = model.GetTypeInfo(instance).Type;
             Assert.Equal("BaseClass<System.Int32>", baseClass.ToTestDisplayString());
 
-            Assert.Empty(model.LookupSymbols(instance.Position, baseClass, "SetMember", includeReducedExtensionMethods: true));
-            Assert.Empty(model.LookupSymbols(instance.Position, baseClass, includeReducedExtensionMethods: true).Where(s => s.Name == "SetMembers"));
+            var setMember = model.LookupSymbols(instance.Position, baseClass, "SetMember", includeReducedExtensionMethods: true).Single();
+            Assert.Equal("BaseClass<System.Int32> BaseClass<System.Int32>.SetMember<BaseClass<System.Int32>, TMember>(TMember NewValue)", setMember.ToTestDisplayString());
+            Assert.Contains(setMember, model.LookupSymbols(instance.Position, baseClass, includeReducedExtensionMethods: true));
         }
 
         [Fact]
@@ -3926,8 +3927,9 @@ public class BaseClass<TMember> : I1<TMember>
             var baseClass = model.GetTypeInfo(instance).Type;
             Assert.Equal("BaseClass<System.Int32>", baseClass.ToTestDisplayString());
 
-            Assert.Empty(model.LookupSymbols(instance.Position, baseClass, "SetMember", includeReducedExtensionMethods: true));
-            Assert.Empty(model.LookupSymbols(instance.Position, baseClass, includeReducedExtensionMethods: true).Where(s => s.Name == "SetMembers"));
+            var setMember = model.LookupSymbols(instance.Position, baseClass, "SetMember", includeReducedExtensionMethods: true).Single();
+            Assert.Equal("BaseClass<System.Int32> BaseClass<System.Int32>.SetMember<BaseClass<System.Int32>, TMember>(TMember NewValue)", setMember.ToTestDisplayString());
+            Assert.Contains(setMember, model.LookupSymbols(instance.Position, baseClass, includeReducedExtensionMethods: true));
         }
 
         [Fact]