From d56269e3fc3d8a4b3e50de469fdcd69bac5cd15f Mon Sep 17 00:00:00 2001
From: =?utf8?q?Michal=20Strehovsk=C3=BD?=
 <MichalStrehovsky@users.noreply.github.com>
Date: Wed, 22 May 2019 15:59:43 -0700
Subject: [PATCH] Allow CORINFO_BOX_THIS for primitives and enums
 (dotnet/coreclr#24644)

We abort R2R compiling methods with `thisTransform == CORINFO_BOX_THIS`. This means we don't R2R compile some methods that do virtual calls on valuetypes (e.g. calling `ToString` on a struct that doesn't itself provide a `ToString` method).

We can't allow this in general, but enums and primitives should be fine.

Commit migrated from https://github.com/dotnet/coreclr/commit/85e4ce49ecdcfa51d4c1d7fd9ab9b57658ba92fa
---
 src/coreclr/src/zap/zapinfo.cpp                | 11 ++++++++++-
 src/coreclr/tests/src/readytorun/tests/main.cs |  9 +++++++++
 src/coreclr/tests/src/readytorun/tests/test.cs | 13 +++++++++++++
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/src/coreclr/src/zap/zapinfo.cpp b/src/coreclr/src/zap/zapinfo.cpp
index 6d66a56..23fb336 100644
--- a/src/coreclr/src/zap/zapinfo.cpp
+++ b/src/coreclr/src/zap/zapinfo.cpp
@@ -2152,7 +2152,16 @@ void ZapInfo::getCallInfo(CORINFO_RESOLVED_TOKEN * pResolvedToken,
         if (pResult->thisTransform == CORINFO_BOX_THIS)
         {
             // READYTORUN: FUTURE: Optionally create boxing stub at runtime
-            ThrowHR(E_NOTIMPL);
+            // We couldn't resolve the constrained call into a valuetype instance method and we're asking the JIT
+            // to box and do a virtual dispatch. If we were to allow the boxing to happen now, it could break future code
+            // when the user adds a method to the valuetype that makes it possible to avoid boxing (if there is state
+            // mutation in the method).
+            
+            // We allow this at least for primitives and enums because we control them
+            // and we know there's no state mutation.
+            CorInfoType constrainedType = getTypeForPrimitiveValueClass(pConstrainedResolvedToken->hClass);
+            if (constrainedType == CORINFO_TYPE_UNDEF)
+                ThrowHR(E_NOTIMPL);
         }
     }
 
diff --git a/src/coreclr/tests/src/readytorun/tests/main.cs b/src/coreclr/tests/src/readytorun/tests/main.cs
index 0223a00..bcadc37 100644
--- a/src/coreclr/tests/src/readytorun/tests/main.cs
+++ b/src/coreclr/tests/src/readytorun/tests/main.cs
@@ -59,6 +59,15 @@ class Program
          var iface = (IMyInterface)o;
          Assert.AreEqual(iface.InterfaceMethod(" "), "Interface result");
          Assert.AreEqual(MyClass.TestInterfaceMethod(iface, "+"), "Interface+result");
+
+        // V2 adds override of ToString
+        if (typeof(MyStructWithVirtuals).GetMethod("ToString").DeclaringType == typeof(MyStructWithVirtuals))
+        {
+            // Make sure the constrained call to ToString doesn't box
+            var mystruct = new MyStructWithVirtuals();
+            mystruct.ToString();
+            Assert.AreEqual(mystruct.X, "Overriden");
+        }
     }
 
     static void TestMovedVirtualMethods()
diff --git a/src/coreclr/tests/src/readytorun/tests/test.cs b/src/coreclr/tests/src/readytorun/tests/test.cs
index 8a6beae..508b369 100644
--- a/src/coreclr/tests/src/readytorun/tests/test.cs
+++ b/src/coreclr/tests/src/readytorun/tests/test.cs
@@ -389,6 +389,19 @@ public struct MyChangingHFAStruct
     }
 }
 
+public struct MyStructWithVirtuals
+{
+    public string X;
+
+#if V2
+    public override string ToString()
+    {
+        X = "Overriden";
+        return base.ToString();
+    }
+#endif
+}
+
 public class ByteBaseClass : List<byte>
 {
     public byte BaseByte;
-- 
2.7.4