Merge pull request #23438 from VSadov/refFixes2

Several small bug fixes related to readonly references.

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

@@ -615,7 +615,7 @@ private bool CheckFieldValueKind(SyntaxNode node, BoundFieldAccess fieldAccess,
             return CheckIsValidReceiverForVariable(node, fieldAccess.ReceiverOpt, valueKind, diagnostics);
         }
 
-        private static bool CheckFieldRefEscape(SyntaxNode node, BoundFieldAccess fieldAccess, uint escapeFrom, uint escapeTo, bool checkingReceiver, DiagnosticBag diagnostics)
+        private static bool CheckFieldRefEscape(SyntaxNode node, BoundFieldAccess fieldAccess, uint escapeFrom, uint escapeTo, DiagnosticBag diagnostics)
         {
             var fieldSymbol = fieldAccess.FieldSymbol;
             // fields that are static or belong to reference types can ref escape anywhere
@@ -628,7 +628,7 @@ private static bool CheckFieldRefEscape(SyntaxNode node, BoundFieldAccess fieldA
             return CheckRefEscape(node, fieldAccess.ReceiverOpt, escapeFrom, escapeTo, checkingReceiver: true, diagnostics: diagnostics);
         }
 
-        private static bool CheckFieldLikeEventRefEscape(SyntaxNode node, BoundEventAccess eventAccess, uint escapeFrom, uint escapeTo, bool checkingReceiver, DiagnosticBag diagnostics)
+        private static bool CheckFieldLikeEventRefEscape(SyntaxNode node, BoundEventAccess eventAccess, uint escapeFrom, uint escapeTo, DiagnosticBag diagnostics)
         {
             var eventSymbol = eventAccess.EventSymbol;
 
@@ -1858,7 +1858,7 @@ internal static bool CheckRefEscape(SyntaxNode node, BoundExpression expr, uint
 
                 case BoundKind.FieldAccess:
                     var fieldAccess = (BoundFieldAccess)expr;
-                    return CheckFieldRefEscape(node, fieldAccess, escapeFrom, escapeTo, checkingReceiver, diagnostics);
+                    return CheckFieldRefEscape(node, fieldAccess, escapeFrom, escapeTo, diagnostics);
 
                 case BoundKind.EventAccess:
                     var eventAccess = (BoundEventAccess)expr;
@@ -1868,7 +1868,7 @@ internal static bool CheckRefEscape(SyntaxNode node, BoundExpression expr, uint
                         break;
                     }
 
-                    return CheckFieldLikeEventRefEscape(node, eventAccess, escapeFrom, escapeTo, checkingReceiver, diagnostics);
+                    return CheckFieldLikeEventRefEscape(node, eventAccess, escapeFrom, escapeTo, diagnostics);
 
                 case BoundKind.Call:
                     var call = (BoundCall)expr;

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

@@ -7071,9 +7071,12 @@ private BoundConditionalAccess BindConditionalAccessExpression(ConditionalAccess
                 return GenerateBadConditionalAccessNodeError(node, receiver, access, diagnostics);
             }
 
-            // access cannot have unconstrained generic type
-            // access cannot be a pointer
-            if ((!accessType.IsReferenceType && !accessType.IsValueType) || accessType.IsPointerType())
+            // The resulting type must be either a reference type T or Nullable<T>
+            // Therefore we must reject cases resulting in types that are not reference types and cannot be lifted into nullable.
+            // - access cannot have unconstrained generic type
+            // - access cannot be a pointer
+            // - access cannot be a restricted type
+            if ((!accessType.IsReferenceType && !accessType.IsValueType) || accessType.IsPointerType() || accessType.IsRestrictedType())
             {
                 // Result type of the access is void when result value cannot be made nullable.
                 // For improved diagnostics we detect the cases where the value will be used and produce a

src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs

@@ -1481,7 +1481,7 @@ private void EmitCallExpression(BoundCall call, UseKind useKind)
                     {
                         // if method is defined in the struct itself it is assumed to be mutating, unless 
                         // it is a member of a readonly struct and is not a constructor
-                        var receiverAddresskind = methodContainingType.IsReadOnly && method.MethodKind != MethodKind.Constructor ?
+                        var receiverAddresskind = IsReadOnlyCall(method, methodContainingType) ?
                                                                         AddressKind.ReadOnly :
                                                                         AddressKind.Writeable;
                         if (MayUseCallForStructMethod(method))
@@ -1659,6 +1659,30 @@ private void EmitCallExpression(BoundCall call, UseKind useKind)
             FreeOptTemp(tempOpt);
         }
 
+        private bool IsReadOnlyCall(MethodSymbol method, NamedTypeSymbol methodContainingType)
+        {
+            Debug.Assert(methodContainingType.IsVerifierValue(), "only struct calls can be readonly");
+
+            if (methodContainingType.IsReadOnly && method.MethodKind != MethodKind.Constructor)
+            {
+                return true;
+            }
+
+            if (methodContainingType.IsNullableType())
+            {
+                var originalMethod = method.OriginalDefinition;
+
+                if ((object)originalMethod == this._module.Compilation.GetSpecialTypeMember(SpecialMember.System_Nullable_T_GetValueOrDefault) ||
+                    (object)originalMethod == this._module.Compilation.GetSpecialTypeMember(SpecialMember.System_Nullable_T_get_Value) ||
+                    (object)originalMethod == this._module.Compilation.GetSpecialTypeMember(SpecialMember.System_Nullable_T_get_HasValue))
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
         // returns true when receiver is already a ref.
         // in such cases calling through a ref could be preferred over 
         // calling through indirectly loaded value.

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenInParametersTests.cs

@@ -1818,5 +1818,105 @@ static void Main()
     }
 }", expectedOutput: "5-9");
         }
+
+        [WorkItem(23338, "https://github.com/dotnet/roslyn/issues/23338")]
+        [Fact]
+        public void InParamsNullable()
+        {
+            var text = @"
+
+class Program
+{
+    static void Main(string[] args)
+    {
+        S1 s = new S1();
+        s.val = 42;
+
+        S1? ns = s;
+
+        Test1(in ns);
+        Test2(ref ns);
+    }
+
+    static void Test1(in S1? arg)
+    {
+        // cannot not mutate
+        System.Console.Write(arg.GetValueOrDefault());
+        // should not mutate
+        arg.ToString();
+        // cannot not mutate
+        System.Console.Write(arg.GetValueOrDefault());
+    }
+
+    static void Test2(ref S1? arg)
+    {
+        // cannot not mutate
+        System.Console.Write(arg.GetValueOrDefault());
+        // can mutate
+        arg.ToString();
+        // cannot not mutate
+        System.Console.Write(arg.GetValueOrDefault());
+    }
+
+}
+
+struct S1
+{
+    public int val;
+
+    public override string ToString()
+    {
+        var result = val.ToString();
+        val = 0;
+
+        return result;
+    }
+}
+";
+
+            var comp = CompileAndVerify(text, parseOptions: TestOptions.Regular, verify: Verification.Passes, expectedOutput: @"4242420");
+
+            comp.VerifyIL("Program.Test1(in S1?)", @"
+{
+  // Code size       54 (0x36)
+  .maxstack  1
+  .locals init (S1? V_0)
+  IL_0000:  ldarg.0
+  IL_0001:  call       ""S1 S1?.GetValueOrDefault()""
+  IL_0006:  box        ""S1""
+  IL_000b:  call       ""void System.Console.Write(object)""
+  IL_0010:  ldarg.0
+  IL_0011:  ldobj      ""S1?""
+  IL_0016:  stloc.0
+  IL_0017:  ldloca.s   V_0
+  IL_0019:  constrained. ""S1?""
+  IL_001f:  callvirt   ""string object.ToString()""
+  IL_0024:  pop
+  IL_0025:  ldarg.0
+  IL_0026:  call       ""S1 S1?.GetValueOrDefault()""
+  IL_002b:  box        ""S1""
+  IL_0030:  call       ""void System.Console.Write(object)""
+  IL_0035:  ret
+}");
+
+            comp.VerifyIL("Program.Test2(ref S1?)", @"
+{
+  // Code size       46 (0x2e)
+  .maxstack  1
+  IL_0000:  ldarg.0
+  IL_0001:  call       ""S1 S1?.GetValueOrDefault()""
+  IL_0006:  box        ""S1""
+  IL_000b:  call       ""void System.Console.Write(object)""
+  IL_0010:  ldarg.0
+  IL_0011:  constrained. ""S1?""
+  IL_0017:  callvirt   ""string object.ToString()""
+  IL_001c:  pop
+  IL_001d:  ldarg.0
+  IL_001e:  call       ""S1 S1?.GetValueOrDefault()""
+  IL_0023:  box        ""S1""
+  IL_0028:  call       ""void System.Console.Write(object)""
+  IL_002d:  ret
+}");
+        }
     }
 }

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenShortCircuitOperatorTests.cs

@@ -5172,6 +5172,56 @@ .maxstack  1
 }");
         }
 
+        [WorkItem(23422, "https://github.com/dotnet/roslyn/issues/23422")]
+        [Fact]
+        public void ConditionalRefLike()
+        {
+            var source = @"
+class C
+{
+    static void Main()
+    {
+        System.Console.WriteLine(""---"");
+        Goo(new C());
+        System.Console.WriteLine(""---"");
+        Goo(null);
+        System.Console.WriteLine(""---"");
+    }
+
+    static void Goo(C x)
+    {
+        x?.M();
+    }
+
+    public RefLike M()
+    {
+        System.Console.WriteLine(""M"");
+        return default;
+    }
+
+    public ref struct RefLike{}
+}
+";
+            var verifier = CompileAndVerify(source, options: TestOptions.DebugExe.WithAllowUnsafe(true), expectedOutput: @"---
+M
+---
+---");
+
+            verifier.VerifyIL("C.Goo", @"
+{
+  // Code size       14 (0xe)
+  .maxstack  1
+  IL_0000:  nop
+  IL_0001:  ldarg.0
+  IL_0002:  brtrue.s   IL_0006
+  IL_0004:  br.s       IL_000d
+  IL_0006:  ldarg.0
+  IL_0007:  call       ""C.RefLike C.M()""
+  IL_000c:  pop
+  IL_000d:  ret
+}");
+        }
+
         [WorkItem(1109164, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/1109164")]
         [Fact]
         public void Bug1109164_01()

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

@@ -147,6 +147,7 @@ static void Main()
 }";
 
             var verifier = CompileAndVerify(source: source2, expectedOutput: "0");
+            verifier = CompileAndVerify(source: source2, expectedOutput: "0");
 
             // And in fact, this should work if there is an implicit conversion from the result of the addition
             // to the type:
@@ -169,7 +170,8 @@ static void Main()
   }
 }";
 
-            verifier = CompileAndVerify(source: source3, expectedOutput: "1");
+            verifier = CompileAndVerify(source: source3, expectedOutput: "1", verify: Verification.Fails);
+            verifier = CompileAndVerify(source: source3, expectedOutput: "1", parseOptions: TestOptions.RegularLatest.WithPEVerifyCompatFeature());
         }
 
         [Fact, WorkItem(543954, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/543954")]
@@ -227,7 +229,8 @@ static void F(int line, bool b)
 ";
             foreach (string type in new[] { "int", "ushort", "byte", "long", "float", "decimal" })
             {
-                CompileAndVerify(source: source4.Replace("TYPE", type), expectedOutput: "0");
+                CompileAndVerify(source: source4.Replace("TYPE", type), expectedOutput: "0", verify: Verification.Fails);
+                CompileAndVerify(source: source4.Replace("TYPE", type), expectedOutput: "0", parseOptions: TestOptions.RegularLatest.WithPEVerifyCompatFeature());
             }
         }
 

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

@@ -23588,6 +23588,52 @@ static void Main()
                );
         }
 
