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.

Sign up for GitHub

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

Handle nullable annotations on user defined operators and conversions #59169

Merged
merged 7 commits into from
Feb 2, 2022
Merged

Handle nullable annotations on user defined operators and conversions #59169

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2e80b20
Honor annotations for user defined operators and conversions:
chsienki Mar 28, 2019
badc5ba
Move VisitOperands call into ApplyUserDefinedConversion
chsienki Mar 28, 2019
9b1e02c
Clarify tests
chsienki Mar 28, 2019
0211097
Merge remote-tracking branch 'chsienki/user_defined_operators_annotat…
RikkiGibson Jan 29, 2022
ecfe673
Respect annotations on user defined operators and conversions
RikkiGibson Jan 30, 2022
44008f6
Update src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
RikkiGibson Feb 2, 2022
f1d5533
Update NullableWalker.cs
RikkiGibson Feb 2, 2022
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
96 changes: 55 additions & 41 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2519,6 +2519,7 @@ private void EnterParameter(ParameterSymbol parameter, TypeWithAnnotations param
useLegacyWarnings: false,
trackMembers: false,
assignmentKind: AssignmentKind.Assignment);
Unsplit();

// If the LHS has annotations, we perform an additional check for nullable value types
CheckDisallowedNullAssignment(resultType, parameterAnnotations, equalsValue.Value.Syntax.Location);
Expand Down Expand Up @@ -3037,6 +3038,7 @@ private void DeclareLocals(ImmutableArray<LocalSymbol> locals)
else
{
valueType = VisitOptionalImplicitConversion(initializer, targetTypeOpt: inferredType ? default : type, useLegacyWarnings: true, trackMembers: true, AssignmentKind.Assignment);
Unsplit();
}

if (inferredType)
Expand Down Expand Up @@ -3600,6 +3602,7 @@ int getOrCreateSlot(int containingSlot, Symbol symbol)
conversionCompletion is not null ?
(conversionCompletion(type), null) :
VisitOptionalImplicitConversion(node.Right, type, useLegacyWarnings: false, trackMembers: true, AssignmentKind.Assignment, delayCompletionForType);
Unsplit();

