Repository navigation
Conversation
|
Azure Pipelines: 16 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @steveisok, @dotnet/area-system-reflection |
Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
| } | ||
|
|
||
| [Fact] | ||
| public static void IsAssignableFrom_OpenNullable_SameTypeReturnsTrue() |
Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
|
@jkotas is the fix on this branch generally correct? I fixed it differently locally by adjusting the assert to be: |
It is not. Also, I am not convinced whether the reflection behavior for this corner case make sense. |
| // in the hierarchy, and then doing a bit of math to find the right dictionary, but since we know this is nullable | ||
| // we can do a simple double deference to do the same thing. | ||
| Debug.Assert(typeMT->InstantiationArg0() == **typeMT->PerInstInfo); | ||
| // Note that the type argument is a TypeDesc instead of a MethodTable for the open Nullable<> |
There was a problem hiding this comment.
@copilot Let's avoid using MethodTable* for values that are not guaranteed to be MethodTable* . Check what type code in System.Private.CoreLib typically uses in these situations and update to that. Do not hesitate to change the type of PerInstInfo as appropriate.
Also, change InstantiationArg0 FCall to be only used for debug-only verification, and introduce NullableType property on MethodTable that will fully managed impl optimized for Nulalble . InstantiationArg0 FCall should be only used for debug-only validation in the new property.
|
Stuck on "Error: Model "claude-opus-5" is not available." |
CastHelpers.IsNullableForTypeasserted on a checked/Debug runtime whenevertypeMTwas an openNullable<>(orNullable<T>over a generic variable), because the assert calledInstantiationArg0(), whose FCall doesTypeHandle::AsMethodTable()on what is actually aTypeVarTypeDesc. Reachable from ordinary managed code, e.g.typeof(Nullable<>).IsAssignableFrom(typeof(int)).Changes
CastHelpers.IsNullableForType— the assert now short-circuits when the type argument read out ofPerInstInfois aTypeDesc, soInstantiationArg0()is only called when it is valid.FEATURE_TYPEEQUIVALENCEbuilds the mismatch path passed the taggedTypeDescpointer toAreTypesEquivalent, which readHasTypeEquivalenceflags off a non-MethodTablepointer. ATypeDescargument can never be equal or equivalent to a boxed type, so it now returnsfalsebefore that read. This matches nativeNullable::IsNullableForTypeHelper, which goes throughTypeHandle::IsEquivalentTo.pMTNullableArg == boxedMT) is untouched — no added work on the hot unboxing path.Tests
NullableTests:[Theory]overtypeof(Nullable<>).IsAssignableFrom(...)forint,int?,GStruct<int>, plusNullable<!0>constructed over another type's generic parameter (allfalse), and a[Fact]for thetruecases (Nullable<>fromNullable<>,int?fromint).System.Reflection.MetadataLoadContextTypeTests.TestIsAssignableFrom: the same open-Nullable<>andNullable<!0>cases in the projected metadata world.Verified against a checked CoreCLR: the repro aborts with
ASSERT FAILED Expression: !IsTypeDesc()before the change and prints the expectedTrue/False/False/True/False/Falseafter.