From ec4fc3f08e85f1733272973be3155cfa4ce3208e Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Mon, 17 Jul 2023 16:23:53 -0700 Subject: [PATCH 1/7] Add support for default implmentation of static virtuals with method constraints Add support for default implmentation of static virtuals with method constraints --- src/coreclr/vm/genericdict.cpp | 6 +- src/coreclr/vm/methodtable.cpp | 97 +++++++++++---- src/coreclr/vm/methodtable.h | 77 ++++++++++-- src/coreclr/vm/runtimehandles.cpp | 5 +- src/coreclr/vm/typedesc.cpp | 5 +- ...faceMapWithStaticVirtualsAndConstraints.cs | 115 ++++++++++++++++++ ...MapWithStaticVirtualsAndConstraints.csproj | 8 ++ .../RegressionTests/GitHub_73658.cs | 115 ++++++++++++++++++ .../RegressionTests/GitHub_78865.cs | 42 +++++++ ...thodConstraintsAndDefaultImplementation.cs | 42 +++++++ ...ConstraintsAndDefaultImplementation.csproj | 8 ++ 11 files changed, 478 insertions(+), 42 deletions(-) create mode 100644 src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GetInterfaceMapWithStaticVirtualsAndConstraints.cs create mode 100644 src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GetInterfaceMapWithStaticVirtualsAndConstraints.csproj create mode 100644 src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_73658.cs create mode 100644 src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_78865.cs create mode 100644 src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/StaticVirtualsAndMethodConstraintsAndDefaultImplementation.cs create mode 100644 src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/StaticVirtualsAndMethodConstraintsAndDefaultImplementation.csproj diff --git a/src/coreclr/vm/genericdict.cpp b/src/coreclr/vm/genericdict.cpp index 456bec758b2a6..09dcda4b11ebc 100644 --- a/src/coreclr/vm/genericdict.cpp +++ b/src/coreclr/vm/genericdict.cpp @@ -1144,9 +1144,9 @@ Dictionary::PopulateEntry( pResolvedMD = constraintType.GetMethodTable()->ResolveVirtualStaticMethod( ownerType.GetMethodTable(), pMethod, - /* allowNullResult */ TRUE, - /* verifyImplemented */ FALSE, - /* allowVariantMatches */ TRUE, + ResolveVirtualStaticMethodFlags::AllowNullResult | + ResolveVirtualStaticMethodFlags::AllowVariantMatches | + ResolveVirtualStaticMethodFlags::InstantiateResultOverFinalMethodDesc, &uniqueResolution); // If we couldn't get an exact result, fall back to using a stub to make the exact function call diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 2709be3f7ef62..3dafa74dbd67d 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -6210,22 +6210,26 @@ MethodTable::FindDispatchImpl( // Try exact match first MethodDesc *pDefaultMethod = NULL; + + FindDefaultInterfaceImplementationFlags flags = FindDefaultInterfaceImplementationFlags::InstantiateFoundMethodDesc; + if (throwOnConflict) + flags = flags | FindDefaultInterfaceImplementationFlags::ThrowOnConflict; + BOOL foundDefaultInterfaceImplementation = FindDefaultInterfaceImplementation( pIfcMD, // the interface method being resolved pIfcMT, // the interface being resolved &pDefaultMethod, - FALSE, // allowVariance - throwOnConflict); + flags); // If there's no exact match, try a variant match if (!foundDefaultInterfaceImplementation && pIfcMT->HasVariance()) { + flags = flags | FindDefaultInterfaceImplementationFlags::AllowVariance; foundDefaultInterfaceImplementation = FindDefaultInterfaceImplementation( pIfcMD, // the interface method being resolved pIfcMT, // the interface being resolved &pDefaultMethod, - TRUE, // allowVariance - throwOnConflict); + flags); } if (foundDefaultInterfaceImplementation) @@ -6324,10 +6328,13 @@ namespace MethodTable *pMT, MethodDesc *interfaceMD, MethodTable *interfaceMT, - BOOL allowVariance, + FindDefaultInterfaceImplementationFlags findDefaultImplementationFlags, MethodDesc **candidateMD, ClassLoadLevel level) { + bool allowVariance = (findDefaultImplementationFlags & FindDefaultInterfaceImplementationFlags::AllowVariance) != FindDefaultInterfaceImplementationFlags::None; + bool instantiateMethodInstantiation = (findDefaultImplementationFlags & FindDefaultInterfaceImplementationFlags::InstantiateFoundMethodDesc) != FindDefaultInterfaceImplementationFlags::None; + *candidateMD = NULL; MethodDesc *candidateMaybe = NULL; @@ -6418,11 +6425,20 @@ namespace else { // Static virtual methods don't record MethodImpl slots so they need special handling + ResolveVirtualStaticMethodFlags resolveVirtualStaticMethodFlags = ResolveVirtualStaticMethodFlags::None; + if (allowVariance) + { + resolveVirtualStaticMethodFlags |= ResolveVirtualStaticMethodFlags::AllowVariantMatches; + } + if (instantiateMethodInstantiation) + { + resolveVirtualStaticMethodFlags |= ResolveVirtualStaticMethodFlags::InstantiateResultOverFinalMethodDesc; + } + candidateMaybe = pMT->TryResolveVirtualStaticMethodOnThisType( interfaceMT, interfaceMD, - /* verifyImplemented */ FALSE, - /* allowVariance */ allowVariance, + resolveVirtualStaticMethodFlags, /* level */ level); } } @@ -6440,6 +6456,7 @@ namespace pMT, FALSE, // forceBoxedEntryPoint candidateMaybe->HasMethodInstantiation() ? +// TODO SHOULD WE CHANGE THIS TO candidateMaybe->HasMethodInstantiation() && instantiateMethodInstantiation ? candidateMaybe->AsInstantiatedMethodDesc()->IMD_GetMethodInstantiation() : Instantiation(), // for method themselves that are generic FALSE, // allowInstParam @@ -6461,8 +6478,7 @@ BOOL MethodTable::FindDefaultInterfaceImplementation( MethodDesc *pInterfaceMD, MethodTable *pInterfaceMT, MethodDesc **ppDefaultMethod, - BOOL allowVariance, - BOOL throwOnConflict, + FindDefaultInterfaceImplementationFlags findDefaultImplementationFlags, ClassLoadLevel level ) { @@ -6478,12 +6494,13 @@ BOOL MethodTable::FindDefaultInterfaceImplementation( } CONTRACT_END; #ifdef FEATURE_DEFAULT_INTERFACES + bool allowVariance = (findDefaultImplementationFlags & FindDefaultInterfaceImplementationFlags::AllowVariance) != FindDefaultInterfaceImplementationFlags::None; CQuickArray candidates; unsigned candidatesCount = 0; // Check the current method table itself MethodDesc *candidateMaybe = NULL; - if (IsInterface() && TryGetCandidateImplementation(this, pInterfaceMD, pInterfaceMT, allowVariance, &candidateMaybe, level)) + if (IsInterface() && TryGetCandidateImplementation(this, pInterfaceMD, pInterfaceMT, findDefaultImplementationFlags, &candidateMaybe, level)) { _ASSERTE(candidateMaybe != NULL); @@ -6523,7 +6540,7 @@ BOOL MethodTable::FindDefaultInterfaceImplementation( MethodTable *pCurMT = it.GetInterface(pMT, level); MethodDesc *pCurMD = NULL; - if (TryGetCandidateImplementation(pCurMT, pInterfaceMD, pInterfaceMT, allowVariance, &pCurMD, level)) + if (TryGetCandidateImplementation(pCurMT, pInterfaceMD, pInterfaceMT, findDefaultImplementationFlags, &pCurMD, level)) { // // Found a match. But is it a more specific match (we want most specific interfaces) @@ -6619,6 +6636,8 @@ BOOL MethodTable::FindDefaultInterfaceImplementation( } else if (pBestCandidateMT != candidates[i].pMT) { + bool throwOnConflict = (findDefaultImplementationFlags & FindDefaultInterfaceImplementationFlags::ThrowOnConflict) != FindDefaultInterfaceImplementationFlags::None; + if (throwOnConflict) ThrowExceptionForConflictingOverride(this, pInterfaceMT, pInterfaceMD); @@ -8875,12 +8894,15 @@ MethodDesc * MethodTable::ResolveVirtualStaticMethod( MethodTable* pInterfaceType, MethodDesc* pInterfaceMD, - BOOL allowNullResult, - BOOL verifyImplemented, - BOOL allowVariantMatches, + ResolveVirtualStaticMethodFlags resolveVirtualStaticMethodFlags, BOOL* uniqueResolution, ClassLoadLevel level) { + bool verifyImplemented = (resolveVirtualStaticMethodFlags & ResolveVirtualStaticMethodFlags::VerifyImplemented) != ResolveVirtualStaticMethodFlags::None; + bool allowVariantMatches = (resolveVirtualStaticMethodFlags & ResolveVirtualStaticMethodFlags::AllowVariantMatches) != ResolveVirtualStaticMethodFlags::None; + bool instantiateMethodParameters = (resolveVirtualStaticMethodFlags & ResolveVirtualStaticMethodFlags::InstantiateResultOverFinalMethodDesc) != ResolveVirtualStaticMethodFlags::None; + bool allowNullResult = (resolveVirtualStaticMethodFlags & ResolveVirtualStaticMethodFlags::AllowNullResult) != ResolveVirtualStaticMethodFlags::None; + if (uniqueResolution != nullptr) { *uniqueResolution = TRUE; @@ -8912,7 +8934,7 @@ MethodTable::ResolveVirtualStaticMethod( // Search for match on a per-level in the type hierarchy for (MethodTable* pMT = this; pMT != nullptr; pMT = pMT->GetParentMethodTable()) { - MethodDesc* pMD = pMT->TryResolveVirtualStaticMethodOnThisType(pInterfaceType, pInterfaceMD, verifyImplemented, /*allowVariance*/ FALSE, level); + MethodDesc* pMD = pMT->TryResolveVirtualStaticMethodOnThisType(pInterfaceType, pInterfaceMD, resolveVirtualStaticMethodFlags & ~ResolveVirtualStaticMethodFlags::AllowVariantMatches, level); if (pMD != nullptr) { return pMD; @@ -8956,7 +8978,7 @@ MethodTable::ResolveVirtualStaticMethod( { // Variant or equivalent matching interface found // Attempt to resolve on variance matched interface - pMD = pMT->TryResolveVirtualStaticMethodOnThisType(pItfInMap, pInterfaceMD, verifyImplemented, /*allowVariance*/ FALSE, level); + pMD = pMT->TryResolveVirtualStaticMethodOnThisType(pItfInMap, pInterfaceMD, resolveVirtualStaticMethodFlags & ~ResolveVirtualStaticMethodFlags::AllowVariantMatches, level); if (pMD != nullptr) { return pMD; @@ -8970,12 +8992,25 @@ MethodTable::ResolveVirtualStaticMethod( BOOL allowVariantMatchInDefaultImplementationLookup = FALSE; do { + FindDefaultInterfaceImplementationFlags findDefaultImplementationFlags = FindDefaultInterfaceImplementationFlags::None; + if (allowVariantMatchInDefaultImplementationLookup) + { + findDefaultImplementationFlags |= FindDefaultInterfaceImplementationFlags::AllowVariance; + } + if (uniqueResolution == nullptr) + { + findDefaultImplementationFlags |= FindDefaultInterfaceImplementationFlags::ThrowOnConflict; + } + if (instantiateMethodParameters) + { + findDefaultImplementationFlags |= FindDefaultInterfaceImplementationFlags::InstantiateFoundMethodDesc; + } + BOOL haveUniqueDefaultImplementation = FindDefaultInterfaceImplementation( pInterfaceMD, pInterfaceType, &pMDDefaultImpl, - /* allowVariance */ allowVariantMatchInDefaultImplementationLookup, - /* throwOnConflict */ uniqueResolution == nullptr, + findDefaultImplementationFlags, level); if (haveUniqueDefaultImplementation || (pMDDefaultImpl != nullptr && (verifyImplemented || uniqueResolution != nullptr))) { @@ -9002,7 +9037,7 @@ MethodTable::ResolveVirtualStaticMethod( } // Default implementation logic, which only kicks in for default implementations when looking up on an exact interface target - if (!pInterfaceMD->IsAbstract() && !(this == g_pCanonMethodTableClass) && !IsSharedByGenericInstantiations()) + if (!pInterfaceMD->IsAbstract() && !(this == g_pCanonMethodTableClass) && !IsSharedByGenericInstantiations() && instantiateMethodParameters) { return pInterfaceMD->FindOrCreateAssociatedMethodDesc(pInterfaceMD, pInterfaceType, FALSE, pInterfaceMD->GetMethodInstantiation(), FALSE); } @@ -9018,8 +9053,12 @@ MethodTable::ResolveVirtualStaticMethod( // Try to locate the appropriate MethodImpl matching a given interface static virtual method. // Returns nullptr on failure. MethodDesc* -MethodTable::TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType, MethodDesc* pInterfaceMD, BOOL verifyImplemented, BOOL allowVariance, ClassLoadLevel level) +MethodTable::TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType, MethodDesc* pInterfaceMD, ResolveVirtualStaticMethodFlags resolveVirtualStaticMethodFlags, ClassLoadLevel level) { + bool instantiateMethodParameters = (resolveVirtualStaticMethodFlags & ResolveVirtualStaticMethodFlags::InstantiateResultOverFinalMethodDesc) != ResolveVirtualStaticMethodFlags::None; + bool allowVariance = (resolveVirtualStaticMethodFlags & ResolveVirtualStaticMethodFlags::AllowVariantMatches) != ResolveVirtualStaticMethodFlags::None; + bool verifyImplemented = (resolveVirtualStaticMethodFlags & ResolveVirtualStaticMethodFlags::VerifyImplemented) != ResolveVirtualStaticMethodFlags::None; + HRESULT hr = S_OK; IMDInternalImport* pMDInternalImport = GetMDImport(); HENUMInternalMethodImplHolder hEnumMethodImpl(pMDInternalImport); @@ -9148,7 +9187,7 @@ MethodTable::TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType COMPlusThrow(kTypeLoadException, E_FAIL); } - if (!verifyImplemented) + if (!verifyImplemented && instantiateMethodParameters) { pMethodImpl = pMethodImpl->FindOrCreateAssociatedMethodDesc( pMethodImpl, @@ -9202,9 +9241,7 @@ MethodTable::VerifyThatAllVirtualStaticMethodsAreImplemented() !ResolveVirtualStaticMethod( pInterfaceMT, pMD, - /* allowNullResult */ TRUE, - /* verifyImplemented */ TRUE, - /* allowVariantMatches */ FALSE, + ResolveVirtualStaticMethodFlags::AllowNullResult | ResolveVirtualStaticMethodFlags::VerifyImplemented, /* uniqueResolution */ &uniqueResolution, /* level */ CLASS_LOAD_EXACTPARENTS))) { @@ -9240,12 +9277,18 @@ MethodTable::TryResolveConstraintMethodApprox( _ASSERTE(!thInterfaceType.IsTypeDesc()); _ASSERTE(thInterfaceType.IsInterface()); BOOL uniqueResolution; + + ResolveVirtualStaticMethodFlags flags = ResolveVirtualStaticMethodFlags::AllowVariantMatches + | ResolveVirtualStaticMethodFlags::InstantiateResultOverFinalMethodDesc; + if (pfForceUseRuntimeLookup != NULL) + { + flags |= ResolveVirtualStaticMethodFlags::AllowNullResult; + } + MethodDesc *result = ResolveVirtualStaticMethod( thInterfaceType.GetMethodTable(), pInterfaceMD, - /* allowNullResult */pfForceUseRuntimeLookup != NULL, - /* verifyImplemented */ FALSE, - /* allowVariantMatches */ TRUE, + flags, &uniqueResolution); if (result == NULL || !uniqueResolution) { diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index 9b347bf5ccc85..e3dd6ebfe5d35 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -63,6 +63,73 @@ class ClassFactoryBase; class ArgDestination; enum class WellKnownAttribute : DWORD; +enum class ResolveVirtualStaticMethodFlags +{ + None = 0, + AllowNullResult = 1, + VerifyImplemented = 2, + AllowVariantMatches = 4, + InstantiateResultOverFinalMethodDesc = 8 +}; + +inline ResolveVirtualStaticMethodFlags operator& (ResolveVirtualStaticMethodFlags left, ResolveVirtualStaticMethodFlags right) +{ + return ResolveVirtualStaticMethodFlags(int(left) & int(right)); +} + +inline ResolveVirtualStaticMethodFlags operator| (ResolveVirtualStaticMethodFlags left, ResolveVirtualStaticMethodFlags right) +{ + return ResolveVirtualStaticMethodFlags(int(left) | int(right)); +} +inline ResolveVirtualStaticMethodFlags& operator|=(ResolveVirtualStaticMethodFlags& left, ResolveVirtualStaticMethodFlags right) +{ + left = left | right; + return left; +} +inline ResolveVirtualStaticMethodFlags operator^ (ResolveVirtualStaticMethodFlags left, ResolveVirtualStaticMethodFlags right) +{ + return ResolveVirtualStaticMethodFlags(int(left) ^ int(right)); +} + +inline ResolveVirtualStaticMethodFlags operator~ (ResolveVirtualStaticMethodFlags value) +{ + return ResolveVirtualStaticMethodFlags(~int(value)); +} + +enum class FindDefaultInterfaceImplementationFlags +{ + None, + AllowVariance = 1, + ThrowOnConflict = 2, + InstantiateFoundMethodDesc = 4 +}; + +inline FindDefaultInterfaceImplementationFlags operator& (FindDefaultInterfaceImplementationFlags left, FindDefaultInterfaceImplementationFlags right) +{ + return FindDefaultInterfaceImplementationFlags(int(left) & int(right)); +} + +inline FindDefaultInterfaceImplementationFlags operator| (FindDefaultInterfaceImplementationFlags left, FindDefaultInterfaceImplementationFlags right) +{ + return FindDefaultInterfaceImplementationFlags(int(left) | int(right)); +} + +inline FindDefaultInterfaceImplementationFlags& operator|=(FindDefaultInterfaceImplementationFlags& left, FindDefaultInterfaceImplementationFlags right) +{ + left = left | right; + return left; +} + +inline FindDefaultInterfaceImplementationFlags operator^ (FindDefaultInterfaceImplementationFlags left, FindDefaultInterfaceImplementationFlags right) +{ + return FindDefaultInterfaceImplementationFlags(int(left) ^ int(right)); +} + +inline FindDefaultInterfaceImplementationFlags operator~ (FindDefaultInterfaceImplementationFlags value) +{ + return FindDefaultInterfaceImplementationFlags(~int(value)); +} + //============================================================================ // This is the in-memory structure of a class and it will evolve. //============================================================================ @@ -2084,7 +2151,6 @@ class MethodTable MethodDesc *GetMethodDescForComInterfaceMethod(MethodDesc *pItfMD, bool fNullOk); #endif // FEATURE_COMINTEROP - // Resolve virtual static interface method pInterfaceMD on this type. // // Specify allowNullResult to return NULL instead of throwing if the there is no implementation @@ -2096,9 +2162,7 @@ class MethodTable MethodDesc *ResolveVirtualStaticMethod( MethodTable* pInterfaceType, MethodDesc* pInterfaceMD, - BOOL allowNullResult, - BOOL verifyImplemented = FALSE, - BOOL allowVariantMatches = TRUE, + ResolveVirtualStaticMethodFlags resolveVirtualStaticMethodFlags, BOOL *uniqueResolution = NULL, ClassLoadLevel level = CLASS_LOADED); @@ -2178,8 +2242,7 @@ class MethodTable MethodDesc *pInterfaceMD, MethodTable *pObjectMT, MethodDesc **ppDefaultMethod, - BOOL allowVariance, - BOOL throwOnConflict, + FindDefaultInterfaceImplementationFlags findDefaultImplementationFlags, ClassLoadLevel level = CLASS_LOADED); #endif // DACCESS_COMPILE @@ -2219,7 +2282,7 @@ class MethodTable // Try to resolve a given static virtual method override on this type. Return nullptr // when not found. - MethodDesc *TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType, MethodDesc* pInterfaceMD, BOOL verifyImplemented, BOOL allowVariance, ClassLoadLevel level); + MethodDesc *TryResolveVirtualStaticMethodOnThisType(MethodTable* pInterfaceType, MethodDesc* pInterfaceMD, ResolveVirtualStaticMethodFlags resolveVirtualStaticMethodFlags, ClassLoadLevel level); public: static MethodDesc *MapMethodDeclToMethodImpl(MethodDesc *pMDDecl); diff --git a/src/coreclr/vm/runtimehandles.cpp b/src/coreclr/vm/runtimehandles.cpp index ef5a2dcaaf93a..533419640e6a2 100644 --- a/src/coreclr/vm/runtimehandles.cpp +++ b/src/coreclr/vm/runtimehandles.cpp @@ -1102,9 +1102,8 @@ extern "C" MethodDesc* QCALLTYPE RuntimeTypeHandle_GetInterfaceMethodImplementat pResult = typeHandle.GetMethodTable()->ResolveVirtualStaticMethod( thOwnerOfMD.GetMethodTable(), pMD, - /* allowNullResult */ TRUE, - /* verifyImplemented*/ FALSE, - /* allowVariantMatches */ TRUE); + ResolveVirtualStaticMethodFlags::AllowNullResult | + ResolveVirtualStaticMethodFlags::AllowVariantMatches); } else { diff --git a/src/coreclr/vm/typedesc.cpp b/src/coreclr/vm/typedesc.cpp index 4840d9ec8701f..fdb35b574051b 100644 --- a/src/coreclr/vm/typedesc.cpp +++ b/src/coreclr/vm/typedesc.cpp @@ -1611,8 +1611,9 @@ BOOL TypeVarTypeDesc::SatisfiesConstraints(SigTypeContext *pTypeContextOfConstra if (pMD->IsVirtual() && pMD->IsStatic() && (pMD->IsAbstract() && !thElem.AsMethodTable()->ResolveVirtualStaticMethod( - pInterfaceMT, pMD, /* allowNullResult */ TRUE, /* verifyImplemented */ TRUE, - /*allowVariantMatches*/ TRUE, /*uniqueResolution*/ NULL, CLASS_DEPENDENCIES_LOADED))) + pInterfaceMT, pMD, + ResolveVirtualStaticMethodFlags::AllowNullResult | ResolveVirtualStaticMethodFlags::VerifyImplemented | ResolveVirtualStaticMethodFlags::AllowVariantMatches, + /*uniqueResolution*/ NULL, CLASS_DEPENDENCIES_LOADED))) { virtualStaticResolutionCheckFailed = true; break; diff --git a/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GetInterfaceMapWithStaticVirtualsAndConstraints.cs b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GetInterfaceMapWithStaticVirtualsAndConstraints.cs new file mode 100644 index 0000000000000..fe9c2dcebc6c7 --- /dev/null +++ b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GetInterfaceMapWithStaticVirtualsAndConstraints.cs @@ -0,0 +1,115 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Reflection; +using Xunit; + +namespace GetInterfaceMapWithStaticVirtualsAndConstraints +{ + internal static class Program + { + static int Main() + { + Type i, s; + InterfaceMapping imap; + Console.WriteLine("Inner"); + s = typeof(Program).Assembly.GetType("GetInterfaceMapWithStaticVirtualsAndConstraints.Outer`1+Inner"); + s = s.MakeGenericType(typeof(int)); + i = s.GetInterface("IStatics"); + imap = s.GetInterfaceMap(i); + foreach (var iMethod in imap.InterfaceMethods) + { + Console.WriteLine($"{iMethod.DeclaringType } {iMethod}"); + } + + Assert.Equal(1, imap.TargetMethods.Length); + foreach (var iMethod in imap.TargetMethods) + { + Console.WriteLine($"{iMethod.DeclaringType } {iMethod}"); + Assert.Equal(typeof(Outer.Inner), iMethod.DeclaringType); + } + + i = s.GetInterface("IInstance"); + imap = s.GetInterfaceMap(i); + foreach (var iMethod in imap.InterfaceMethods) + { + Console.WriteLine($"{iMethod.DeclaringType } {iMethod}"); + } + + Assert.Equal(1, imap.TargetMethods.Length); + foreach (var iMethod in imap.TargetMethods) + { + Console.WriteLine($"{iMethod.DeclaringType } {iMethod}"); + Assert.Equal(typeof(Outer.Inner), iMethod.DeclaringType); + } + + Console.WriteLine("Inner2"); + s = typeof(Program).Assembly.GetType("GetInterfaceMapWithStaticVirtualsAndConstraints.Outer`1+Inner2"); + s = s.MakeGenericType(typeof(int)); + i = s.GetInterface("IStatics"); + imap = s.GetInterfaceMap(i); + foreach (var iMethod in imap.InterfaceMethods) + { + Console.WriteLine($"{iMethod.DeclaringType } {iMethod}"); + } + Assert.Equal(1, imap.TargetMethods.Length); + foreach (var iMethod in imap.TargetMethods) + { + Console.WriteLine($"{iMethod.DeclaringType } {iMethod}"); + Assert.Equal(typeof(Outer.IStaticsImpl), iMethod.DeclaringType); + } + + i = s.GetInterface("IInstance"); + imap = s.GetInterfaceMap(i); + foreach (var iMethod in imap.InterfaceMethods) + { + Console.WriteLine($"{iMethod.DeclaringType } {iMethod}"); + } + Assert.Equal(1, imap.TargetMethods.Length); + foreach (var iMethod in imap.TargetMethods) + { + Console.WriteLine($"{iMethod.DeclaringType } {iMethod}"); + Assert.Equal(typeof(Outer.IInstanceImpl), iMethod.DeclaringType); + } + + return 100; + } + } + + class Outer + { + public struct Inner : IStatics, IInstance + { + public void M() where TInner : IConstraint { } + public static void MStatic() where TInner : IConstraint { } + } + + public struct Inner2 : IStaticsImpl, IInstanceImpl + { + + } + + public interface IStaticsImpl : IStatics + { + static void IStatics.MStatic() { } + } + + public interface IInstanceImpl : IInstance + { + void IInstance.M() { } + } + + public interface IStatics + { + static abstract void MStatic() where TInner2 : IConstraint; + } + + public interface IInstance + { + abstract void M() where TInner2 : IConstraint; + } + + public interface IConstraint { } + } +} diff --git a/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GetInterfaceMapWithStaticVirtualsAndConstraints.csproj b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GetInterfaceMapWithStaticVirtualsAndConstraints.csproj new file mode 100644 index 0000000000000..570644f1dbcb7 --- /dev/null +++ b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GetInterfaceMapWithStaticVirtualsAndConstraints.csproj @@ -0,0 +1,8 @@ + + + Exe + + + + + diff --git a/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_73658.cs b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_73658.cs new file mode 100644 index 0000000000000..fe9c2dcebc6c7 --- /dev/null +++ b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_73658.cs @@ -0,0 +1,115 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Reflection; +using Xunit; + +namespace GetInterfaceMapWithStaticVirtualsAndConstraints +{ + internal static class Program + { + static int Main() + { + Type i, s; + InterfaceMapping imap; + Console.WriteLine("Inner"); + s = typeof(Program).Assembly.GetType("GetInterfaceMapWithStaticVirtualsAndConstraints.Outer`1+Inner"); + s = s.MakeGenericType(typeof(int)); + i = s.GetInterface("IStatics"); + imap = s.GetInterfaceMap(i); + foreach (var iMethod in imap.InterfaceMethods) + { + Console.WriteLine($"{iMethod.DeclaringType } {iMethod}"); + } + + Assert.Equal(1, imap.TargetMethods.Length); + foreach (var iMethod in imap.TargetMethods) + { + Console.WriteLine($"{iMethod.DeclaringType } {iMethod}"); + Assert.Equal(typeof(Outer.Inner), iMethod.DeclaringType); + } + + i = s.GetInterface("IInstance"); + imap = s.GetInterfaceMap(i); + foreach (var iMethod in imap.InterfaceMethods) + { + Console.WriteLine($"{iMethod.DeclaringType } {iMethod}"); + } + + Assert.Equal(1, imap.TargetMethods.Length); + foreach (var iMethod in imap.TargetMethods) + { + Console.WriteLine($"{iMethod.DeclaringType } {iMethod}"); + Assert.Equal(typeof(Outer.Inner), iMethod.DeclaringType); + } + + Console.WriteLine("Inner2"); + s = typeof(Program).Assembly.GetType("GetInterfaceMapWithStaticVirtualsAndConstraints.Outer`1+Inner2"); + s = s.MakeGenericType(typeof(int)); + i = s.GetInterface("IStatics"); + imap = s.GetInterfaceMap(i); + foreach (var iMethod in imap.InterfaceMethods) + { + Console.WriteLine($"{iMethod.DeclaringType } {iMethod}"); + } + Assert.Equal(1, imap.TargetMethods.Length); + foreach (var iMethod in imap.TargetMethods) + { + Console.WriteLine($"{iMethod.DeclaringType } {iMethod}"); + Assert.Equal(typeof(Outer.IStaticsImpl), iMethod.DeclaringType); + } + + i = s.GetInterface("IInstance"); + imap = s.GetInterfaceMap(i); + foreach (var iMethod in imap.InterfaceMethods) + { + Console.WriteLine($"{iMethod.DeclaringType } {iMethod}"); + } + Assert.Equal(1, imap.TargetMethods.Length); + foreach (var iMethod in imap.TargetMethods) + { + Console.WriteLine($"{iMethod.DeclaringType } {iMethod}"); + Assert.Equal(typeof(Outer.IInstanceImpl), iMethod.DeclaringType); + } + + return 100; + } + } + + class Outer + { + public struct Inner : IStatics, IInstance + { + public void M() where TInner : IConstraint { } + public static void MStatic() where TInner : IConstraint { } + } + + public struct Inner2 : IStaticsImpl, IInstanceImpl + { + + } + + public interface IStaticsImpl : IStatics + { + static void IStatics.MStatic() { } + } + + public interface IInstanceImpl : IInstance + { + void IInstance.M() { } + } + + public interface IStatics + { + static abstract void MStatic() where TInner2 : IConstraint; + } + + public interface IInstance + { + abstract void M() where TInner2 : IConstraint; + } + + public interface IConstraint { } + } +} diff --git a/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_78865.cs b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_78865.cs new file mode 100644 index 0000000000000..254a854ed3094 --- /dev/null +++ b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/GitHub_78865.cs @@ -0,0 +1,42 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +namespace StaticVirtualsAndMethodConstraintsAndDefaultImplementation +{ + public interface ITestItem + { + } + + public interface IStaticInterfaceBase + where T : class + where TRequest : IStaticInterfaceBase + { + static abstract int TryInvoke(TItem item, TRequest request) where TItem : ITestItem; + } + + public interface IStaticInterface : IStaticInterfaceBase + where T : class + where TRequest : IStaticInterface + { + static int IStaticInterfaceBase.TryInvoke(TItem item, TRequest request) + { + return 100; + } + } + + public class Request : IStaticInterface + { + } + + internal class Program + { + public static int Invoke(TRequest request) + where T : class + where TRequest : IStaticInterfaceBase => + TRequest.TryInvoke((ITestItem) null!, request); + + public static int Main(string[] args) => Invoke(new Request()); + } +} diff --git a/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/StaticVirtualsAndMethodConstraintsAndDefaultImplementation.cs b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/StaticVirtualsAndMethodConstraintsAndDefaultImplementation.cs new file mode 100644 index 0000000000000..254a854ed3094 --- /dev/null +++ b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/StaticVirtualsAndMethodConstraintsAndDefaultImplementation.cs @@ -0,0 +1,42 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +namespace StaticVirtualsAndMethodConstraintsAndDefaultImplementation +{ + public interface ITestItem + { + } + + public interface IStaticInterfaceBase + where T : class + where TRequest : IStaticInterfaceBase + { + static abstract int TryInvoke(TItem item, TRequest request) where TItem : ITestItem; + } + + public interface IStaticInterface : IStaticInterfaceBase + where T : class + where TRequest : IStaticInterface + { + static int IStaticInterfaceBase.TryInvoke(TItem item, TRequest request) + { + return 100; + } + } + + public class Request : IStaticInterface + { + } + + internal class Program + { + public static int Invoke(TRequest request) + where T : class + where TRequest : IStaticInterfaceBase => + TRequest.TryInvoke((ITestItem) null!, request); + + public static int Main(string[] args) => Invoke(new Request()); + } +} diff --git a/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/StaticVirtualsAndMethodConstraintsAndDefaultImplementation.csproj b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/StaticVirtualsAndMethodConstraintsAndDefaultImplementation.csproj new file mode 100644 index 0000000000000..570644f1dbcb7 --- /dev/null +++ b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/StaticVirtualsAndMethodConstraintsAndDefaultImplementation.csproj @@ -0,0 +1,8 @@ + + + Exe + + + + + From 0c377f8a30d71c61741d4b71a433b7bed85e9d09 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Mon, 17 Jul 2023 16:46:51 -0700 Subject: [PATCH 2/7] Code review tweaks --- src/coreclr/vm/methodtable.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/vm/methodtable.cpp b/src/coreclr/vm/methodtable.cpp index 3dafa74dbd67d..c7e5589319732 100644 --- a/src/coreclr/vm/methodtable.cpp +++ b/src/coreclr/vm/methodtable.cpp @@ -6456,7 +6456,6 @@ namespace pMT, FALSE, // forceBoxedEntryPoint candidateMaybe->HasMethodInstantiation() ? -// TODO SHOULD WE CHANGE THIS TO candidateMaybe->HasMethodInstantiation() && instantiateMethodInstantiation ? candidateMaybe->AsInstantiatedMethodDesc()->IMD_GetMethodInstantiation() : Instantiation(), // for method themselves that are generic FALSE, // allowInstParam @@ -9037,7 +9036,7 @@ MethodTable::ResolveVirtualStaticMethod( } // Default implementation logic, which only kicks in for default implementations when looking up on an exact interface target - if (!pInterfaceMD->IsAbstract() && !(this == g_pCanonMethodTableClass) && !IsSharedByGenericInstantiations() && instantiateMethodParameters) + if (!pInterfaceMD->IsAbstract() && !(this == g_pCanonMethodTableClass) && !IsSharedByGenericInstantiations()) { return pInterfaceMD->FindOrCreateAssociatedMethodDesc(pInterfaceMD, pInterfaceType, FALSE, pInterfaceMD->GetMethodInstantiation(), FALSE); } From 0e73eddb5fb8efcaf83e8e56287cc46fd547b985 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 18 Jul 2023 09:20:29 -0700 Subject: [PATCH 3/7] Disable test for NativeAOT compilation --- ...icVirtualsAndMethodConstraintsAndDefaultImplementation.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/StaticVirtualsAndMethodConstraintsAndDefaultImplementation.csproj b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/StaticVirtualsAndMethodConstraintsAndDefaultImplementation.csproj index 570644f1dbcb7..60e2d6921f4fd 100644 --- a/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/StaticVirtualsAndMethodConstraintsAndDefaultImplementation.csproj +++ b/src/tests/Loader/classloader/StaticVirtualMethods/RegressionTests/StaticVirtualsAndMethodConstraintsAndDefaultImplementation.csproj @@ -1,6 +1,7 @@ Exe + true From 0864b3885e747b674b57899fa02253fefc1cdacb Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 18 Jul 2023 12:36:57 -0700 Subject: [PATCH 4/7] Code review feedback --- src/coreclr/inc/enum_class_flags.h | 52 +++++++++++++++++++++++++++ src/coreclr/vm/methodtable.h | 58 ++++-------------------------- 2 files changed, 59 insertions(+), 51 deletions(-) create mode 100644 src/coreclr/inc/enum_class_flags.h diff --git a/src/coreclr/inc/enum_class_flags.h b/src/coreclr/inc/enum_class_flags.h new file mode 100644 index 0000000000000..e196da63a8b1a --- /dev/null +++ b/src/coreclr/inc/enum_class_flags.h @@ -0,0 +1,52 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#ifndef ENUM_CLASS_FLAGS_OPERATORS +#define ENUM_CLASS_FLAGS_OPERATORS + +template +inline constexpr auto operator& (T left, T right) -> decltype(T::support_use_as_flags) +{ + return static_cast(static_cast(left) & static_cast(right)); +} + +template +inline constexpr auto operator| (T left, T right) -> decltype(T::support_use_as_flags) +{ + return static_cast(static_cast(left) & static_cast(right)); +} + +template +inline constexpr auto operator^ (T left, T right) -> decltype(T::support_use_as_flags) +{ + return static_cast(static_cast(left) & static_cast(right)); +} + +template +inline constexpr auto operator~ (T value) -> decltype(T::support_use_as_flags) +{ + return static_cast(~static_cast(value)); +} + +template +inline constexpr auto operator |= (T& left, T right) -> const decltype(T::support_use_as_flags)& +{ + left = left | right; + return left; +} + +template +inline constexpr auto operator &= (T& left, T right) -> const decltype(T::support_use_as_flags)& +{ + left = left | right; + return left; +} + +template +inline constexpr auto operator ^= (T& left, T right) -> const decltype(T::support_use_as_flags)& +{ + left = left | right; + return left; +} + +#endif /* ENUM_CLASS_FLAGS_OPERATORS */ \ No newline at end of file diff --git a/src/coreclr/vm/methodtable.h b/src/coreclr/vm/methodtable.h index e3dd6ebfe5d35..a1e74ca7dda99 100644 --- a/src/coreclr/vm/methodtable.h +++ b/src/coreclr/vm/methodtable.h @@ -25,6 +25,7 @@ #include "contractimpl.h" #include "generics.h" #include "gcinfotypes.h" +#include "enum_class_flags.h" /* * Forward Declarations @@ -69,66 +70,21 @@ enum class ResolveVirtualStaticMethodFlags AllowNullResult = 1, VerifyImplemented = 2, AllowVariantMatches = 4, - InstantiateResultOverFinalMethodDesc = 8 -}; - -inline ResolveVirtualStaticMethodFlags operator& (ResolveVirtualStaticMethodFlags left, ResolveVirtualStaticMethodFlags right) -{ - return ResolveVirtualStaticMethodFlags(int(left) & int(right)); -} + InstantiateResultOverFinalMethodDesc = 8, -inline ResolveVirtualStaticMethodFlags operator| (ResolveVirtualStaticMethodFlags left, ResolveVirtualStaticMethodFlags right) -{ - return ResolveVirtualStaticMethodFlags(int(left) | int(right)); -} -inline ResolveVirtualStaticMethodFlags& operator|=(ResolveVirtualStaticMethodFlags& left, ResolveVirtualStaticMethodFlags right) -{ - left = left | right; - return left; -} -inline ResolveVirtualStaticMethodFlags operator^ (ResolveVirtualStaticMethodFlags left, ResolveVirtualStaticMethodFlags right) -{ - return ResolveVirtualStaticMethodFlags(int(left) ^ int(right)); -} + support_use_as_flags // Enable the template functions in enum_class_flags.h +}; -inline ResolveVirtualStaticMethodFlags operator~ (ResolveVirtualStaticMethodFlags value) -{ - return ResolveVirtualStaticMethodFlags(~int(value)); -} enum class FindDefaultInterfaceImplementationFlags { None, AllowVariance = 1, ThrowOnConflict = 2, - InstantiateFoundMethodDesc = 4 -}; + InstantiateFoundMethodDesc = 4, -inline FindDefaultInterfaceImplementationFlags operator& (FindDefaultInterfaceImplementationFlags left, FindDefaultInterfaceImplementationFlags right) -{ - return FindDefaultInterfaceImplementationFlags(int(left) & int(right)); -} - -inline FindDefaultInterfaceImplementationFlags operator| (FindDefaultInterfaceImplementationFlags left, FindDefaultInterfaceImplementationFlags right) -{ - return FindDefaultInterfaceImplementationFlags(int(left) | int(right)); -} - -inline FindDefaultInterfaceImplementationFlags& operator|=(FindDefaultInterfaceImplementationFlags& left, FindDefaultInterfaceImplementationFlags right) -{ - left = left | right; - return left; -} - -inline FindDefaultInterfaceImplementationFlags operator^ (FindDefaultInterfaceImplementationFlags left, FindDefaultInterfaceImplementationFlags right) -{ - return FindDefaultInterfaceImplementationFlags(int(left) ^ int(right)); -} - -inline FindDefaultInterfaceImplementationFlags operator~ (FindDefaultInterfaceImplementationFlags value) -{ - return FindDefaultInterfaceImplementationFlags(~int(value)); -} + support_use_as_flags // Enable the template functions in enum_class_flags.h +}; //============================================================================ // This is the in-memory structure of a class and it will evolve. From 3826e77a0bfd729324b3d433e9faab90d837b4c6 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 18 Jul 2023 12:43:15 -0700 Subject: [PATCH 5/7] Add test exclusion for NativeAOT --- src/tests/issues.targets | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index a97866b950e27..596a6315f22c0 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -694,6 +694,9 @@ + + https://github.com/dotnet/runtime/issues/89122 + https://github.com/dotnet/runtime/issues/88690 From 10b6b04628a0fe0cc5e32b63d1a448d360798db5 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 18 Jul 2023 16:30:31 -0700 Subject: [PATCH 6/7] Fix oops around the new templates --- src/coreclr/inc/enum_class_flags.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/coreclr/inc/enum_class_flags.h b/src/coreclr/inc/enum_class_flags.h index e196da63a8b1a..8d2a1924360e8 100644 --- a/src/coreclr/inc/enum_class_flags.h +++ b/src/coreclr/inc/enum_class_flags.h @@ -5,47 +5,47 @@ #define ENUM_CLASS_FLAGS_OPERATORS template -inline constexpr auto operator& (T left, T right) -> decltype(T::support_use_as_flags) +inline auto operator& (T left, T right) -> decltype(T::support_use_as_flags) { return static_cast(static_cast(left) & static_cast(right)); } template -inline constexpr auto operator| (T left, T right) -> decltype(T::support_use_as_flags) +inline auto operator| (T left, T right) -> decltype(T::support_use_as_flags) { - return static_cast(static_cast(left) & static_cast(right)); + return static_cast(static_cast(left) | static_cast(right)); } template -inline constexpr auto operator^ (T left, T right) -> decltype(T::support_use_as_flags) +inline auto operator^ (T left, T right) -> decltype(T::support_use_as_flags) { - return static_cast(static_cast(left) & static_cast(right)); + return static_cast(static_cast(left) ^ static_cast(right)); } template -inline constexpr auto operator~ (T value) -> decltype(T::support_use_as_flags) +inline auto operator~ (T value) -> decltype(T::support_use_as_flags) { return static_cast(~static_cast(value)); } template -inline constexpr auto operator |= (T& left, T right) -> const decltype(T::support_use_as_flags)& +inline auto operator |= (T& left, T right) -> const decltype(T::support_use_as_flags)& { left = left | right; return left; } template -inline constexpr auto operator &= (T& left, T right) -> const decltype(T::support_use_as_flags)& +inline auto operator &= (T& left, T right) -> const decltype(T::support_use_as_flags)& { - left = left | right; + left = left & right; return left; } template -inline constexpr auto operator ^= (T& left, T right) -> const decltype(T::support_use_as_flags)& +inline auto operator ^= (T& left, T right) -> const decltype(T::support_use_as_flags)& { - left = left | right; + left = left ^ right; return left; } From 14c3ecd7de6493807c18aeaa719367d8615d7f9c Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Wed, 19 Jul 2023 14:53:23 -0700 Subject: [PATCH 7/7] Update src/tests/issues.targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Michal Strehovský --- src/tests/issues.targets | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/issues.targets b/src/tests/issues.targets index d7371f61a9c81..2533601ac77c1 100644 --- a/src/tests/issues.targets +++ b/src/tests/issues.targets @@ -698,7 +698,7 @@ - https://github.com/dotnet/runtime/issues/89122 + https://github.com/dotnet/runtime/issues/89157 https://github.com/dotnet/runtime/issues/88690