Skip to content

Fix Nullable.GetUnderlyingType for MetadataLoadContext types#125356

Draft
AaronRobinsonMSFT wants to merge 1 commit intodotnet:mainfrom
AaronRobinsonMSFT:fix/124216-nullable-getunderlyingtype
Draft

Fix Nullable.GetUnderlyingType for MetadataLoadContext types#125356
AaronRobinsonMSFT wants to merge 1 commit intodotnet:mainfrom
AaronRobinsonMSFT:fix/124216-nullable-getunderlyingtype

Conversation

@AaronRobinsonMSFT
Copy link
Member

Summary

Nullable.GetUnderlyingType() returns null for Nullable<T> types loaded via MetadataLoadContext because it uses ReferenceEquals(genericType, typeof(Nullable<>)) to identify nullable types. MLC types are RoType instances (not RuntimeType), so the reference check always fails.

This adds a name-based fallback guarded by is not RuntimeType, so only non-runtime types reach the string comparison. RuntimeType keeps the existing zero-overhead ReferenceEquals fast path.

Bug Details

The ReferenceEquals check on line 111 of Nullable.cs compares a RoType/EcmaDefinitionType against typeof(Nullable<>) (a RuntimeType). These are always different object references, even though they represent the same logical type. Type.Equals() also does not work cross-context because RoType.Equals() checks obj is RoType first, rejecting RuntimeType.

This cascades into NullabilityInfoContext, which calls GetUnderlyingType() at three callsites (lines 352, 565, 600) — causing completely wrong nullability analysis for MLC-loaded types:

  • Nullable<int> is reported as NullabilityState.NotNull instead of Nullable
  • Index mismatches in NullableAttribute.NullableFlags corrupt results for all subsequent generic type arguments in types like ValueTuple<int, int?, string, string?>

Fix

if (ReferenceEquals(genericType, typeof(Nullable<>))
    || (genericType is not RuntimeType
        && genericType.Namespace == "System"
        && genericType.Name == "Nullable`1"))
  • Zero overhead for RuntimeTypeReferenceEquals short-circuits; the is not RuntimeType guard ensures the name check is never reached for runtime types.
  • Correct for all Type implementations — MLC types, custom Type subclasses, etc.
  • Proven patternNullabilityInfoContext already uses name-based matching for attribute detection.

Tests

Added three new tests to NullableTests.cs:

  • GetUnderlyingType_MetadataLoadContext_NullableInt_ReturnsUnderlyingType — positive case
  • GetUnderlyingType_MetadataLoadContext_NonNullableTypes_ReturnsNull — negative cases (int, string, KeyValuePair)
  • GetUnderlyingType_MetadataLoadContext_OpenNullable_ReturnsNull — open generic edge case

All 21 NullableTests and 267 NullabilityInfoContextTests pass.

Fixes #124216

@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes Nullable.GetUnderlyingType() returning null for Nullable<T> types loaded via MetadataLoadContext by adding a non-RuntimeType fallback that identifies System.Nullable\1` by name/namespace.

Changes:

  • Update Nullable.GetUnderlyingType(Type) to recognize Nullable<T> when the generic type definition is not a RuntimeType.
  • Add MetadataLoadContext-based regression tests for Nullable.GetUnderlyingType() (positive/negative/open generic cases).
  • Reference System.Reflection.MetadataLoadContext from System.Runtime.Tests to enable the new tests.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/libraries/System.Private.CoreLib/src/System/Nullable.cs Adds a name/namespace fallback for non-RuntimeType generic type definitions so MLC-loaded Nullable<T> is detected correctly.
src/libraries/System.Runtime/tests/System.Runtime.Tests/System/NullableTests.cs Adds three new regression tests using MetadataLoadContext to validate the corrected behavior.
src/libraries/System.Runtime/tests/System.Runtime.Tests/System.Runtime.Tests.csproj Adds a project reference to System.Reflection.MetadataLoadContext for the new tests.

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>
@AaronRobinsonMSFT
Copy link
Member Author

Updated this PR based on review feedback from @jkotas and @MichalStrehovsky.

New approach: Added a public virtual Type.GetNullableUnderlyingType() method following the Enum.GetUnderlyingType()Type.GetEnumUnderlyingType() pattern. Nullable.GetUnderlyingType() now forwards to this virtual.

API Proposal: #125388

Changes:

  • Type.cs — New virtual Type? GetNullableUnderlyingType() with ReferenceEquals default
  • Nullable.csGetUnderlyingType forwards to the new virtual
  • RoType.cssealed override using CoreType.NullableT identity comparison
  • RuntimeType.Mono.csIsNullableOfT updated to use new virtual
  • System.Runtime.cs — API added to ref assembly
  • NullableTests.cs — Tests for both RuntimeType and MLC paths, ConditionalFact added

All 24 NullableTests + 267 NullabilityInfoContextTests pass.

@AaronRobinsonMSFT AaronRobinsonMSFT marked this pull request as draft March 10, 2026 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

MetadataLoadContext: Nullable.GetUnderlyingType() always returns null

4 participants