Set generic type arguments nullability for value types #58036
Conversation
ghost
added
the
| public Tuple<T, string?, string>? PropertyTupleNonNullNonNull { get; set; } | ||
| public Tuple<string, string, T> PropertyTupleNonNonNonNon { get; set; } = null!; | ||
| public Tuple<T, int?, string>? PropertyTupleNonNullNonNull { get; set; } | ||
| public Tuple<int, string, T> PropertyTupleNonNonNonNon { get; set; } = null!; |
There was a problem hiding this comment.
Found another bug when there is a value type included in the parameter list, the nullability of that value type is omitted from the nullability byte array, updated these test properties to test that fix
|
|
||
| for (int i = 0, offset = 0; i < genericArguments.Length; i++) | ||
| { | ||
| if (!genericArguments[i].IsValueType) |
There was a problem hiding this comment.
when there is a value type included in the parameter list, the nullability of that value type is omitted from the nullability byte array
src/libraries/System.Private.CoreLib/src/System/Reflection/NullabilityInfoContext.cs
Outdated
Show resolved
Hide resolved
| return nullability; | ||
| nullability = new NullabilityInfo(type, state, state, elementState, genericArgumentsState); | ||
|
|
||
| if (!type.IsValueType && state != NullabilityState.Unknown) |
There was a problem hiding this comment.
Why the change to add !type.IsValueType?
There was a problem hiding this comment.
The API returns NullabilityState.NotNull for non-nullable value types, even if nullability is not enabled (unknown context), the same should happen for generics when the bound concrete type was a value type. Do not need to call TryLoadGenericMetaTypeNullability method to check if the meta definition of the member was a generic type and load the nullability info from there.
src/libraries/System.Runtime/tests/System/Reflection/NullabilityInfoContextTests.cs
Outdated
Show resolved
Hide resolved
|
Should this be ported to 6.0? |
Yes it is, was planning to do so after the PR review done /backport to release/6.0 |
|
Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1180922096 |
|
/backport to release/6.0 |
|
Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1183351744 |
3 participants
Fixes #57920
Initially, I thought no need to check generic parameter for value types but turns out it needed