Allow reabstraction of default interface methods

Documentation/design-docs/default-interface-methods.md

@@ -41,12 +41,13 @@ The algorithm is amended as follows:
     * If the interface method itself is not abstract, add it to the list.
     * Apply all MethodImpls specified in the list of interfaces implicitly implemented by the runtime class of the instance through which the interface method is invoked and add the methods to the list.
     * Go over the owning types of each of the candidate methods in the list. If the owning type is less concrete than some other type in the list (there is another method in the list whose owning type requires the less concrete type), remove it from the list.
-    * If there's more than one method in the list, throw NotSupportedException
-    * If there's exactly one method in the list call that method
+    * If there's more than one method in the list, throw AmbiguousImplementationException
+    * If there's exactly one method in the list and the method is not abstract, call that method
+    * If there's exactly one method in the list but the method is abstract, throw `EntryPointNotFoundException`.
     * If there's no method in the list and the interface is variant, repeat the above algorithm, looking for a variant match. Return the first variant match provided by a most specific interface.
 
 **Section** "III.2.1 constrained. prefix" the paragraph starting with "This last case can only occur when method was defined on `System.Object`, `System.ValueType`, or `System.Enum`" is extended to also cover default interface method implementation. In the case the interface method implementation is provided by an interface, the implicit boxing becomes _observable_ to the program.
 
-**Section** "III.4.2 callvirt" is extended to allow throwing `AmbiguousImplementationException` if the implementation of the interface method resolves at runtime to more than one default interface method.
+**Section** "III.4.2 callvirt" is extended to allow throwing `AmbiguousImplementationException` if the implementation of the interface method resolves at runtime to more than one default interface method. It's also extended to specify throwing `EntryPointNotFoundException` if the default interface implementation is abstract.
 
-**Section** "III.4.18 ldvirtftn" is extended to allow throwing `AmbiguousImplementationException` if the implementation of the interface method resolves at runtime to more than one default interface method.
+**Section** "III.4.18 ldvirtftn" is extended to allow throwing `AmbiguousImplementationException` if the implementation of the interface method resolves at runtime to more than one default interface method. It's also extended to specify throwing `EntryPointNotFoundException` if the default interface implementation is abstract.

src/dlls/mscorrc/mscorrc.rc

@@ -732,6 +732,7 @@ BEGIN
     IDS_CLASSLOAD_MI_FINAL_IMPL             "Method implementation on an interface '%1' from assembly '%2' must be a final method."
     IDS_CLASSLOAD_AMBIGUOUS_OVERRIDE        "Could not call method '%1' on interface '%2' with type '%3' from assembly '%4' because there are multiple incompatible interface methods overriding this method."
     IDS_CLASSLOAD_UNSUPPORTED_DISPATCH      "Could not make constrained call to method '%1' on interface '%2' with type '%3' from assembly '%4'. Dispatch to default interface methods is not supported in this situation."
+    IDS_CLASSLOAD_METHOD_NOT_IMPLEMENTED    "Could not call method '%1' on type '%2' with an instance of '%3' from assembly '%4' because there is no implementation for the method."
 
     IDS_CLASSLOAD_MISSINGMETHODRVA          "Could not load type '%1' from assembly '%2' because the method '%3' has no implementation (no RVA)."
     SECURITY_E_INCOMPATIBLE_EVIDENCE        "Assembly '%1' already loaded without additional security evidence."

src/dlls/mscorrc/resource.h

@@ -462,6 +462,7 @@
 #define IDS_CLASSLOAD_MI_FINAL_IMPL             0x1ac8
 #define IDS_CLASSLOAD_AMBIGUOUS_OVERRIDE        0x1ac9
 #define IDS_CLASSLOAD_UNSUPPORTED_DISPATCH      0x1aca
+#define IDS_CLASSLOAD_METHOD_NOT_IMPLEMENTED    0x1acb
 
 #define BFA_INVALID_TOKEN_TYPE                  0x2001
 #define BFA_INVALID_TOKEN                       0x2003

src/vm/methodtable.cpp

@@ -6870,6 +6870,39 @@ BOOL MethodTable::FindDispatchEntry(UINT32 typeID,
     RETURN (FALSE);
 }
 
+#ifndef DACCESS_COMPILE
+
+void ThrowExceptionForAbstractOverride(
+    MethodTable *pTargetClass,
+    MethodTable *pInterfaceMT,
+    MethodDesc *pInterfaceMD)
+{
+    LIMITED_METHOD_CONTRACT;
+
+    SString assemblyName;
+
+    pTargetClass->GetAssembly()->GetDisplayName(assemblyName);
+
+    SString strInterfaceName;
+    TypeString::AppendType(strInterfaceName, TypeHandle(pInterfaceMT));
+
+    SString strMethodName;
+    TypeString::AppendMethod(strMethodName, pInterfaceMD, pInterfaceMD->GetMethodInstantiation());
+
+    SString strTargetClassName;
+    TypeString::AppendType(strTargetClassName, pTargetClass);
+
+    COMPlusThrow(
+        kEntryPointNotFoundException,
+        IDS_CLASSLOAD_METHOD_NOT_IMPLEMENTED,
+        strMethodName,
+        strInterfaceName,
+        strTargetClassName,
+        assemblyName);
+}
+
+#endif // !DACCESS_COMPILE
+
 //==========================================================================================
 // Possible cases:
 //      1. Typed (interface) contract
