Skip to content

Commit d219c4f

Browse files
authored
Convert operand of true/false operator invoked as part of &&/||operator evaluation (#78629)
Fixes #78609.
1 parent fcb913e commit d219c4f

File tree

7 files changed

+730
-48
lines changed

7 files changed

+730
-48
lines changed

src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,17 +1156,36 @@ private BoundExpression BindConditionalLogicalOperator(BinaryExpressionSyntax no
11561156
CheckConstraintLanguageVersionAndRuntimeSupportForOperator(node, kind == BinaryOperatorKind.LogicalAnd ? falseOperator : trueOperator,
11571157
isUnsignedRightShift: false, signature.ConstrainedToTypeOpt, diagnostics);
11581158

1159+
var operandPlaceholder = new BoundValuePlaceholder(resultLeft.Syntax, resultLeft.Type).MakeCompilerGenerated();
1160+
var trueFalseParameterType = (resultKind.Operator() == BinaryOperatorKind.And ? falseOperator : trueOperator).Parameters[0].Type;
1161+
CompoundUseSiteInfo<AssemblySymbol> useSiteInfo = GetNewCompoundUseSiteInfo(diagnostics);
1162+
var conversion = this.Conversions.ClassifyConversionFromType(resultLeft.Type, trueFalseParameterType, isChecked: CheckOverflowAtRuntime, ref useSiteInfo);
1163+
Debug.Assert(conversion.IsImplicit);
1164+
1165+
BoundExpression operandConversion = CreateConversion(
1166+
resultLeft.Syntax,
1167+
operandPlaceholder,
1168+
conversion,
1169+
isCast: false,
1170+
conversionGroupOpt: null,
1171+
trueFalseParameterType,
1172+
diagnostics);
1173+
1174+
diagnostics.Add(operandConversion.Syntax, useSiteInfo);
1175+
11591176
return new BoundUserDefinedConditionalLogicalOperator(
11601177
node,
11611178
resultKind,
1162-
resultLeft,
1163-
resultRight,
11641179
signature.Method,
11651180
trueOperator,
11661181
falseOperator,
1182+
operandPlaceholder,
1183+
operandConversion,
11671184
signature.ConstrainedToTypeOpt,
11681185
lookupResult,
11691186
originalUserDefinedOperators,
1187+
resultLeft,
1188+
resultRight,
11701189
signature.ReturnType);
11711190
}
11721191
else
@@ -1234,8 +1253,35 @@ private bool IsValidDynamicCondition(BoundExpression left, bool isNegative, Bind
12341253
return false;
12351254
}
12361255

1256+
// Strictly speaking, we probably should be digging through nullable type here to look for an operator
1257+
// declared by the underlying type. However, it doesn't look like dynamic binder is able to deal with
1258+
// nullable types while evaluating logical binary operators. Exceptions that it throws look like:
1259+
//
1260+
// Microsoft.CSharp.RuntimeBinder.RuntimeBinderException : The type ('S1?') must contain declarations of operator true and operator false
1261+
// Stack Trace:
1262+
// at CallSite.Target(Closure, CallSite, Object, Object)
1263+
// at System.Dynamic.UpdateDelegates.UpdateAndExecute2[T0,T1,TRet](CallSite site, T0 arg0, T1 arg1)
1264+
//
1265+
// Microsoft.CSharp.RuntimeBinder.RuntimeBinderException : Operator '&&' cannot be applied to operands of type 'S1' and 'S1?'
1266+
// Stack Trace:
1267+
// at CallSite.Target(Closure, CallSite, Object, Nullable`1)
1268+
// at System.Dynamic.UpdateDelegates.UpdateAndExecute2[T0,T1,TRet](CallSite site, T0 arg0, T1 arg1)
12371269
var namedType = type as NamedTypeSymbol;
12381270
var result = HasApplicableBooleanOperator(namedType, isNegative ? WellKnownMemberNames.FalseOperatorName : WellKnownMemberNames.TrueOperatorName, type, ref useSiteInfo, out userDefinedOperator);
1271+
1272+
if (result)
1273+
{
1274+
Debug.Assert(userDefinedOperator is not null);
1275+
1276+
var operandPlaceholder = new BoundValuePlaceholder(left.Syntax, left.Type).MakeCompilerGenerated();
1277+
TypeSymbol parameterType = userDefinedOperator.Parameters[0].Type;
1278+
1279+
implicitConversion = this.Conversions.ClassifyConversionFromType(operandPlaceholder.Type, parameterType, isChecked: CheckOverflowAtRuntime, ref useSiteInfo);
1280+
Debug.Assert(implicitConversion.IsImplicit);
1281+
1282+
CreateConversion(left.Syntax, operandPlaceholder, implicitConversion, isCast: false, conversionGroupOpt: null, parameterType, diagnostics);
1283+
}
1284+
12391285
diagnostics.Add(left.Syntax, useSiteInfo);
12401286

12411287
return result;

src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,15 @@
501501
<Field Name="LogicalOperator" Type="MethodSymbol"/>
502502
<Field Name="TrueOperator" Type="MethodSymbol"/>
503503
<Field Name="FalseOperator" Type="MethodSymbol"/>
504+
505+
<!-- Used as an operand to bind TrueFalseOperandConversion -->
506+
<Field Name="TrueFalseOperandPlaceholder" Type="BoundValuePlaceholder?" SkipInVisitor="true" Null="allow"/>
507+
<!--
508+
Could be a BoundConversion or, in some cases of an identity conversion, the OperandPlaceholder, or null for no conversion case
509+
If the node survives lowering, this property in it must be null.
510+
-->
511+
<Field Name="TrueFalseOperandConversion" Type="BoundExpression?" SkipInVisitor="true" Null="allow"/>
512+
504513
<Field Name="ConstrainedToTypeOpt" Type="TypeSymbol?" Null="allow"/>
505514
<Field Name="ResultKind" PropertyOverrides="true" Type="LookupResultKind"/>
506515
<!--The set of method symbols from which this operator's method was chosen.

src/Compilers/CSharp/Portable/BoundTree/Constructors.cs

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -454,39 +454,6 @@ public BoundBinaryOperator Update(UncommonData uncommonData)
454454
}
455455
}
456456

