Put default interfaces behind a define (#15358)

This is needed so that we can turn default interfaces off in release branches. I can't find a central location where the PRERELEASE flag could be defined because native and managed builds seem to be completely disconnected. To limit the risk of only flipping the flag in one build type, I'm adding a test that verifies being able to load an interface with default methods matches what RuntimeFeature says.
dotnet · Dec 5, 2017 · 809b8f7 · 809b8f7
1 parent d714acd
commit 809b8f7
Show file tree

Hide file tree

Showing 16 changed files with 146 additions and 118 deletions.
diff --git a/clr.defines.targets b/clr.defines.targets
@@ -1,4 +1,10 @@
 <Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+
+    <!-- Features we're currently flighting, but don't intend to ship in officially supported releases -->
+    <PropertyGroup Condition="'$(IsPrerelease)' == 'true'">
+        <FeatureDefaultInterfaces>true</FeatureDefaultInterfaces>
+    </PropertyGroup>
+
     <PropertyGroup>
         <DefineConstants Condition="'$(FeatureAppX)' == 'true'">$(DefineConstants);FEATURE_APPX</DefineConstants>
         <DefineConstants Condition="'$(FeatureArrayStubAsIL)' == 'true'">$(DefineConstants);FEATURE_ARRAYSTUB_AS_IL</DefineConstants>
@@ -17,6 +23,7 @@
         <DefineConstants Condition="'$(FeatureXplatEventSource)' == 'true'">$(DefineConstants);FEATURE_EVENTSOURCE_XPLAT</DefineConstants>
         <DefineConstants Condition="'$(FeatureUseLcid)' == 'true'">$(DefineConstants);FEATURE_USE_LCID</DefineConstants>
         <DefineConstants Condition="'$(FeatureWin32Registry)' == 'true'">$(DefineConstants);FEATURE_WIN32_REGISTRY</DefineConstants>
+        <DefineConstants Condition="'$(FeatureDefaultInterfaces)' == 'true'">$(DefineConstants);FEATURE_DEFAULT_INTERFACES</DefineConstants>
 
         <DefineConstants Condition="'$(ProfilingSupportedBuild)' == 'true'">$(DefineConstants);PROFILING_SUPPORTED</DefineConstants>
 

diff --git a/clrdefinitions.cmake b/clrdefinitions.cmake
@@ -1,5 +1,14 @@
 include(clrfeatures.cmake)
 
+# If set, indicates that this is not an officially supported release
+# Keep in sync with IsPrerelease in dir.props
+set(PRERELEASE 1)
+
+# Features we're currently flighting, but don't intend to ship in officially supported releases
+if (PRERELEASE)
+  add_definitions(-DFEATURE_DEFAULT_INTERFACES=1)  
+endif (PRERELEASE)
+
 if (CLR_CMAKE_TARGET_ARCH_AMD64)
   if (CLR_CMAKE_PLATFORM_UNIX)
     add_definitions(-DDBG_TARGET_AMD64_UNIX)

diff --git a/dir.props b/dir.props
@@ -140,6 +140,11 @@
     <PackageThirdPartyNoticesFile>$(ProjectDir)THIRD-PARTY-NOTICES.TXT</PackageThirdPartyNoticesFile>
     <SyncInfoDirectory>$(BaseIntermediateOutputPath)</SyncInfoDirectory>
 
+    <!-- If true, indicates that this is not an officially supported release -->
+    <!-- It is important to flip this to false in official release branches -->
+    <!-- Keep it in sync with PRERELEASE in clrdefinitions.cmake -->
+    <IsPrerelease>true</IsPrerelease>
+
     <!-- This should be kept in sync with package details in src/.nuget/init/project.json -->
     <RuntimeIdGraphDefinitionVersion>1.0.2-beta-24224-02</RuntimeIdGraphDefinitionVersion>
     <RuntimeIdGraphDefinitionFile>$(PackagesDir)/microsoft.netcore.platforms/$(RuntimeIdGraphDefinitionVersion)/runtime.json</RuntimeIdGraphDefinitionFile>

diff --git a/src/inc/clrconfigvalues.h b/src/inc/clrconfigvalues.h
@@ -874,7 +874,6 @@ RETAIL_CONFIG_DWORD_INFO(UNSUPPORTED_TieredCompilation, W("EXPERIMENTAL_TieredCo
 // TypeLoader
 // 
 CONFIG_DWORD_INFO(INTERNAL_TypeLoader_InjectInterfaceDuplicates, W("INTERNAL_TypeLoader_InjectInterfaceDuplicates"), 0, "Injects duplicates in interface map for all types.")
-RETAIL_CONFIG_DWORD_INFO(UNSUPPORTED_TypeLoader_DefaultInterfaces, W("UNSUPPORTED_TypeLoader_DefaultInterfaces"), 0, "Enables support for default interfaces.")
 
 // 
 // Virtual call stubs

diff --git a/src/mscorlib/shared/System/Runtime/CompilerServices/RuntimeFeature.cs b/src/mscorlib/shared/System/Runtime/CompilerServices/RuntimeFeature.cs
@@ -11,9 +11,12 @@ public static class RuntimeFeature
         /// </summary>
         public const string PortablePdb = nameof(PortablePdb);
 
+#if FEATURE_DEFAULT_INTERFACES
+        /// <summary>
         /// Indicates that this version of runtime supports default interface method implementations.
         /// </summary>
         public const string DefaultImplementationsOfInterfaces = nameof(DefaultImplementationsOfInterfaces);
+#endif
 
         /// <summary>
         /// Checks whether a certain feature is supported by the Runtime.
@@ -23,7 +26,9 @@ public static bool IsSupported(string feature)
             switch (feature)
             {
                 case PortablePdb:
+#if FEATURE_DEFAULT_INTERFACES
                 case DefaultImplementationsOfInterfaces:
+#endif
                     return true;
             }
 

diff --git a/src/vm/classcompat.cpp b/src/vm/classcompat.cpp
@@ -2612,25 +2612,27 @@ VOID    MethodTableBuilder::EnumerateClassMethods()
             }
         }
 
+#ifndef FEATURE_DEFAULT_INTERFACES
         // Some interface checks.
         if (fIsClassInterface)
         {
             if (IsMdVirtual(dwMemberAttrs))
             {
-                if (!IsMdAbstract(dwMemberAttrs) && (CLRConfig::GetConfigValue(CLRConfig::UNSUPPORTED_TypeLoader_DefaultInterfaces) == 0))
+                if (!IsMdAbstract(dwMemberAttrs))
                 {
                     BuildMethodTableThrowException(BFA_VIRTUAL_NONAB_INT_METHOD);
                 }
             } 
             else
             {
                 // Instance field/method
-                if (!IsMdStatic(dwMemberAttrs) && (CLRConfig::GetConfigValue(CLRConfig::UNSUPPORTED_TypeLoader_DefaultInterfaces) == 0))
+                if (!IsMdStatic(dwMemberAttrs))
                 {
                     BuildMethodTableThrowException(BFA_NONVIRT_INST_INT_METHOD);
                 }
             }
         }
+#endif
 
         // No synchronized methods in ValueTypes
         if(fIsClassValueType && IsMiSynchronized(dwImplFlags))

diff --git a/src/vm/methodtablebuilder.cpp b/src/vm/methodtablebuilder.cpp
@@ -2900,25 +2900,27 @@ MethodTableBuilder::EnumerateClassMethods()
             }
         }
 
