CR feedback on fix for #13486

src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AsyncExceptionHandlerRewriter.cs

@@ -572,10 +572,10 @@ public override BoundNode VisitCatchBlock(BoundCatchBlock node)
                 var sourceOpt = node.ExceptionSourceOpt;
                 var rewrittenFilter = (BoundExpression)this.Visit(filterOpt);
                 var newFilter = sourceOpt == null ?
-                                _F.Sequence(
+                                _F.MakeSequence(
                                     storePending,
                                     rewrittenFilter) :
-                                _F.Sequence(
+                                _F.MakeSequence(
                                     storePending,
                                     AssignCatchSource((BoundExpression)this.Visit(sourceOpt), currentAwaitCatchFrame),
                                     rewrittenFilter);

src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AwaitExpressionSpiller.cs

@@ -965,7 +965,7 @@ public override BoundNode VisitLoweredConditionalAccess(BoundLoweredConditionalA
                 // isNotCalss || {clone = receiver; (object)clone != null}
                 condition = _F.LogicalOr(
                                     isNotClass,
-                                    _F.Sequence(
+                                    _F.MakeSequence(
                                         _F.AssignmentExpression(_F.Local(clone), receiver),
                                         _F.ObjectNotEqual(
                                             _F.Convert(_F.SpecialType(SpecialType.System_Object), _F.Local(clone)),

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BinaryOperator.cs

@@ -266,13 +266,19 @@ public BoundExpression VisitBinaryOperator(BoundBinaryOperator node, BoundUnaryO
                         if (loweredRight.ConstantValue == ConstantValue.True) return loweredLeft;
                         if (loweredLeft.ConstantValue == ConstantValue.True) return loweredRight;
 
+                        // note that we are using IsDefaultValue instead of False.
+                        // that is just to catch cases like default(bool) ot others resulting in 
+                        // a default bool value, that we know to be "false"
+                        // bool? generally should not reach here, since it is handled by RewriteLiftedBinaryOperator
+                        // regardless, the following code shoudl handle default(bool?) correctly since
+                        // default(bool?) & <expr> == default(bool?)  with sideeffects of <expr>
                         if (loweredLeft.IsDefaultValue())
                         {
-                            return _factory.Sequence(loweredRight, loweredLeft);
+                            return _factory.MakeSequence(loweredRight, loweredLeft);
                         }
                         if (loweredRight.IsDefaultValue())
                         {
-                            return _factory.Sequence(loweredLeft, loweredRight);
+                            return _factory.MakeSequence(loweredLeft, loweredRight);
                         }
 
                         goto default;
@@ -400,11 +406,11 @@ public BoundExpression VisitBinaryOperator(BoundBinaryOperator node, BoundUnaryO
                     case BinaryOperatorKind.ULongMultiplication:
                         if (loweredLeft.IsDefaultValue())
                         {
-                            return _factory.Sequence(loweredRight, loweredLeft);
+                            return _factory.MakeSequence(loweredRight, loweredLeft);
                         }
                         if (loweredRight.IsDefaultValue())
                         {
-                            return _factory.Sequence(loweredLeft, loweredRight);
+                            return _factory.MakeSequence(loweredLeft, loweredRight);
                         }
                         if (loweredLeft.ConstantValue?.UInt64Value == 1)
                         {
@@ -794,7 +800,7 @@ private BoundExpression MakeTruthTestForDynamicLogicalOperator(SyntaxNode syntax
                 else
                 {
                     BoundExpression falseExpr = MakeBooleanConstant(syntax, operatorKind == BinaryOperatorKind.NotEqual);
-                    return _factory.Sequence(maybeNull, falseExpr);
+                    return _factory.MakeSequence(maybeNull, falseExpr);
                 }
             }
 

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_CompoundAssignmentOperator.cs

@@ -699,7 +699,7 @@ private BoundExpression BoxReceiver(BoundExpression rewrittenReceiver, NamedType
 
                     // common production of lowered lifted operators
                     // GetValueOrDefault is known to be not sideeffecting.
-                    if (IsSpecialMember(method.OriginalDefinition, SpecialMember.System_Nullable_T_GetValueOrDefault))
+                    if (IsSpecialMember(method, SpecialMember.System_Nullable_T_GetValueOrDefault))
                     {
                         return ReadIsSideeffecting(call.ReceiverOpt);
                     }
@@ -713,7 +713,7 @@ private BoundExpression BoxReceiver(BoundExpression rewrittenReceiver, NamedType
 
         private static bool IsSpecialMember(MethodSymbol method, SpecialMember specialMember)
         {
-            Debug.Assert(method != null);
+            method = method.OriginalDefinition;
             return method.ContainingAssembly?.GetSpecialTypeMember(specialMember) == method;
         }
 

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ConditionalAccess.cs

@@ -162,7 +162,7 @@ internal BoundExpression RewriteConditionalAccess(BoundConditionalAccess node, b
 
                 case ConditionalAccessLoweringKind.TernaryCaptureReceiverByVal:
                     // capture the receiver into a temp
-                    loweredReceiver = _factory.Sequence(
+                    loweredReceiver = _factory.MakeSequence(
                                             _factory.AssignmentExpression(_factory.Local(temp), loweredReceiver),
                                             _factory.Local(temp));
 
@@ -186,7 +186,7 @@ internal BoundExpression RewriteConditionalAccess(BoundConditionalAccess node, b
 
                         if (temp != null)
                         {
-                            result = _factory.Sequence(temp, result);
+                            result = _factory.MakeSequence(temp, result);
                         }
                     }
                     break;

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Conversion.cs

@@ -640,7 +640,7 @@ private BoundExpression MakeImplicitConversion(BoundExpression rewrittenOperand,
             }
 
             var result = MakeTupleCreationExpression(syntax, rewrittenType, fieldAccessorsBuilder.ToImmutableAndFree());
-            return _factory.Sequence(savedTuple.LocalSymbol, assignmentToTemp, result);
+            return _factory.MakeSequence(savedTuple.LocalSymbol, assignmentToTemp, result);
         }
 
         private static bool NullableNeverHasValue(BoundExpression expression)

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Patterns.cs

@@ -58,7 +58,7 @@ private BoundExpression LowerDeclarationPattern(BoundDeclarationPattern pattern,
                 Debug.Assert(input.Type == pattern.LocalSymbol.Type);
                 var assignment = _factory.AssignmentExpression(_factory.Local(pattern.LocalSymbol), input);
                 var result = _factory.Literal(true);
-                return _factory.Sequence(assignment, result);
+                return _factory.MakeSequence(assignment, result);
             }
 
             return MakeDeclarationPattern(pattern.Syntax, input, pattern.LocalSymbol, requiresNullTest: true);
@@ -69,7 +69,7 @@ private BoundExpression LowerDeclarationPattern(BoundDeclarationPattern pattern,
         /// </summary>
         BoundExpression LogicalAndForPatterns(BoundExpression left, BoundExpression right)
         {
-            return IsIrrefutablePatternTest(left) ? _factory.Sequence(left, right) : _factory.LogicalAnd(left, right);
+            return IsIrrefutablePatternTest(left) ? _factory.MakeSequence(left, right) : _factory.LogicalAnd(left, right);
         }
 
         /// <summary>
@@ -125,13 +125,13 @@ BoundExpression MakeDeclarationPattern(SyntaxNode syntax, BoundExpression input,
                     var result = requiresNullTest
                         ? _factory.ObjectNotEqual(_factory.Local(target), _factory.Null(type))
                         : (BoundExpression)_factory.Literal(true);
-                    return _factory.Sequence(assignment, result);
+                    return _factory.MakeSequence(assignment, result);
                 }
                 else
                 {
                     var assignment = _factory.AssignmentExpression(_factory.Local(target), _factory.As(input, type));
                     var result = _factory.ObjectNotEqual(_factory.Local(target), _factory.Null(type));
-                    return _factory.Sequence(assignment, result);
+                    return _factory.MakeSequence(assignment, result);
                 }
             }
             else if (type.IsNullableType())
@@ -146,7 +146,7 @@ BoundExpression MakeDeclarationPattern(SyntaxNode syntax, BoundExpression input,
                 // }
                 var assignment = _factory.AssignmentExpression(_factory.Local(target), _factory.As(input, type));
                 var result = _factory.Call(_factory.Local(target), GetNullableMethod(syntax, type, SpecialMember.System_Nullable_T_get_HasValue));
-                return _factory.Sequence(assignment, result);
+                return _factory.MakeSequence(assignment, result);
             }
             else if (type.IsValueType)
             {
@@ -154,7 +154,7 @@ BoundExpression MakeDeclarationPattern(SyntaxNode syntax, BoundExpression input,
                 // is irrefutable, and we can just do the assignment and return true.
                 if (input.Type == type)
                 {
-                    return _factory.Sequence(
+                    return _factory.MakeSequence(
                         _factory.AssignmentExpression(_factory.Local(target), input),
                         _factory.Literal(true));
                 }
@@ -175,7 +175,7 @@ BoundExpression MakeDeclarationPattern(SyntaxNode syntax, BoundExpression input,
                         GetNullableMethod(syntax, tmpType, SpecialMember.System_Nullable_T_GetValueOrDefault));
                     var asg2 = _factory.AssignmentExpression(_factory.Local(target), value);
                     var result = requiresNullTest ? MakeNullableHasValue(syntax, input) : _factory.Literal(true);
-                    return _factory.Sequence(asg2, result);
+                    return _factory.MakeSequence(asg2, result);
                 }
                 else
                 {
@@ -186,7 +186,7 @@ BoundExpression MakeDeclarationPattern(SyntaxNode syntax, BoundExpression input,
                         GetNullableMethod(syntax, tmpType, SpecialMember.System_Nullable_T_GetValueOrDefault));
                     var asg2 = _factory.AssignmentExpression(_factory.Local(target), value);
                     var result = MakeNullableHasValue(syntax, _factory.Local(tmp));
-                    return _factory.Sequence(tmp, asg1, asg2, result);
+                    return _factory.MakeSequence(tmp, asg1, asg2, result);
                 }
             }
             else // type parameter
@@ -200,7 +200,7 @@ BoundExpression MakeDeclarationPattern(SyntaxNode syntax, BoundExpression input,
                 //     return s;
                 // }
                 return _factory.Conditional(_factory.Is(input, type),
-                    _factory.Sequence(_factory.AssignmentExpression(
+                    _factory.MakeSequence(_factory.AssignmentExpression(
                         _factory.Local(target),
                         _factory.Convert(type, input)),
                         _factory.Literal(true)),

src/Compilers/CSharp/Portable/Lowering/SyntheticBoundNodeFactory.cs

@@ -734,17 +734,17 @@ public BoundLocal Local(LocalSymbol local)
             return new BoundLocal(Syntax, local, null, local.Type) { WasCompilerGenerated = true };
         }
 
-        public BoundExpression Sequence(LocalSymbol temp, params BoundExpression[] parts)
+        public BoundExpression MakeSequence(LocalSymbol temp, params BoundExpression[] parts)
         {
-            return Sequence(ImmutableArray.Create<LocalSymbol>(temp), parts);
+            return MakeSequence(ImmutableArray.Create<LocalSymbol>(temp), parts);
         }
 
-        public BoundExpression Sequence(params BoundExpression[] parts)
+        public BoundExpression MakeSequence(params BoundExpression[] parts)
         {
-            return Sequence(ImmutableArray<LocalSymbol>.Empty, parts);
+            return MakeSequence(ImmutableArray<LocalSymbol>.Empty, parts);
         }
 
-        public BoundExpression Sequence(ImmutableArray<LocalSymbol> locals, params BoundExpression[] parts)
+        public BoundExpression MakeSequence(ImmutableArray<LocalSymbol> locals, params BoundExpression[] parts)
         {
             var builder = ArrayBuilder<BoundExpression>.GetInstance();
             for (int i = 0; i < parts.Length - 1; i++)
@@ -766,12 +766,12 @@ public BoundExpression Sequence(ImmutableArray<LocalSymbol> locals, params Bound
             return Sequence(locals, builder.ToImmutableAndFree(), lastExpression);
         }
 
-        public BoundExpression Sequence(BoundExpression[] sideEffects, BoundExpression result, TypeSymbol type = null)
+        public BoundSequence Sequence(BoundExpression[] sideEffects, BoundExpression result, TypeSymbol type = null)
         {
             return new BoundSequence(Syntax, ImmutableArray<LocalSymbol>.Empty, sideEffects.AsImmutableOrNull(), result, type ?? result.Type) { WasCompilerGenerated = true };
         }
 
-        public BoundExpression Sequence(ImmutableArray<LocalSymbol> locals, ImmutableArray<BoundExpression> sideEffects, BoundExpression result)
+        public BoundSequence Sequence(ImmutableArray<LocalSymbol> locals, ImmutableArray<BoundExpression> sideEffects, BoundExpression result)
         {
             return new BoundSequence(Syntax, locals, sideEffects, result, result.Type) { WasCompilerGenerated = true };
         }