+        [WorkItem(23422, "https://github.com/dotnet/roslyn/issues/23422")]
+        [Fact]
+        public void ConditionalMemberAccessRefLike()
+        {
+            var text = @"
+class Program
+{
+    static void Main(string[] args)
+    {
+        var o = new Program();
+
+        o?.F(); // this is ok
+
+        var x = o?.F();
+
+        var y = o?.F() ?? default;
+
+        var z = o?.F().field ?? default;
+    }
+
+    S2 F() => throw null;
+}
+
+public ref struct S1
+{
+
+}
+
+public ref struct S2
+{
+    public S1 field;
+}
+";
+            CreateCompilationWithMscorlib45(text, options: TestOptions.ReleaseDll).VerifyDiagnostics(
+                // (10,18): error CS0023: Operator '?' cannot be applied to operand of type 'S2'
+                //         var x = o?.F();
+                Diagnostic(ErrorCode.ERR_BadUnaryOp, "?").WithArguments("?", "S2").WithLocation(10, 18),
+                // (12,18): error CS0023: Operator '?' cannot be applied to operand of type 'S2'
+                //         var y = o?.F() ?? default;
+                Diagnostic(ErrorCode.ERR_BadUnaryOp, "?").WithArguments("?", "S2").WithLocation(12, 18),
+                // (14,18): error CS0023: Operator '?' cannot be applied to operand of type 'S1'
+                //         var z = o?.F().field ?? default;
+                Diagnostic(ErrorCode.ERR_BadUnaryOp, "?").WithArguments("?", "S1").WithLocation(14, 18)
+               );
+        }
+
         [Fact]
         [WorkItem(1179322, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/1179322")]
         public void LabelSameNameAsParameter()

src/Compilers/VisualBasic/Portable/Binding/Binder_Expressions.vb

@@ -1199,12 +1199,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                 Dim resultType As TypeSymbol = access.Type
 
                 If Not resultType.IsErrorType() Then
-                    If resultType.IsValueType Then
+                    If resultType.IsValueType AndAlso Not resultType.IsRestrictedType Then
                         If Not resultType.IsNullableType() Then
                             resultType = GetSpecialType(SpecialType.System_Nullable_T, expr.Syntax, diagnostics).Construct(resultType)
                         End If
                     ElseIf Not resultType.IsReferenceType Then
-                        ' Access cannot have unconstrained generic type
+                        ' Access cannot have unconstrained generic type or a restricted type
                         ReportDiagnostic(diagnostics, access.Syntax, ERRID.ERR_CannotBeMadeNullable1, resultType)
                         resultType = ErrorTypeSymbol.UnknownResultType
                     End If

src/Compilers/VisualBasic/Test/Semantic/Semantics/ConditionalAccessTests.vb

 ' Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
+Imports Microsoft.CodeAnalysis.Test.Utilities
 Imports Microsoft.CodeAnalysis.VisualBasic
 Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
 Imports Roslyn.Test.Utilities
@@ -1141,6 +1142,96 @@ BC37238: 'T' cannot be made nullable.
 </expected>)
         End Sub
 
+        <WorkItem(23422, "https://github.com/dotnet/roslyn/issues/23422")>
+        <Fact()>
+        Public Sub ERR_CannotBeMadeNullable1_2()
+
+            Dim compilationDef =
+<compilation>
+    <file name="a.vb">
+Imports System
+
+Module Module1
+
+    Sub Main()
+        Dim o = New C1
+        Dim x = o?.F ' this should be an error
+    End Sub
+End Module
+
+Public Class C1
+    Public Function F() As TypedReference
+        System.Console.WriteLine("hi")
+        Return Nothing
+    End Function
+End Class
+    </file>
+</compilation>
+
+            Dim compilation = CompilationUtils.CreateCompilationWithMscorlibAndVBRuntime(compilationDef, TestOptions.ReleaseExe)
+
+            AssertTheseDiagnostics(compilation,
+<expected>
+BC37238: 'TypedReference' cannot be made nullable.
+        Dim x = o?.F ' this should be an error
+                  ~~
+</expected>)
+        End Sub
+
+        <WorkItem(23422, "https://github.com/dotnet/roslyn/issues/23422")>
+        <Fact()>
+        Public Sub ERR_CannotBeMadeNullable1_3()
+
+            Dim compilationDef =
+<compilation>
+    <file name="a.vb"><![CDATA[
+Imports System
+
+Module Module1
+
+    Sub Main()
+        Dim o = New C1
+        o?.F() ' this is ok
+    End Sub
+End Module
+
+Public Class C1
+    Public Function F() As TypedReference
+        System.Console.WriteLine("hi")
+        Return Nothing
+    End Function
+End Class
+    ]]></file>
+</compilation>
+
+            Dim compilation = CompilationUtils.CreateCompilationWithMscorlibAndVBRuntime(compilationDef, TestOptions.ReleaseExe)
+
+            ' VB seems to allow methods that return TypedReference, likely for compat reasons
+            ' that is technically not verifiable, but it is not relevant to this test
+            Dim verifier = CompileAndVerify(compilation, verify:=Verification.Fails, expectedOutput:=
+            <![CDATA[
+hi
+]]>)
+
+            verifier.VerifyIL("Module1.Main()",
+            <![CDATA[
+{
+  // Code size       17 (0x11)
+  .maxstack  1
+  .locals init (C1 V_0) //o
+  IL_0000:  newobj     "Sub C1..ctor()"
+  IL_0005:  stloc.0
+  IL_0006:  ldloc.0
+  IL_0007:  brfalse.s  IL_0010
+  IL_0009:  ldloc.0
+  IL_000a:  call       "Function C1.F() As System.TypedReference"
+  IL_000f:  pop
+  IL_0010:  ret
+}
+]]>)
+
+        End Sub
+
         <Fact()>
         Public Sub ERR_UnaryOperand2_1()