+#ifndef FEATURE_DEFAULT_INTERFACES
         // Some interface checks.
         if (fIsClassInterface)
         {
             if (IsMdVirtual(dwMemberAttrs))
             {
-                if (!IsMdAbstract(dwMemberAttrs) && (CLRConfig::GetConfigValue(CLRConfig::UNSUPPORTED_TypeLoader_DefaultInterfaces) == 0))
+                if (!IsMdAbstract(dwMemberAttrs))
                 {
                     BuildMethodTableThrowException(BFA_VIRTUAL_NONAB_INT_METHOD);
                 }
             }
             else
             {
                 // Instance field/method
-                if (!IsMdStatic(dwMemberAttrs) && (CLRConfig::GetConfigValue(CLRConfig::UNSUPPORTED_TypeLoader_DefaultInterfaces) == 0))
+                if (!IsMdStatic(dwMemberAttrs))
                 {
                     BuildMethodTableThrowException(BFA_NONVIRT_INST_INT_METHOD);
                 }
             }
         }
+#endif
 
         // No synchronized methods in ValueTypes
         if(fIsClassValueType && IsMiSynchronized(dwImplFlags))

diff --git a/tests/src/Loader/classloader/DefaultInterfaceMethods/constrainedcall/constrainedcall.ilproj b/tests/src/Loader/classloader/DefaultInterfaceMethods/constrainedcall/constrainedcall.ilproj
@@ -14,23 +14,7 @@
     <ReferenceLocalMscorlib>true</ReferenceLocalMscorlib>
     <OutputType>Exe</OutputType>
     <CLRTestKind>BuildAndRun</CLRTestKind>
-    <CLRTestPriority>0</CLRTestPriority>
-    <!-- Use ILAsm that we just built for the new fixes required in default interface methods -->
-    <UseCustomILAsm>True</UseCustomILAsm>
-
-    <CLRTestBatchPreCommands>
-<![CDATA[
-      $(CLRTestBatchPreCommands)
-set COMPlus_UNSUPPORTED_TypeLoader_DefaultInterfaces=1
-    ]]>
-    </CLRTestBatchPreCommands>
-    <BashCLRTestPreCommands>
-<![CDATA[
-      $(BashCLRTestPreCommands)
-export COMPlus_UNSUPPORTED_TypeLoader_DefaultInterfaces=1
-    ]]>
-    </BashCLRTestPreCommands>
-
+    <CLRTestPriority>0</CLRTestPriority>    
   </PropertyGroup>
 
   <ItemGroup>