if (delayCompletionForType)
{
Expand Down Expand Up @@ -3897,6 +3900,7 @@ private int GetOrCreatePlaceholderSlot(object identifier, TypeWithAnnotations ty
foreach (var expr in expressions)
{
_ = VisitOptionalImplicitConversion(expr, elementType, useLegacyWarnings: false, trackMembers: false, AssignmentKind.Assignment);
Unsplit();
}
}
else
Expand Down Expand Up @@ -4356,25 +4360,26 @@ private void ReinferBinaryOperatorAndSetResult(

// Analyze operator call properly (honoring [Disallow|Allow|Maybe|NotNull] attribute annotations) https://github.com/dotnet/roslyn/issues/32671
var parameters = method.Parameters;
visitOperandConversion(binary.Left, leftOperand, leftConversion, parameters[0], leftUnderlyingType);
visitOperandConversion(binary.Right, rightOperand, rightConversion, parameters[1], rightUnderlyingType);
visitOperandConversionAndPostConditions(binary.Left, leftOperand, leftConversion, parameters[0], leftUnderlyingType);
visitOperandConversionAndPostConditions(binary.Right, rightOperand, rightConversion, parameters[1], rightUnderlyingType);
SetUpdatedSymbol(binary, binary.Method!, method);

void visitOperandConversion(
void visitOperandConversionAndPostConditions(
BoundExpression expr,
BoundExpression operand,
Conversion conversion,
ParameterSymbol parameter,
TypeWithState operandType)
{
TypeWithAnnotations targetTypeWithNullability = parameter.TypeWithAnnotations;
var parameterAnnotations = GetParameterAnnotations(parameter);
var targetTypeWithNullability = ApplyLValueAnnotations(parameter.TypeWithAnnotations, parameterAnnotations);

if (isLifted && targetTypeWithNullability.Type.IsNonNullableValueType())
{
targetTypeWithNullability = TypeWithAnnotations.Create(MakeNullableOf(targetTypeWithNullability));
}

_ = VisitConversion(
var resultType = VisitConversion(
expr as BoundConversion,
operand,
conversion,
Expand All @@ -4385,6 +4390,9 @@ void visitOperandConversion(
useLegacyWarnings: false,
AssignmentKind.Argument,
parameter);
CheckDisallowedNullAssignment(resultType, parameterAnnotations, expr.Syntax.Location, operand);

LearnFromPostConditions(operand, GetParameterAnnotations(parameter));
}
}
else
Expand Down Expand Up @@ -6404,7 +6412,7 @@ private void VisitArgumentOutboundAssignmentsAndPostConditions(
case RefKind.In:
{
// learn from post-conditions [Maybe/NotNull, Maybe/NotNullWhen] without using an assignment
learnFromPostConditions(argument, parameterType, parameterAnnotations);
LearnFromPostConditions(argument, parameterAnnotations);
}
break;
case RefKind.Ref:
Expand Down Expand Up @@ -6587,50 +6595,53 @@ static TypeWithState applyPostConditionsWhenFalse(TypeWithState typeWithState, F

return typeWithState;
}
}

void learnFromPostConditions(BoundExpression argument, TypeWithAnnotations parameterType, FlowAnalysisAnnotations parameterAnnotations)
{
// Note: NotNull = NotNullWhen(true) + NotNullWhen(false)
bool notNullWhenTrue = (parameterAnnotations & FlowAnalysisAnnotations.NotNullWhenTrue) != 0;
bool notNullWhenFalse = (parameterAnnotations & FlowAnalysisAnnotations.NotNullWhenFalse) != 0;
/// <summary>
/// Learn from postconditions on a by-value or 'in' argument.
/// </summary>
private void LearnFromPostConditions(BoundExpression argument, FlowAnalysisAnnotations parameterAnnotations)
{
// Note: NotNull = NotNullWhen(true) + NotNullWhen(false)
bool notNullWhenTrue = (parameterAnnotations & FlowAnalysisAnnotations.NotNullWhenTrue) != 0;
bool notNullWhenFalse = (parameterAnnotations & FlowAnalysisAnnotations.NotNullWhenFalse) != 0;

// Note: MaybeNull = MaybeNullWhen(true) + MaybeNullWhen(false)
bool maybeNullWhenTrue = (parameterAnnotations & FlowAnalysisAnnotations.MaybeNullWhenTrue) != 0;
bool maybeNullWhenFalse = (parameterAnnotations & FlowAnalysisAnnotations.MaybeNullWhenFalse) != 0;

// Note: MaybeNull = MaybeNullWhen(true) + MaybeNullWhen(false)
bool maybeNullWhenTrue = (parameterAnnotations & FlowAnalysisAnnotations.MaybeNullWhenTrue) != 0;
bool maybeNullWhenFalse = (parameterAnnotations & FlowAnalysisAnnotations.MaybeNullWhenFalse) != 0;
if (maybeNullWhenTrue && maybeNullWhenFalse && !IsConditionalState && !(notNullWhenTrue && notNullWhenFalse))
{
LearnFromNullTest(argument, ref State);
}
else if (notNullWhenTrue && notNullWhenFalse
&& !IsConditionalState
&& !(maybeNullWhenTrue || maybeNullWhenFalse))
{
LearnFromNonNullTest(argument, ref State);
}
else if (notNullWhenTrue || notNullWhenFalse || maybeNullWhenTrue || maybeNullWhenFalse)
{
Split();

if (maybeNullWhenTrue && maybeNullWhenFalse && !IsConditionalState && !(notNullWhenTrue && notNullWhenFalse))
if (notNullWhenTrue)
{
LearnFromNullTest(argument, ref State);
LearnFromNonNullTest(argument, ref StateWhenTrue);
}
else if (notNullWhenTrue && notNullWhenFalse
&& !IsConditionalState
&& !(maybeNullWhenTrue || maybeNullWhenFalse))

if (notNullWhenFalse)
{
LearnFromNonNullTest(argument, ref State);
LearnFromNonNullTest(argument, ref StateWhenFalse);
}
else if (notNullWhenTrue || notNullWhenFalse || maybeNullWhenTrue || maybeNullWhenFalse)
{
Split();

if (notNullWhenTrue)
{
LearnFromNonNullTest(argument, ref StateWhenTrue);
}

if (notNullWhenFalse)
{
LearnFromNonNullTest(argument, ref StateWhenFalse);
}

if (maybeNullWhenTrue)
{
LearnFromNullTest(argument, ref StateWhenTrue);
}
if (maybeNullWhenTrue)
{
LearnFromNullTest(argument, ref StateWhenTrue);
}

if (maybeNullWhenFalse)
{
LearnFromNullTest(argument, ref StateWhenFalse);
}
if (maybeNullWhenFalse)
{
LearnFromNullTest(argument, ref StateWhenFalse);
}
}
}
Expand Down Expand Up @@ -8260,6 +8271,8 @@ private TypeWithState VisitUserDefinedConversion(
fromExplicitCast: conversionOpt?.ExplicitCastInCode ?? false,
diagnosticLocation);

LearnFromPostConditions(conversionOperand, parameterAnnotations);

TrackAnalyzedNullabilityThroughConversionGroup(operandType, conversionOpt, conversionOperand);

return operandType;
Expand Down Expand Up @@ -10742,6 +10755,7 @@ private void VisitThrow(BoundExpression? expr)
method.ReturnType, errorLocation: null, diagnostics: null);

_ = VisitOptionalImplicitConversion(expr, elementType, useLegacyWarnings: false, trackMembers: false, AssignmentKind.Return);
Unsplit();
return null;
}

Expand Down
Loading