Nit: this whole loop can just be converted to a foreach, we don't use i anywhere except accessing overridingParameters. #ByDesign

Unfortunately i is used for overriddenParameters as well as overridingParameters.

In reply to: 238440431 [](ancestors = 238440431)

Ah dang, and I thought I had gone through thoroughly here to avoid looking blind.

In reply to: 238500830 [](ancestors = 238500830,238440431)

overridingParameters[i] [](start = 158, length = 23)

If you don't want to update the whole loop, please at least do this instance. #ByDesign

This feels like it's the wrong location. The chance for the null reference happens on the invocation of .Value, right? Unless this is reporting on the implicit conversion, in which case I feel like this isn't descriptive enough, and I would still have expected a warning on .Value. #Closed

This still feels wrong.

In reply to: 238442273 [](ancestors = 238442273)

Yeah, I think this example is more illustrative of wanting to have a clearer error message here. As a user, my first thought is "The nullabilities here don't differ, why are you warning me?" #Resolved

Actually, by my reading of the spec, this is an invalid warning.

Given a user-defined conversion operator that converts from a non-nullable value type S to a non-nullable value type T, a lifted conversion operator exists that converts from S? to T?.
https://github.com/dotnet/csharplang/blob/54df8973faabe32335fa087f9d32fd959366ff83/spec/conversions.md#lifted-conversion-operators

In reply to: 238443507 [](ancestors = 238443507)

Good catch, thanks.

In reply to: 238445282 [](ancestors = 238445282,238443507)

Perhaps add a dereference of y after this? #Closed

Is this substantially different from NullableT_06, other than also having the negative case? If not, consider consolidating. #Closed

Shouldn't we have a warning here for something along the lines of "this will never be false"? #Closed

Logged #31516.

In reply to: 238456621 [](ancestors = 238456621)

Again, there should not be a arning here. Top-level nullabilities match. #Closed

case NullableAnnotation.NotAnnotated: [](start = 16, length = 37)

Would it make sense to add an assert to TypeSymbolWithAnnotations's constructor that Unknown and NotAnnotated are never combined with a Nullable<T>? #Closed

.OriginalDefinition.SpecialType != SpecialType.System_Nullable_T) [](start = 37, length = 65)

We have a IsNullableType helper. #Closed

return containingSlot < 0 ? -1 : GetNullableOfTValueSlot(operand.Type, containingSlot); [](start = 27, length = 88)

I would like to better understand why do we need to be able to get a slots for conversions. Could you provide some details? Offline would be fine. #Closed

As we discussed offline, in the following, need to track slots for a, a.Value, a.Value.B and a.Value.B.Value, and when evaluating ((A)a).B, we need to recognize the nullability of (A)a (not nullable) and the slot (a.Value).

struct A { B? B; }
struct B { }
class Program
{
    static void F(A? a)
    {
        if (a?.B != null)
            _ = ((A)a).B.Value;
    }
}

See NullableT_19.

In reply to: 238815266 [](ancestors = 238815266)

GetTypeOrReturnType [](start = 47, length = 19)

It looks like we also have VariableType helper in the base class. Would it be more appropriate to use it here? #Closed

Are we expecting to get warnings here, or is this the actual final behavior? If the former, consider logging a bug. #ByDesign

There shouldn't be any warnings here. (There were warnings in an earlier iteration of this change.)

In reply to: 238833118 [](ancestors = 238833118)

return containingSlot < 0 ? -1 : GetNullableOfTValueSlot(operand.Type, containingSlot); [](start = 28, length = 87)

Can we get here for a conversion from Nullable<Type1> to Type2? #Closed

Converting from Nullable<T1> to T2 would be either a Boxing conversion or an ExplicitUserDefined conversion.

In reply to: 238852151 [](ancestors = 238852151)

Converting from Nullable to T2 would be either a Boxing conversion or an ExplicitUserDefined conversion.

Are you sure? What about int? -> long and the like?

In reply to: 239155510 [](ancestors = 239155510,238852151)

parameterType.Equals(underlyingOperandType = operandType.GetNullableUnderlyingType(), TypeCompareKind.AllIgnoreOptions) [](start = 28, length = 119)

It feels like this condition is too strict. For example, it is valid for a lifted conversion to have long as the parameter type and int? as the operand type. #Closed

The operand -> conversion "from" type conversion above should represent the conversion from int? to long?.

In reply to: 238872741 [](ancestors = 238872741)

Do we have a test for a scenario like this? Also, the fact that we are "redefining" what operandType represents throughout this sequence, makes it very hard to reason about the code. Consider introducing locals with specific names.

In reply to: 239170622 [](ancestors = 239170622,238872741)

Added tests.

In reply to: 239174936 [](ancestors = 239174936,239170622,238872741)

operandType.IsValueType ? compilation.GetSpecialType(SpecialType.System_Nullable_T).Construct(ImmutableArray.Create(operandType)) : operandType.TypeSymbol, [](start = 32, length = 155)

Unless I am misinterpreting something, I am not sure why do we need to wrap parameterType to nullable. What observable effect would that have on the analysis? #Closed

operandAnnotation.IsAnyNullable() ? NullableAnnotation.Nullable : NullableAnnotation.NotNullable); [](start = 32, length = 98)

It is not clear why would operandAnnotation matter, the lifted conversion method is always invoked with unwrapped (not nullable) value. #Closed

operandAnnotation.IsAnyNullable() ? NullableAnnotation.Nullable : NullableAnnotation.NotNullable [](start = 32, length = 96)

It feels like operandAnnotation shouldn't matter if methodOpt.ReturnType is nullable value type, or a reference type. #Closed

operandType.IsNullableType() [](start = 43, length = 28)

I think we should keep the IsNullableType check. Malformed annotations in metadata can have nullable annotation of a non-nullable value type. #Closed