diff --git a/tests/src/Loader/classloader/DefaultInterfaceMethods/diamondshape/diamondshape.ilproj b/tests/src/Loader/classloader/DefaultInterfaceMethods/diamondshape/diamondshape.ilproj
@@ -15,22 +15,6 @@
     <OutputType>Exe</OutputType>
     <CLRTestKind>BuildAndRun</CLRTestKind>
     <CLRTestPriority>0</CLRTestPriority>
-    <!-- Use ILAsm that we just built for the new fixes required in default interface methods -->
-    <UseCustomILAsm>True</UseCustomILAsm>    
-
-    <CLRTestBatchPreCommands>
-<![CDATA[
-      $(CLRTestBatchPreCommands)
-set COMPlus_UNSUPPORTED_TypeLoader_DefaultInterfaces=1
-    ]]>
-    </CLRTestBatchPreCommands>
-    <BashCLRTestPreCommands>
-<![CDATA[
-      $(BashCLRTestPreCommands)
-export COMPlus_UNSUPPORTED_TypeLoader_DefaultInterfaces=1
-    ]]>
-    </BashCLRTestPreCommands>
-
   </PropertyGroup>
 
   <ItemGroup>

diff --git a/tests/src/Loader/classloader/DefaultInterfaceMethods/genericmethods/genericmethods.ilproj b/tests/src/Loader/classloader/DefaultInterfaceMethods/genericmethods/genericmethods.ilproj
@@ -15,22 +15,6 @@
     <OutputType>Exe</OutputType>
     <CLRTestKind>BuildAndRun</CLRTestKind>
     <CLRTestPriority>0</CLRTestPriority>
-    <!-- Use ILAsm that we just built for the new fixes required in default interface methods -->
-    <UseCustomILAsm>True</UseCustomILAsm>
-
-    <CLRTestBatchPreCommands>
-<![CDATA[
-      $(CLRTestBatchPreCommands)
-set COMPlus_UNSUPPORTED_TypeLoader_DefaultInterfaces=1
-    ]]>
-    </CLRTestBatchPreCommands>
-    <BashCLRTestPreCommands>
-<![CDATA[
-      $(BashCLRTestPreCommands)
-export COMPlus_UNSUPPORTED_TypeLoader_DefaultInterfaces=1
-    ]]>
-    </BashCLRTestPreCommands>
-
   </PropertyGroup>
 
   <ItemGroup>

diff --git a/tests/src/Loader/classloader/DefaultInterfaceMethods/methodimpl/methodimpl.ilproj b/tests/src/Loader/classloader/DefaultInterfaceMethods/methodimpl/methodimpl.ilproj
@@ -15,22 +15,6 @@
     <OutputType>Exe</OutputType>
     <CLRTestKind>BuildAndRun</CLRTestKind>
     <CLRTestPriority>0</CLRTestPriority>
-    <!-- Use ILAsm that we just built for the new fixes required in default interface methods -->
-    <UseCustomILAsm>True</UseCustomILAsm>
-
-    <CLRTestBatchPreCommands>
-<![CDATA[
-      $(CLRTestBatchPreCommands)
-set COMPlus_UNSUPPORTED_TypeLoader_DefaultInterfaces=1
-    ]]>
-    </CLRTestBatchPreCommands>
-    <BashCLRTestPreCommands>
-<![CDATA[
-      $(BashCLRTestPreCommands)
-export COMPlus_UNSUPPORTED_TypeLoader_DefaultInterfaces=1
-    ]]>
-    </BashCLRTestPreCommands>
-
   </PropertyGroup>
 
   <ItemGroup>

diff --git a/tests/src/Loader/classloader/DefaultInterfaceMethods/sharedgenerics/sharedgenerics.ilproj b/tests/src/Loader/classloader/DefaultInterfaceMethods/sharedgenerics/sharedgenerics.ilproj
@@ -15,22 +15,6 @@
     <OutputType>Exe</OutputType>
     <CLRTestKind>BuildAndRun</CLRTestKind>
     <CLRTestPriority>0</CLRTestPriority>
-    <!-- Use ILAsm that we just built for the new fixes required in default interface methods -->
-    <UseCustomILAsm>True</UseCustomILAsm>
-
-    <CLRTestBatchPreCommands>
-<![CDATA[
-      $(CLRTestBatchPreCommands)
-set COMPlus_UNSUPPORTED_TypeLoader_DefaultInterfaces=1
-    ]]>
-    </CLRTestBatchPreCommands>
-    <BashCLRTestPreCommands>
-<![CDATA[
-      $(BashCLRTestPreCommands)
-export COMPlus_UNSUPPORTED_TypeLoader_DefaultInterfaces=1
-    ]]>
-    </BashCLRTestPreCommands>
-
   </PropertyGroup>
 
   <ItemGroup>

