Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved nullable '=='/'!=' analysis #53198

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 28 additions & 14 deletions src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2697,7 +2697,7 @@ private bool TryVisitConditionalAccess(BoundExpression node, [NotNullWhen(true)]
var access = node switch
{
BoundConditionalAccess ca => ca,
BoundConversion { Conversion: Conversion innerConversion, Operand: BoundConditionalAccess ca } when CanPropagateStateWhenNotNull(ca, innerConversion) => ca,
BoundConversion { Conversion: Conversion conversion, Operand: BoundConditionalAccess ca } when CanPropagateStateWhenNotNull(conversion) => ca,
_ => null
};

Expand All @@ -2713,30 +2713,29 @@ private bool TryVisitConditionalAccess(BoundExpression node, [NotNullWhen(true)]

stateWhenNotNull = default;
return false;

}

/// <summary>
/// "State when not null" can only propagate out of a conditional access if
/// it is not subject to a user-defined conversion whose parameter is not of a non-nullable value type.
/// </summary>
protected static bool CanPropagateStateWhenNotNull(BoundConditionalAccess operand, Conversion conversion)
protected static bool CanPropagateStateWhenNotNull(Conversion conversion)
{
if (conversion.Kind is not (ConversionKind.ImplicitUserDefined or ConversionKind.ExplicitUserDefined))
if (!conversion.IsValid)
{
return true;
return false;
}

var method = conversion.Method;
Debug.Assert(method is not null);
Debug.Assert(method.ParameterCount is 1);
var param = method.Parameters[0];
if (operand.Type.IsNullableType() && param.Type.IsNonNullableValueType())
if (!conversion.IsUserDefined)
{
return true;
}

return false;
var method = conversion.Method;
Debug.Assert(method is object);
Debug.Assert(method.ParameterCount is 1);
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
var param = method.Parameters[0];
return param.Type.IsNonNullableValueType();
}

/// <summary>
Expand All @@ -2759,17 +2758,27 @@ private bool VisitPossibleConditionalAccess(BoundExpression node, [NotNullWhen(t

private void VisitConditionalAccess(BoundConditionalAccess node, out TLocalState stateWhenNotNull)
{
VisitRvalue(node.Receiver);
// The receiver may also be a conditional access.
// `(a?.b(x = 1))?.c(y = 1)`
if (VisitPossibleConditionalAccess(node.Receiver, out var receiverStateWhenNotNull))
{
stateWhenNotNull = receiverStateWhenNotNull;
}
else
{
Unsplit();
stateWhenNotNull = this.State.Clone();
}

if (node.Receiver.ConstantValue != null && !IsConstantNull(node.Receiver))
{
// Consider a scenario like `"a"?.M0(x = 1)?.M0(y = 1)`.
// We can "know" that `.M0(x = 1)` was evaluated unconditionally but not `M0(y = 1)`.
// Therefore we do a VisitPossibleConditionalAccess here which unconditionally includes the "after receiver" state in State
// and includes the "after subsequent conditional accesses" in stateWhenNotNull
if (VisitPossibleConditionalAccess(node.AccessExpression, out var innerStateWhenNotNull))
if (VisitPossibleConditionalAccess(node.AccessExpression, out var firstAccessStateWhenNotNull))
{
stateWhenNotNull = innerStateWhenNotNull;
stateWhenNotNull = firstAccessStateWhenNotNull;
}
else
{
Expand All @@ -2784,6 +2793,10 @@ private void VisitConditionalAccess(BoundConditionalAccess node, out TLocalState
{
SetUnreachable();
}
else
{
SetState(stateWhenNotNull);
}

// We want to preserve stateWhenNotNull from accesses in the same "chain":
// a?.b(out x)?.c(out y); // expected to preserve stateWhenNotNull from both ?.b(out x) and ?.c(out y)
Expand All @@ -2792,6 +2805,7 @@ private void VisitConditionalAccess(BoundConditionalAccess node, out TLocalState
BoundExpression expr = node.AccessExpression;
while (expr is BoundConditionalAccess innerCondAccess)
{
Debug.Assert(innerCondAccess.Receiver is not (BoundConditionalAccess or BoundConversion));
// we assume that non-conditional accesses can never contain conditional accesses from the same "chain".
// that is, we never have to dig through non-conditional accesses to find and handle conditional accesses.
VisitRvalue(innerCondAccess.Receiver);
Expand Down
Loading