@@ -6989,15 +7022,32 @@ MethodTable::FindDispatchImpl(
 
                 if (foundDefaultInterfaceImplementation)
                 {
-                    // Now, construct a DispatchSlot to return in *pImplSlot
-                    DispatchSlot ds(pDefaultMethod->GetMethodEntryPoint());
-
-                    if (pImplSlot != NULL)
+                    //
+                    // If the default implementation we found is abstract, we hit a reabstraction.
+                    //
+                    // interface IFoo { void Frob() { ... } }
+                    // interface IBar { abstract void IFoo.Frob() }
+                    // class Foo : IBar { /* IFoo.Frob not implemented here */ }
+                    //
+                    if (pDefaultMethod->IsAbstract())
                     {
-                        *pImplSlot = ds;
+                        if (throwOnConflict)
+                        {
+                            ThrowExceptionForAbstractOverride(this, pIfcMT, pIfcMD);
+                        }
                     }
+                    else
+                    {
+                        // Now, construct a DispatchSlot to return in *pImplSlot
+                        DispatchSlot ds(pDefaultMethod->GetMethodEntryPoint());
 
-                    RETURN(TRUE);
+                        if (pImplSlot != NULL)
+                        {
+                            *pImplSlot = ds;
+                        }
+
+                        RETURN(TRUE);
+                    }
                 }
             }
 

src/vm/methodtablebuilder.cpp

