Add Type.GetNullableUnderlyingType() virtual API#126905
Add Type.GetNullableUnderlyingType() virtual API#126905AaronRobinsonMSFT wants to merge 21 commits intodotnet:mainfrom
Type.GetNullableUnderlyingType() virtual API#126905Conversation
Add a new public virtual Type.GetNullableUnderlyingType() method that returns the underlying type T for Nullable<T>, or null otherwise. Nullable.GetUnderlyingType() now forwards to this virtual method. This follows the same pattern as Enum.GetUnderlyingType() forwarding to Type.GetEnumUnderlyingType(), enabling Type subclasses like MetadataLoadContext's RoType to provide correct implementations. Changes: - Type.cs: New virtual with ReferenceEquals default (works for RuntimeType) - Nullable.cs: Forward GetUnderlyingType to the new virtual - RoType.cs: Override using CoreType.NullableT identity comparison - RuntimeType.Mono.cs: Update IsNullableOfT to use new virtual - System.Runtime.cs: Add API to ref assembly - NullableTests.cs: Tests for both RuntimeType and MLC paths All 24 NullableTests + 267 NullabilityInfoContextTests pass. Fixes dotnet#124216 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Base virtual now throws NotSupportedException(SR.NotSupported_SubclassOverride) instead of falling back to ReferenceEquals check (matches IsByRefLike pattern) - Add override to RuntimeType (shared) with the ReferenceEquals logic - Add override to RuntimeType.NativeAot.cs with the same logic - Add TypeDelegator override forwarding to typeImpl.GetNullableUnderlyingType() - Add TypeDelegator entry to System.Runtime ref assembly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-getunderlyingtype
Type.GetNullableUnderlyingType() virtual API
There was a problem hiding this comment.
Pull request overview
This PR introduces a new public virtual Type.GetNullableUnderlyingType() API so non-RuntimeType Type providers (notably MetadataLoadContext’s RoType) can correctly identify closed Nullable<T> types, and updates Nullable.GetUnderlyingType(Type) to delegate to this new virtual.
Changes:
- Add
Type.GetNullableUnderlyingType()and implement/override it forRuntimeType(CoreCLR/Mono), NativeAOTRuntimeType,TypeDelegator, andMetadataLoadContext’sRoType. - Change
Nullable.GetUnderlyingType(Type)to forward toType.GetNullableUnderlyingType(). - Add System.Runtime tests covering both
RuntimeTypeandMetadataLoadContextbehavior, plus a test project reference toSystem.Reflection.MetadataLoadContext.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Type.cs | Adds the new public virtual GetNullableUnderlyingType() method. |
| src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs | Overrides GetNullableUnderlyingType() for runtime types. |
| src/coreclr/nativeaot/System.Private.CoreLib/src/System/RuntimeType.NativeAot.cs | Adds NativeAOT override of GetNullableUnderlyingType(). |
| src/libraries/System.Private.CoreLib/src/System/Reflection/TypeDelegator.cs | Forwards the new virtual through TypeDelegator. |
| src/libraries/System.Reflection.MetadataLoadContext/src/System/Reflection/TypeLoading/Types/RoType.cs | Implements nullable detection for MLC RoType. |
| src/libraries/System.Private.CoreLib/src/System/Nullable.cs | Updates Nullable.GetUnderlyingType to delegate to the new virtual. |
| src/libraries/System.Runtime/ref/System.Runtime.cs | Updates the ref assembly surface area for the new API and TypeDelegator override. |
| src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs | Switches Mono’s internal nullable check to use GetNullableUnderlyingType(). |
| src/libraries/System.Runtime/tests/System.Runtime.Tests/System/NullableTests.cs | Adds tests for Type.GetNullableUnderlyingType() and MLC scenarios. |
| src/libraries/System.Runtime/tests/System.Runtime.Tests/System.Runtime.Tests.csproj | Adds test-time project reference to System.Reflection.MetadataLoadContext. |
Copilot's findings
- Files reviewed: 10/10 changed files
- Comments generated: 3
- Use GetType(..., throwOnError: true) for clearer failure messages - Add Assert.Same(intType, underlying) and Assert.NotSame(typeof(int), underlying) to verify the returned type is the MLC-projected type, not a runtime type Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| protected override bool IsValueTypeImpl() => typeImpl.IsValueType; | ||
| protected override bool IsCOMObjectImpl() => typeImpl.IsCOMObject; | ||
| public override bool IsByRefLike => typeImpl.IsByRefLike; | ||
| public override Type? GetNullableUnderlyingType() => typeImpl.GetNullableUnderlyingType(); |
There was a problem hiding this comment.
Do you plan to file a breaking change notice on this?
If somebody uses the existing API with an old custom Reflection universe, they are going to get an exception now.
There was a problem hiding this comment.
Do you plan to file a breaking change notice on this?
I can, I wasn't going to. The answer they were getting was often bogus so in this case it seems like instead of a silent bug they are getting an explicit exception. I assume that is probably a benefit and not breaking?
There was a problem hiding this comment.
We are replacing a bogus result for a specific case (nullable type from non-runtime reflection context) with an unconditional exception for much broader set of inputs (all types from existing implementations of non-runtime reflection contexts).
The judgement of whether something needs a breaking change notice is based on what it breaks, it is not based on what it fixes. The benefits get talked about in the justification section of the breaking change notice.
…iveAOT/Mono - Remove shared RuntimeType.cs override; add per-runtime overrides instead - CoreCLR: use TypeHandle.IsNullable + InstantiationArg0() fast path, fallback to GetGenericTypeDefinition() == typeof(Nullable<>) for open generic / non-MethodTable cases - NativeAOT: use _pUnderlyingEEType->NullableType fast path, fallback to GetGenericTypeDefinition() == typeof(Nullable<>) - Mono: use GetGenericTypeDefinition() ReferenceEquals path (no MethodTable access) for compat (virtual omits it per jkotas feedback) - Add GC.KeepAlive(this) in CoreCLR after raw pointer use Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ve MLC tests - TypeBuilderInstantiation: return null (avoids breaking callers of Nullable.GetUnderlyingType on Emit-instantiated types) - SignatureConstructedGenericType: return null (same reason) - ModifiedType: delegate to _unmodifiedType.GetNullableUnderlyingType() - SignatureModifiedType: delegate to _unmodifiedType.GetNullableUnderlyingType() - Fix TypeDelegator ref assembly entry placement: move to the methods section (alphabetically after GetNestedTypes, before GetProperties) - Fix CoreCLR implementation: cache AsMethodTable() result in local pMT to avoid double-call and improve clarity - Move MLC tests from System.Runtime.Tests/NullableTests.cs to System.Reflection.MetadataLoadContext/tests/TypeTests.Nullable.cs; use TestUtils.GetPathToCoreAssembly() instead of RuntimeEnvironment.GetRuntimeDirectory() - Remove MLC ProjectReference from System.Runtime.Tests.csproj Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a new virtual Type.GetNullableUnderlyingType() so non-RuntimeType implementations (notably MetadataLoadContext’s RoType) can correctly detect Nullable<T>, and updates Nullable.GetUnderlyingType(Type) to forward to the virtual.
Changes:
- Introduces
Type.GetNullableUnderlyingType()and wiresNullable.GetUnderlyingType()to call it. - Implements overrides for CoreCLR, Mono, NativeAOT
RuntimeType, and key wrapper types (TypeDelegator, modified/signature types). - Adds tests for
RuntimeTypeandMetadataLoadContextbehavior.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Type.cs | Adds new virtual GetNullableUnderlyingType() API. |
| src/libraries/System.Private.CoreLib/src/System/Nullable.cs | Forwards Nullable.GetUnderlyingType to Type.GetNullableUnderlyingType(). |
| src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs | Implements CoreCLR RuntimeType override using MethodTable fast-path + fallback. |
| src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs | Implements Mono RuntimeType override; updates IsNullableOfT to use it. |
| src/coreclr/nativeaot/System.Private.CoreLib/src/System/RuntimeType.NativeAot.cs | Implements NativeAOT RuntimeType override. |
| src/libraries/System.Private.CoreLib/src/System/Reflection/TypeDelegator.cs | Forwards the new virtual to the underlying Type. |
| src/libraries/System.Private.CoreLib/src/System/Reflection/ModifiedType.cs | Forwards the new virtual to the unmodified type. |
| src/libraries/System.Private.CoreLib/src/System/Reflection/SignatureModifiedType.cs | Forwards the new virtual to the unmodified type. |
| src/libraries/System.Private.CoreLib/src/System/Reflection/SignatureConstructedGenericType.cs | Explicitly returns null for the new virtual. |
| src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilderInstantiation.cs | Explicitly returns null for the new virtual. |
| src/libraries/System.Reflection.MetadataLoadContext/src/System/Reflection/TypeLoading/Types/RoType.cs | Implements MLC RoType override via CoreType.NullableT identity. |
| src/libraries/System.Runtime/ref/System.Runtime.cs | Adds the new API to the ref assembly and TypeDelegator override surface. |
| src/libraries/System.Runtime/tests/System.Runtime.Tests/System/NullableTests.cs | Adds tests for Type.GetNullableUnderlyingType() on runtime types. |
| src/libraries/System.Reflection.MetadataLoadContext/tests/src/Tests/Type/TypeTests.Nullable.cs | Adds MLC-specific tests for nullable detection, including open-generic case. |
| src/libraries/System.Reflection.MetadataLoadContext/tests/System.Reflection.MetadataLoadContext.Tests.csproj | Includes the new MLC test file in compilation. |
Copilot's findings
Comments suppressed due to low confidence (1)
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/NullableTests.cs:99
- The new
GetNullableUnderlyingTypetest cases don't cover the open generic definitiontypeof(Nullable<>). Given the API contract is "closed generic Nullable only", add a case assertingtypeof(Nullable<>).GetNullableUnderlyingType()returnsnullto prevent regressions (and to catch the current behavior in the runtime overrides).
[Theory]
[InlineData(typeof(int?), typeof(int))]
[InlineData(typeof(int), null)]
[InlineData(typeof(G<int>), null)]
public static void GetNullableUnderlyingType_RuntimeType(Type type, Type? expected)
{
Assert.Equal(expected, type.GetNullableUnderlyingType());
}
- Files reviewed: 15/15 changed files
- Comments generated: 4
…tiation behavior - Use IsConstructedGenericType (not IsGenericType) in NativeAOT and Mono overrides to correctly return null for the open generic typeof(Nullable<>) instead of incorrectly returning type parameter T - Fix TypeBuilderInstantiation.GetNullableUnderlyingType to return the type argument when _genericType is typeof(Nullable<>), preserving the behavior that existed via Nullable.GetUnderlyingType before this change - Add [InlineData(typeof(Nullable<>), null)] to the RuntimeType theory test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… in NullableTests.cs
…t NET GetNullableUnderlyingType() is new in .NET 11. The MLC library multi-targets net11.0, net10.0, netstandard2.0, and netfx. Using #if NET caused CS0115 (no suitable method found to override) when building for net10.0, since #if NET is true for net10.0 but Type.GetNullableUnderlyingType() doesn't exist there. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a new public virtual Type.GetNullableUnderlyingType() API so non-RuntimeType implementations (notably MetadataLoadContext’s RoType) can correctly recognize closed Nullable<T> and provide the underlying T. Nullable.GetUnderlyingType(Type) is updated to delegate to this virtual, mirroring the existing Enum.GetUnderlyingType() → Type.GetEnumUnderlyingType() pattern.
Changes:
- Introduce
Type.GetNullableUnderlyingType()(virtual) and wireNullable.GetUnderlyingType(Type)to call it for constructed generic types. - Implement/forward the virtual across CoreCLR, Mono, NativeAOT, MetadataLoadContext (
RoType), and common wrapper types (TypeDelegator, modified/signature types,TypeBuilderInstantiation). - Add tests covering both
RuntimeTypeandMetadataLoadContextbehavior.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Type.cs | Adds the new public virtual GetNullableUnderlyingType() API and docs. |
| src/libraries/System.Private.CoreLib/src/System/Nullable.cs | Updates Nullable.GetUnderlyingType to delegate to Type.GetNullableUnderlyingType(). |
| src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs | CoreCLR override that recognizes Nullable<T> via method table fast-path and fallback. |
| src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs | Mono override implementation + internal IsNullableOfT updated to use the new virtual. |
| src/coreclr/nativeaot/System.Private.CoreLib/src/System/RuntimeType.NativeAot.cs | NativeAOT override implementation. |
| src/libraries/System.Private.CoreLib/src/System/Reflection/TypeDelegator.cs | Forwards the new virtual to the delegated typeImpl. |
| src/libraries/System.Private.CoreLib/src/System/Reflection/ModifiedType.cs | Forwards the new virtual to the underlying unmodified type. |
| src/libraries/System.Private.CoreLib/src/System/Reflection/SignatureModifiedType.cs | Forwards the new virtual to the underlying unmodified type. |
| src/libraries/System.Private.CoreLib/src/System/Reflection/SignatureConstructedGenericType.cs | Sealed override returning null to preserve existing signature-type semantics. |
| src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilderInstantiation.cs | Adds override to surface underlying T for constructed Nullable<T> in Reflection.Emit instantiations. |
| src/libraries/System.Reflection.MetadataLoadContext/src/System/Reflection/TypeLoading/Types/RoType.cs | Adds RoType override (guarded) using core-type identity comparison for Nullable<T>. |
| src/libraries/System.Runtime/ref/System.Runtime.cs | Updates ref surface area for Type and TypeDelegator. |
| src/libraries/System.Runtime/tests/System.Runtime.Tests/System/NullableTests.cs | Adds direct Type.GetNullableUnderlyingType() runtime tests. |
| src/libraries/System.Reflection.MetadataLoadContext/tests/src/Tests/Type/TypeTests.Nullable.cs | Adds MLC tests validating both Nullable.GetUnderlyingType and the direct virtual call. |
| src/libraries/System.Reflection.MetadataLoadContext/tests/System.Reflection.MetadataLoadContext.Tests.csproj | Includes the new MLC test file in the test project. |
Copilot's findings
- Files reviewed: 15/15 changed files
- Comments generated: 0 new
- Add overrides on TypeBuilder, EnumBuilder, GenericTypeParameterBuilder so Reflection.Emit types don't throw from the base virtual. - Add safe null default on SignatureType base class for signature subclasses. - Fix TypeBuilderInstantiation, SignatureConstructedGenericType, ModifiedType to consult the underlying definition via GetNullableUnderlyingType. - Drop IsGenericTypeDefinition short-circuit in CoreCLR RuntimeType so the virtual returns T even for typeof(Nullable<>); static Nullable.GetUnderlyingType preserves COMPAT by filtering generic definitions. - Use IsGenericType (not IsConstructedGenericType) in Mono RuntimeType and MetadataLoadContext RoType for consistency. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…bleUnderlyingType() Type.GetNullableUnderlyingType() now returns the generic type parameter T for typeof(Nullable<>), matching the new contract that the open generic type definition is considered nullable. Nullable.GetUnderlyingType(Type) continues to return null for generic type definitions for COMPAT. - Type.cs: updated XML doc. - RuntimeType.CoreCLR.cs / RuntimeType.NativeAot.cs: return GetGenericArguments()[0] for the open Nullable<> since the native fast paths can't yield a MethodTable for the formal type parameter T. - MetadataLoadContext RoType: switched from GenericTypeArguments[0] (empty for open generic) to GetGenericArguments()[0]. - Tests updated to expect non-null for typeof(Nullable<>) via Type.GetNullableUnderlyingType(), and still null via Nullable.GetUnderlyingType(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-getunderlyingtype
- Make RuntimeTypeInfo.GetNullableUnderlyingType non-virtual with a proper implementation covering closed Nullable<T>, the open Nullable<> definition, and Nullable<T> where T is a generic parameter of another type (the case from jkotas's repro). - Drop the open-generic special case in RuntimeType.NativeAot.cs and fall through to RuntimeTypeInfo.GetNullableUnderlyingType. - Remove obsolete comment on TypeBuilder.GetNullableUnderlyingType. - Add NullableTests coverage for the foreign-generic-parameter case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SignatureModifiedType also overrides this method to surface the unmodified type's Nullable<T> behavior, so the previous comment was inaccurate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Added When you commit this breaking change:
Tagging @dotnet/compat for awareness of the breaking change. |
…riable When Nullable<T> is constructed over a generic type parameter (e.g. typeof(Nullable<>).MakeGenericType(typeof(MyStruct<>).GetGenericArguments()[0])), the resulting MethodTable has IsNullable but InstantiationArg0() returns a TypeDesc, not a MethodTable*. Casting that to MethodTable* and feeding it to RuntimeTypeHandle.GetRuntimeTypeFromHandle trips the Fall back to managed GetGenericArguments()[0] whenever the Nullable<T> contains generic variables (covers both the open Nullable<> definition and Nullable<ABC> over a generic parameter). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| } | ||
| public override MethodBase? DeclaringMethod => null; | ||
| public override Type GetGenericTypeDefinition() { return _genericType; } | ||
| public override Type? GetNullableUnderlyingType() => _genericType.GetNullableUnderlyingType() is not null ? _typeArguments[0] : null; |
There was a problem hiding this comment.
Add some tests for the ref emit overrides?
| public virtual Type? GetNullableUnderlyingType() | ||
| { | ||
| if (!IsGenericType) | ||
| return null; | ||
| if (GetGenericTypeDefinition() != typeof(Nullable<>)) | ||
| return null; | ||
| return IsGenericTypeDefinition ? GenericTypeParameters[0] : GenericTypeArguments[0]; | ||
| } |
There was a problem hiding this comment.
| public virtual Type? GetNullableUnderlyingType() | |
| { | |
| if (!IsGenericType) | |
| return null; | |
| if (GetGenericTypeDefinition() != typeof(Nullable<>)) | |
| return null; | |
| return IsGenericTypeDefinition ? GenericTypeParameters[0] : GenericTypeArguments[0]; | |
| } | |
| public virtual Type? GetNullableUnderlyingType() | |
| { | |
| return null; | |
| } |
And override to NativeFormatRuntimeNamedTypeInfo instead?
public override Type? GetNullableUnderlyingType()
{
return (this.ToType() == typeof(Nullable<>)) ? RuntimeGenericTypeParameters[0].ToType() : null;
}
| public override bool ContainsGenericParameters => _unmodifiedType.ContainsGenericParameters; | ||
| public override Type GetGenericTypeDefinition() => _unmodifiedType.GetGenericTypeDefinition(); | ||
| public override bool IsGenericType => _unmodifiedType.IsGenericType; | ||
| public override Type? GetNullableUnderlyingType() => _unmodifiedType.GetNullableUnderlyingType() is not null ? GetGenericArguments()[0] : null; |
There was a problem hiding this comment.
Add some tests for the modified type and signature type overrides?
Closes #125388
Fixes #124216
Summary
Adds a new public virtual
Type.GetNullableUnderlyingType()method so thatTypesubclasses (e.g.MetadataLoadContext'sRoType) can correctly identifyNullabletypes.Nullable.GetUnderlyingType()now forwards to this virtual.This follows the same pattern as
Enum.GetUnderlyingType()forwarding toType.GetEnumUnderlyingType().Contract
Type.GetNullableUnderlyingType()returns a non-nullresult for both:Nullable<T>(returnsT), andtypeof(Nullable<>)(returns the generic type parameterT).Nullable.GetUnderlyingType(Type)continues to returnnullfor the generic type definition for COMPAT.Changes
Type.cs: Newpublic virtualthat throwsNotSupportedException(SR.NotSupported_SubclassOverride)(matchesIsByRefLikepattern per @MichalStrehovsky's feedback). XML doc documents that the open genericNullable<>is treated as nullable and yields the generic type parameter.RuntimeType.cs(CoreCLR + Mono): Override that handles both constructed and open-generic cases. OpenNullable<>returnsGetGenericArguments()[0]since the native fast-path can't yield aMethodTablefor the formal type parameterT.RuntimeType.NativeAot.cs: Same handling for NativeAOT.TypeDelegator.cs: Override forwarding totypeImpl.GetNullableUnderlyingType().RoType.cs(MetadataLoadContext): Override usingCoreType.NullableTidentity comparison; usesGetGenericArguments()[0]so the openNullable<>returns the MLC-projected generic parameter rather than indexing emptyGenericTypeArguments.SignatureConstructedGenericType.cs/TypeBuilderInstantiation.cs/ModifiedType.cs/SignatureModifiedType.cs: Overrides that delegate through the generic definition.Nullable.cs:Nullable.GetUnderlyingTypenow delegates to the new virtual after preserving the COMPAT short-circuit for generic type definitions.System.Runtime.cs(ref assembly): New API +TypeDelegatoroverride entry.NullableTests.csandMetadataLoadContextTypeTests.Nullable.cscover RuntimeType, MetadataLoadContext, and the new open-generic behavior (non-nullfromType.GetNullableUnderlyingType(),nullfromNullable.GetUnderlyingType()).Note
This PR description was updated with assistance from GitHub Copilot.