457-
internal sealed partial class BoundUserDefinedConditionalLogicalOperator
458-
{
459-
public BoundUserDefinedConditionalLogicalOperator(
460-
SyntaxNode syntax,
461-
BinaryOperatorKind operatorKind,
462-
BoundExpression left,
463-
BoundExpression right,
464-
MethodSymbol logicalOperator,
465-
MethodSymbol trueOperator,
466-
MethodSymbol falseOperator,
467-
TypeSymbol? constrainedToTypeOpt,
468-
LookupResultKind resultKind,
469-
ImmutableArray<MethodSymbol> originalUserDefinedOperatorsOpt,
470-
TypeSymbol type,
471-
bool hasErrors = false)
472-
: this(
473-
syntax,
474-
operatorKind,
475-
logicalOperator,
476-
trueOperator,
477-
falseOperator,
478-
constrainedToTypeOpt,
479-
resultKind,
480-
originalUserDefinedOperatorsOpt,
481-
left,
482-
right,
483-
type,
484-
hasErrors)
485-
{
486-
Debug.Assert(operatorKind.IsUserDefined() && operatorKind.IsLogical());
487-
}
488-
}
489-
490457
internal sealed partial class BoundParameter
491458
{
492459
public BoundParameter(SyntaxNode syntax, ParameterSymbol parameterSymbol, bool hasErrors = false)

src/Compilers/CSharp/Portable/BoundTree/NullabilityRewriter.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ private BoundNode VisitBinaryOperatorBase(BoundBinaryOperatorBase binaryOperator
105105
right,
106106
type!),
107107
// https://github.com/dotnet/roslyn/issues/35031: We'll need to update logical.LogicalOperator
108-
BoundUserDefinedConditionalLogicalOperator logical => logical.Update(logical.OperatorKind, logical.LogicalOperator, logical.TrueOperator, logical.FalseOperator, logical.ConstrainedToTypeOpt, logical.ResultKind, logical.OriginalUserDefinedOperatorsOpt, leftChild, right, type!),
108+
BoundUserDefinedConditionalLogicalOperator logical => logical.Update(logical.OperatorKind, logical.LogicalOperator, logical.TrueOperator, logical.FalseOperator, logical.TrueFalseOperandPlaceholder, logical.TrueFalseOperandConversion, logical.ConstrainedToTypeOpt, logical.ResultKind, logical.OriginalUserDefinedOperatorsOpt, leftChild, right, type!),
109109
_ => throw ExceptionUtilities.UnexpectedValue(currentBinary.Kind),
110110
};
111111

src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs

Lines changed: 14 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)