diff --git a/tests/src/Loader/classloader/DefaultInterfaceMethods/simple/simple.ilproj b/tests/src/Loader/classloader/DefaultInterfaceMethods/simple/simple.ilproj
@@ -15,22 +15,6 @@
     <OutputType>Exe</OutputType>
     <CLRTestKind>BuildAndRun</CLRTestKind>
     <CLRTestPriority>0</CLRTestPriority>
-    <!-- Use ILAsm that we just built for the new fixes required in default interface methods -->
-    <UseCustomILAsm>True</UseCustomILAsm>  
-
-    <CLRTestBatchPreCommands>
-<![CDATA[
-      $(CLRTestBatchPreCommands)
-set COMPlus_UNSUPPORTED_TypeLoader_DefaultInterfaces=1
-    ]]>
-    </CLRTestBatchPreCommands>
-    <BashCLRTestPreCommands>
-<![CDATA[
-      $(BashCLRTestPreCommands)
-export COMPlus_UNSUPPORTED_TypeLoader_DefaultInterfaces=1
-    ]]>
-    </BashCLRTestPreCommands>
-
   </PropertyGroup>
 
   <ItemGroup>

diff --git a/tests/src/Loader/classloader/DefaultInterfaceMethods/valuetypes/valuetypes.ilproj b/tests/src/Loader/classloader/DefaultInterfaceMethods/valuetypes/valuetypes.ilproj
@@ -15,22 +15,6 @@
     <OutputType>Exe</OutputType>
     <CLRTestKind>BuildAndRun</CLRTestKind>
     <CLRTestPriority>0</CLRTestPriority>
-    <!-- Use ILAsm that we just built for the new fixes required in default interface methods -->
-    <UseCustomILAsm>True</UseCustomILAsm>    
-
-    <CLRTestBatchPreCommands>
-<![CDATA[
-      $(CLRTestBatchPreCommands)
-set COMPlus_UNSUPPORTED_TypeLoader_DefaultInterfaces=1
-    ]]>
-    </CLRTestBatchPreCommands>
-    <BashCLRTestPreCommands>
-<![CDATA[
-      $(BashCLRTestPreCommands)
-export COMPlus_UNSUPPORTED_TypeLoader_DefaultInterfaces=1
-    ]]>
-    </BashCLRTestPreCommands>
-
   </PropertyGroup>
 
   <ItemGroup>

diff --git a/tests/src/baseservices/compilerservices/RuntimeFeature/DefaultImplementationsOfInterfaces.il b/tests/src/baseservices/compilerservices/RuntimeFeature/DefaultImplementationsOfInterfaces.il
@@ -0,0 +1,73 @@
+// 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.
+
+//
+// Verifies that RuntimeFeature::IsSupported("DefaultImplementationsOfInterfaces") matches reality.
+// This needs to succeed no matter whether default interfaces are supported.
+//
+
+.assembly extern System.Runtime{}
+.assembly DefaultImplementationsOfInterfaces{}
+
+.class interface private abstract auto ansi DefaultInterface
+{
+  .method public hidebysig newslot virtual 
+          instance void  Method() cil managed
+  {
+    .maxstack  8
+    ret
+  }
+}
+
+.method private hidebysig static void TryLoadDefaultInterface() cil managed noinlining
+{
+    .maxstack  8
+    ldtoken    DefaultInterface
+    call       class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle)
+    callvirt   instance string [System.Runtime]System.Object::ToString()
+    pop
+    ret
+}
+
+.method private hidebysig static bool  SupportsDefaultInterfaces() cil managed
+{
+    .maxstack  1
+    .try
+    {
+      call       void TryLoadDefaultInterface()
+      leave.s    Supports
+
+    }
+    catch [System.Runtime]System.TypeLoadException 
+    {
+      pop
+      leave.s    DoesNotSupport
+
+    }
+  Supports:
+    ldc.i4.1
+    ret
+
+  DoesNotSupport:
+    ldc.i4.0
+    ret
+  }
+
+.method private hidebysig static int32 
+        Main() cil managed
+{
+    .entrypoint
+    .maxstack  2
+    ldstr      "DefaultImplementationsOfInterfaces"
+    call       bool [System.Runtime]System.Runtime.CompilerServices.RuntimeFeature::IsSupported(string)
+    call       bool SupportsDefaultInterfaces()
+    beq        Good
+
+    ldc.i4.m1
+    ret
+
+  Good:
+    ldc.i4     100
+    ret
+}