Check constraints on lifted operator types (#57050)

Fixes #56646. We now do not add operators to the list of possible operators when lifting would cause invalid type arguments to be supplied to Nullable<T>, rather than erroring after creation. In addition, we now do this for lifted conversions as well, As a part of this, I found and corrected an implementation bug that caused code successfully emitted by the native compiler to emit bad code in Roslyn: we were treating pointer types like value types for the purposes of creating nullable value types, rather than treating them like reference types.

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

@@ -2784,6 +2784,7 @@ internal static uint GetValEscape(BoundExpression expr, uint scopeOfTheContainin
                 case BoundKind.AsOperator:
                 case BoundKind.AwaitExpression:
                 case BoundKind.ConditionalAccess:
+                case BoundKind.ConditionalReceiver:
                 case BoundKind.ArrayAccess:
                     // only possible in error cases (if possible at all)
                     return scopeOfTheContainingExpression;

src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/UserDefinedExplicitConversions.cs

@@ -265,10 +265,10 @@ private void AddUserDefinedConversionsToExplicitCandidateSet(
 
                 if (fromConversion.Exists && toConversion.Exists)
                 {
-                    if ((object)source != null && source.IsNullableType() && convertsFrom.IsNonNullableValueType() && target.CanBeAssignedNull())
+                    if ((object)source != null && source.IsNullableType() && convertsFrom.IsValidNullableTypeArgument() && target.CanBeAssignedNull())
                     {
                         TypeSymbol nullableFrom = MakeNullableType(convertsFrom);
-                        TypeSymbol nullableTo = convertsTo.IsNonNullableValueType() ? MakeNullableType(convertsTo) : convertsTo;
+                        TypeSymbol nullableTo = convertsTo.IsValidNullableTypeArgument() ? MakeNullableType(convertsTo) : convertsTo;
                         Conversion liftedFromConversion = EncompassingExplicitConversion(sourceExpression, source, nullableFrom, ref useSiteInfo);
                         Conversion liftedToConversion = EncompassingExplicitConversion(null, nullableTo, target, ref useSiteInfo);
                         Debug.Assert(liftedFromConversion.Exists);
@@ -290,15 +290,16 @@ private void AddUserDefinedConversionsToExplicitCandidateSet(
                         // though it really were X?-->Y for the purposes of determining the best
                         // source type of a set of operators.
                         //
-                        // We perpetuate these fictions here.
+                        // We perpetuate these fictions here, except when X or Y is not a valid
+                        // type argument to `Nullable<T>`. 
 
-                        if (target.IsNullableType() && convertsTo.IsNonNullableValueType())
+                        if (target.IsNullableType() && convertsTo.IsValidNullableTypeArgument())
                         {
                             convertsTo = MakeNullableType(convertsTo);
                             toConversion = EncompassingExplicitConversion(null, convertsTo, target, ref useSiteInfo);
                         }
 
-                        if ((object)source != null && source.IsNullableType() && convertsFrom.IsNonNullableValueType())
+                        if ((object)source != null && source.IsNullableType() && convertsFrom.IsValidNullableTypeArgument())
                         {
                             convertsFrom = MakeNullableType(convertsFrom);
                             fromConversion = EncompassingExplicitConversion(null, convertsFrom, source, ref useSiteInfo);

src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/UserDefinedImplicitConversions.cs

@@ -304,9 +304,12 @@ void addCandidatesFromType(
                         // actually X-->Y? in source for the purposes of determining the best target
                         // type of an operator.
                         //
-                        // We perpetuate this fiction here.
+                        // We perpetuate this fiction here, except for cases when Y is not a valid type
+                        // argument for Nullable<T>. This scenario should only be possible when the corlib
+                        // defines a type such as int or long to be a ref struct (see
+                        // LiftedConversion_InvalidTypeArgument02).
 
-                        if ((object)target != null && target.IsNullableType() && convertsTo.IsNonNullableValueType())
+                        if ((object)target != null && target.IsNullableType() && convertsTo.IsValidNullableTypeArgument())
                         {
                             convertsTo = MakeNullableType(convertsTo);
                             toConversion = allowAnyTarget ? Conversion.Identity :
@@ -315,7 +318,7 @@ void addCandidatesFromType(
 
                         u.Add(UserDefinedConversionAnalysis.Normal(constrainedToTypeOpt, op, fromConversion, toConversion, convertsFrom, convertsTo));
                     }
-                    else if ((object)source != null && source.IsNullableType() && convertsFrom.IsNonNullableValueType() &&
+                    else if ((object)source != null && source.IsNullableType() && convertsFrom.IsValidNullableTypeArgument() &&
                         (allowAnyTarget || target.CanBeAssignedNull()))
                     {
                         // As mentioned above, here we diverge from the specification, in two ways.
@@ -334,7 +337,7 @@ void addCandidatesFromType(
                         // If the answer to all those questions is "yes" then we lift to nullable
                         // and see if the resulting operator is applicable.
                         TypeSymbol nullableFrom = MakeNullableType(convertsFrom);
-                        TypeSymbol nullableTo = convertsTo.IsNonNullableValueType() ? MakeNullableType(convertsTo) : convertsTo;
+                        TypeSymbol nullableTo = convertsTo.IsValidNullableTypeArgument() ? MakeNullableType(convertsTo) : convertsTo;
                         Conversion liftedFromConversion = EncompassingImplicitConversion(sourceExpression, source, nullableFrom, ref useSiteInfo);
                         Conversion liftedToConversion = !allowAnyTarget ?
                             EncompassingImplicitConversion(null, nullableTo, target, ref useSiteInfo) :

src/Compilers/CSharp/Portable/Binder/Semantics/Operators/BinaryOperatorOverloadResolution.cs

@@ -929,10 +929,8 @@ private static LiftingResult UserDefinedBinaryOperatorCanBeLifted(TypeSymbol lef
             // SPEC: types and if the result type is bool. The lifted form is 
             // SPEC: constructed by adding a single ? modifier to each operand type.
 
-            if (!left.IsValueType ||
-                left.IsNullableType() ||
-                !right.IsValueType ||
-                right.IsNullableType())
+            if (!left.IsValidNullableTypeArgument() ||
+                !right.IsValidNullableTypeArgument())
             {
                 return LiftingResult.NotLifted;
             }
@@ -953,7 +951,7 @@ private static LiftingResult UserDefinedBinaryOperatorCanBeLifted(TypeSymbol lef
                         LiftingResult.LiftOperandsButNotResult :
                         LiftingResult.NotLifted;
                 default:
-                    return result.IsValueType && !result.IsNullableType() ?
+                    return result.IsValidNullableTypeArgument() ?
                         LiftingResult.LiftOperandsAndResult :
                         LiftingResult.NotLifted;
             }

src/Compilers/CSharp/Portable/Binder/Semantics/Operators/UnaryOperatorOverloadResolution.cs

@@ -500,8 +500,8 @@ private void GetUserDefinedUnaryOperatorsFromType(
                     case UnaryOperatorKind.PostfixIncrement:
                     case UnaryOperatorKind.LogicalNegation:
                     case UnaryOperatorKind.BitwiseComplement:
-                        if (operandType.IsValueType && !operandType.IsNullableType() &&
-                            resultType.IsValueType && !resultType.IsNullableType())
+                        if (operandType.IsValidNullableTypeArgument() &&
+                            resultType.IsValidNullableTypeArgument())
                         {
                             operators.Add(new UnaryOperatorSignature(
                                 UnaryOperatorKind.Lifted | UnaryOperatorKind.UserDefined | kind,

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Conversion.cs

@@ -1052,7 +1052,7 @@ private BoundExpression RewriteUserDefinedConversion(
 
         private BoundExpression MakeLiftedUserDefinedConversionConsequence(BoundCall call, TypeSymbol resultType)
         {
-            if (call.Method.ReturnType.IsNonNullableValueType())
+            if (call.Method.ReturnType.IsValidNullableTypeArgument())
             {
                 Debug.Assert(resultType.IsNullableType() && TypeSymbol.Equals(resultType.GetNullableUnderlyingType(), call.Method.ReturnType, TypeCompareKind.ConsiderEverything2));
                 MethodSymbol ctor = UnsafeGetNullableMethod(call.Syntax, resultType, SpecialMember.System_Nullable_T__ctor);

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

@@ -132,6 +132,14 @@ public static bool IsNullableType(this TypeSymbol type)
             return type.OriginalDefinition.SpecialType == SpecialType.System_Nullable_T;
         }
 
+        public static bool IsValidNullableTypeArgument(this TypeSymbol type)
+        {
+            return type is { IsValueType: true }
+                   && !type.IsNullableType()
+                   && !type.IsPointerOrFunctionPointer()
+                   && !type.IsRestrictedType();
+        }
+
         public static TypeSymbol GetNullableUnderlyingType(this TypeSymbol type)
         {
             return type.GetNullableUnderlyingTypeWithAnnotations().Type;

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

@@ -11214,5 +11214,119 @@ static unsafe void M(dynamic d, int* p)
                 Diagnostic(ErrorCode.ERR_BadBinaryOps, "d += p").WithArguments("+=", "dynamic", "int*").WithLocation(5, 9)
                 );
         }
+
+        [Fact, WorkItem(56646, "https://github.com/dotnet/roslyn/issues/56646")]
+        public void LiftedUnaryOperator_InvalidTypeArgument01()
+        {
+            var code = @"
+S1? s1 = default;
+var s2 = +s1;
+
+struct S1
+{
+    public static S2 operator+(S1 s1) => throw null;
+}
+
+ref struct S2 {}
+";
+
+            var comp = CreateCompilation(code);
+            comp.VerifyDiagnostics(
+                // (3,10): error CS0023: Operator '+' cannot be applied to operand of type 'S1?'
+                // var s2 = +s1;
+                Diagnostic(ErrorCode.ERR_BadUnaryOp, "+s1").WithArguments("+", "S1?").WithLocation(3, 10)
+            );
+        }
+
+        [Fact, WorkItem(56646, "https://github.com/dotnet/roslyn/issues/56646")]
+        public void LiftedUnaryOperator_InvalidTypeArgument02()
+        {
+            var code = @"
+S1? s1 = default;
+var s2 = +s1;
+
+unsafe struct S1
+{
+    public static unsafe int* operator+(S1 s1) => throw null;
+}
+";
+
+            var comp = CreateCompilation(code, options: TestOptions.UnsafeReleaseExe);
+            comp.VerifyDiagnostics(
+                // (3,10): error CS0023: Operator '+' cannot be applied to operand of type 'S1?'
+                // var s2 = +s1;
+                Diagnostic(ErrorCode.ERR_BadUnaryOp, "+s1").WithArguments("+", "S1?").WithLocation(3, 10)
+            );
+        }
+
+        [Fact, WorkItem(56646, "https://github.com/dotnet/roslyn/issues/56646")]
+        public void LiftedBinaryOperator_InvalidTypeArgument01()
+        {
+            var code = @"
+var x = new S1();
+int? y = 1;
+(x + y)?.M();
+
+public readonly ref struct S1
+{
+    public static S1 operator+ (S1 x, int y) => throw null;
+    public void M() {}
+}
+";
+
+            var comp = CreateCompilation(code);
+            comp.VerifyDiagnostics(
+                // (4,2): error CS0019: Operator '+' cannot be applied to operands of type 'S1' and 'int?'
+                // (x + y)?.M();
+                Diagnostic(ErrorCode.ERR_BadBinaryOps, "x + y").WithArguments("+", "S1", "int?").WithLocation(4, 2)
+            );
+        }
+
+        [Fact, WorkItem(56646, "https://github.com/dotnet/roslyn/issues/56646")]
+        public void LiftedBinaryOperator_InvalidTypeArgument02()
+        {
+            var code = @"
+var x = new S1();
+int? y = 1;
+(y + x)?.M();
+
+public readonly ref struct S1
+{
+    public static S1 operator+ (int y, S1 x) => throw null;
+    public void M() {}
+}
+";
+
+            var comp = CreateCompilation(code);
+            comp.VerifyDiagnostics(
+                // (4,2): error CS0019: Operator '+' cannot be applied to operands of type 'int?' and 'S1'
+                // (y + x)?.M();
+                Diagnostic(ErrorCode.ERR_BadBinaryOps, "y + x").WithArguments("+", "int?", "S1").WithLocation(4, 2)
+            );
+        }
+
+        [Fact, WorkItem(56646, "https://github.com/dotnet/roslyn/issues/56646")]
+        public void LiftedBinaryOperator_InvalidTypeArgument03()
+        {
+            var code = @"
+var x = new S1();
+int? y = 1;
+(y > x).ToString();
+
+public readonly ref struct S1
+{
+    public static bool operator >(int y, S1 x) => throw null;
+    public static bool operator <(int y, S1 x) => throw null;
+    public void M() {}
+}
+";
+
+            var comp = CreateCompilation(code);
+            comp.VerifyDiagnostics(
+                // (4,2): error CS0019: Operator '>' cannot be applied to operands of type 'int?' and 'S1'
+                // (y > x).ToString();
+                Diagnostic(ErrorCode.ERR_BadBinaryOps, "y > x").WithArguments(">", "int?", "S1").WithLocation(4, 2)
+            );
+        }
     }
 }