@@ -10772,6 +10772,18 @@ BOOL MethodTableBuilder::HasDefaultInterfaceImplementation(bmtRTType *pDeclType,
     if (!pDeclMD->IsAbstract())
         return TRUE;
 
+    // If the method is an abstract MethodImpl, this is a reabstraction:
+    //
+    // interface IFoo { void Frob() { } }
+    // interface IBar : IFoo { abstract void IFoo.Frob() }
+    //
+    // We don't require these to have an implementation because they're final anyway.
+    if (pDeclMD->IsMethodImpl())
+    {
+        assert(pDeclMD->IsFinal());
+        return TRUE;
+    }
+
     int targetSlot = pDeclMD->GetSlot();
 
     // Iterate over all the interfaces this type implements

src/vm/runtimehandles.cpp

@@ -1163,7 +1163,7 @@ MethodDesc* QCALLTYPE RuntimeTypeHandle::GetInterfaceMethodImplementation(Enregi
         // with at least an abstract method. b19897_GetInterfaceMap_Abstract.exe tests this case.
         //@TODO:STUBDISPATCH: Don't need to track down the implementation, just the declaration, and this can
         //@TODO:              be done faster - just need to make a function FindDispatchDecl.
-        DispatchSlot slot(typeHandle.GetMethodTable()->FindDispatchSlotForInterfaceMD(thOwnerOfMD, pMD, TRUE /* throwOnConflict */));
+        DispatchSlot slot(typeHandle.GetMethodTable()->FindDispatchSlotForInterfaceMD(thOwnerOfMD, pMD, FALSE /* throwOnConflict */));
     if (!slot.IsNull())
             pResult = slot.GetMethodDesc();
 

tests/src/Loader/classloader/DefaultInterfaceMethods/reabstraction/reabstraction.il

@@ -0,0 +1,218 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+//
+
+.assembly extern mscorlib { }
+
+.assembly reabstraction { }
+
+// Interface with a default method
+.class interface public abstract auto ansi I1
+{
+  .method public hidebysig newslot virtual
+          instance int32 Add(int32 x) cil managed
+  {
+    ldc.i4.1
+    ldarg.1
+    add
+    ret
+  }
+}
+
+// Interface that reabstracts the default method
+.class interface public abstract auto ansi I2
+  implements I1
+{
+  .method public hidebysig newslot virtual abstract final
+          instance int32 Add(int32 x) cil managed
+  {
+    .override I1::Add
+  }
+}
+
+// Interface that overrides reabstracted method
+.class interface public abstract auto ansi I3
+  implements I2
+{
+  .method public hidebysig newslot virtual final
+          instance int32 Add(int32 x) cil managed
+  {
+    .override I1::Add
+    ldc.i4.2
+    ldarg.1
+    add
+    ret
+  }
+}
+
+// Interface that overrides normal default method
+.class interface public abstract auto ansi I4
+  implements I1
+{
+  .method public hidebysig newslot virtual final
+          instance int32 Add(int32 x) cil managed
+  {
+    .override I1::Add
+    ldc.i4.3
+    ldarg.1
+    add
+    ret
+  }
+}
+
+// Plain old interface
+.class interface public abstract auto ansi I5
+{
+  .method public hidebysig newslot virtual abstract
+          instance int32 Add(int32 x) cil managed
+  {
+  }
+}
+
+// Interface that re-abstracts an already abstract method
+.class interface public abstract auto ansi I6
+  implements I5
+{
+  .method public hidebysig newslot virtual abstract final
+          instance int32 Add(int32 x) cil managed
+  {
+    .override I5::Add
+  }
+}
+
+// Class that doesn't implement reabstracted method
+.class public auto ansi C1
+  extends [mscorlib]System.Object
+  implements I2
+{
+  .method public hidebysig specialname rtspecialname 
+          instance void .ctor() cil managed
+  {
+    ldarg.0
+    call instance void [mscorlib]System.Object::.ctor()
+    ret
+  }
+}
+
+// Class that implements reabstracted method
+.class public auto ansi C2
+  extends [mscorlib]System.Object
+  implements I3
+{
+  .method public hidebysig specialname rtspecialname 
+          instance void .ctor() cil managed
+  {
+    ldarg.0
+    call instance void [mscorlib]System.Object::.ctor()
+    ret
+  }
+}
+
+// Class with a diamond between reabstracted and implemented default method
+.class public auto ansi C3
+  extends [mscorlib]System.Object
+  implements I2, I4
+{
+  .method public hidebysig specialname rtspecialname 
+          instance void .ctor() cil managed
+  {
+    ldarg.0
+    call instance void [mscorlib]System.Object::.ctor()
+    ret
+  }
+}
+
+// Class that doesn't implement interface method that also never had an implementation
+.class public auto ansi C4
+  extends [mscorlib]System.Object
+  implements I6
+{
+  .method public hidebysig specialname rtspecialname 
+          instance void .ctor() cil managed
+  {
+    ldarg.0
+    call instance void [mscorlib]System.Object::.ctor()
+    ret
+  }
+}
+
+.method private hidebysig static int32 Main() cil managed
+{
+  .entrypoint
+
+  .try
+  {
+    newobj instance void C1::.ctor()
+    ldc.i4.0
+    callvirt instance int32 I1::Add(int32)
+    pop
+    leave Fail
+  }
+  catch [mscorlib]System.EntryPointNotFoundException
+  {
+    pop
+    leave PureVirtualOK
+  }
+PureVirtualOK:
+
+  .try
+  {
+    newobj instance void C1::.ctor()
+    dup
+    ldvirtftn instance int32 I1::Add(int32)
+    newobj instance void class [mscorlib]System.Func`2<int32,int32>::.ctor(object,
+                                                                           native int)
+    pop
+    leave Fail
+  }
+  catch [mscorlib]System.EntryPointNotFoundException
+  {
+    pop
+    leave PureVirtualDelegateOK
+  }
+PureVirtualDelegateOK:
+
+  newobj instance void C2::.ctor()
+  ldc.i4.0
+  callvirt instance int32 I1::Add(int32)
+  ldc.i4.2
+  bne.un Fail
+
+  .try
+  {
+    newobj instance void C3::.ctor()
+    ldc.i4.0
+    callvirt instance int32 I1::Add(int32)
+    pop
+    leave Fail
+  }
+  catch [System.Private.CoreLib]System.Runtime.AmbiguousImplementationException
+  {
+    pop
+    leave DiamondCaseOK
+  }
+DiamondCaseOK:
+
+  .try
+  {
+    newobj instance void C4::.ctor()
+    ldc.i4.0
+    callvirt instance int32 I5::Add(int32)
+    pop
+    leave Fail
+  }
+  catch [mscorlib]System.EntryPointNotFoundException
+  {
+    pop
+    leave NeverImplementedOK
+  }
+NeverImplementedOK:
+
+  ldc.i4 100
+  ret
+
+Fail:
+  ldc.i4.m1
+  ret
+}

tests/src/Loader/classloader/DefaultInterfaceMethods/reabstraction/reabstraction.ilproj

@@ -0,0 +1,35 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+  <PropertyGroup>
+    <AssemblyName>$(MSBuildProjectName)</AssemblyName>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <SchemaVersion>2.0</SchemaVersion>
+    <ProjectGuid>{85DFC527-4DB1-595E-A7D7-E94EE1F8140D}</ProjectGuid>
+    <FileAlignment>512</FileAlignment>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <ReferenceLocalMscorlib>true</ReferenceLocalMscorlib>
+    <OutputType>Exe</OutputType>
+    <CLRTestKind>BuildAndRun</CLRTestKind>
+    <CLRTestPriority>0</CLRTestPriority>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+
+  <ItemGroup>
+    <Compile Include="reabstraction.il" />
+  </ItemGroup>
+
+
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+</Project>