Move optimization around use of Nullable type in Boolean Expressions into...

Move optimization around use of Nullable type in Boolean Expressions into LocalRewriter and eliminate invalid optimization. (#38802)

* Move optimization around use of Nullable type in Boolean Expressions into LocalRewriter and eliminate invalid optimization.

Fixes #38305.

The goal of optimization is to eliminate the need to deal with instances of Nullable(Of Boolean) type as early as possible, and, as a result, simplify evaluation of built-in OrElse/AndAlso operators by eliminating the need to use three-valued Boolean logic. The optimization is possible because when an entire Boolean Expression is evaluated to Null, that has the same effect as if result of evaluation was False. However, we do want to preserve the original order of evaluation, according to language rules.

* Fix an old bug masked by #38305.

* Fix an old bug masked by #38305.

* More fixes fo old bugs masked by #38305, plus optimizations that wouldn't be performed by compiler.

* PR feedback

docs/compilers/Visual Basic/Compiler Breaking Changes - post VS2019.md

+## This document lists known breaking changes in Roslyn in *Visual Studio 2019 Update 1* and beyond compared to *Visual Studio 2019*.
+
+*Breaks are formatted with a monotonically increasing numbered list to allow them to referenced via shorthand (i.e., "known break #1").
+Each entry should include a short description of the break, followed by either a link to the issue describing the full details of the break or the full details of the break inline.*
+
+1. https://github.com/dotnet/roslyn/issues/38305 
+
+Compiler used to generate incorrect code when a built-in comparison operator producing Boolean? was used
+as an operand of a logical short-circuiting operator used as a Boolean expression.
+For example, for an expression 
+```
+    GetBool3() = True AndAlso GetBool2()
+```
+    and functions
+```
+    Function GetBool2() As Boolean
+        System.Console.WriteLine("GetBool2")
+        Return True
+    End Function
+    Shared Function GetBool3() As Boolean?
+         Return Nothing
+    End Function
+```
+
+it is expected that GetBool2 function going to be called. This is also the expected behavior outside of
+a Boolean expression, but in context of a Boolean expression the GetBool2 function was not called.
+Compiler now generates code that follows language semantics and calls GetBool2 function for the expression above.
\ No newline at end of file

src/Compilers/VisualBasic/Portable/Binding/Binder_Expressions.vb

@@ -91,7 +91,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                     ' parent expression.
 
                     ' Dev10 allows parenthesized type expressions, let's bind as a general expression first.
-                    Dim operand As BoundExpression = BindExpression(DirectCast(node, ParenthesizedExpressionSyntax).Expression, False, False, eventContext, diagnostics)
+                    Dim operand As BoundExpression = BindExpression(DirectCast(node, ParenthesizedExpressionSyntax).Expression,
+                                                                    isInvocationOrAddressOf:=False,
+                                                                    isOperandOfConditionalBranch:=isOperandOfConditionalBranch,
+                                                                    eventContext, diagnostics)
 
                     If operand.Kind = BoundKind.TypeExpression Then
                         Dim asType = DirectCast(operand, BoundTypeExpression)

src/Compilers/VisualBasic/Portable/Binding/Binder_Operators.vb

@@ -319,7 +319,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
             Dim operatorResultType As TypeSymbol = operandType
 
             Dim forceToBooleanType As TypeSymbol = Nothing
-            Dim applyIsTrue As Boolean = False
 
             Select Case preliminaryOperatorKind
 
@@ -353,13 +352,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                                                      If(preliminaryOperatorKind = BinaryOperatorKind.Equals,
                                                         ERRID.WRN_EqualToLiteralNothing, ERRID.WRN_NotEqualToLiteralNothing)))
                             End If
-
-                            If isOperandOfConditionalBranch Then
-                                ' TODO: I believe the IsTrue is just an optimization to prevent Nullable from unnecessary bubbling up the tree.
-                                ' Perhaps we can do this optimization as a rewrite.
-                                applyIsTrue = True
-                                forceToBooleanType = booleanType
-                            End If
                         Else
                             If Not operatorResultType.IsObjectType() Then
                                 operatorResultType = booleanType
@@ -524,17 +516,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                 End If
             End If
 
-            Dim result As BoundExpression = New BoundBinaryOperator(node, operatorKind, left, right, CheckOverflow, value, operatorResultType, hasError)
-
-            Debug.Assert(Not applyIsTrue OrElse forceToBooleanType IsNot Nothing)
+            Dim result As BoundExpression = New BoundBinaryOperator(node, operatorKind Or If(isOperandOfConditionalBranch, BinaryOperatorKind.IsOperandOfConditionalBranch, Nothing),
+                                                                    left, right, CheckOverflow, value, operatorResultType, hasError)
 
             If forceToBooleanType IsNot Nothing Then
                 Debug.Assert(forceToBooleanType.IsBooleanType())
 
-                If applyIsTrue Then
-                    Return ApplyNullableIsTrueOperator(result, forceToBooleanType)
-                End If
-
                 result = ApplyConversion(node, forceToBooleanType, result, isExplicit:=True, diagnostics:=diagnostics)
             End If
 

src/Compilers/VisualBasic/Portable/CodeGen/EmitOperators.vb

@@ -93,7 +93,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGen
 
         Private Sub EmitBinaryOperatorExpression(expression As BoundBinaryOperator, used As Boolean)
 
-            Dim operationKind = expression.OperatorKind
+            Dim operationKind = expression.OperatorKind And BinaryOperatorKind.OpMask
             Dim shortCircuit As Boolean = operationKind = BinaryOperatorKind.AndAlso OrElse operationKind = BinaryOperatorKind.OrElse
 
             If Not used AndAlso Not shortCircuit AndAlso Not OperatorHasSideEffects(expression) Then

src/Compilers/VisualBasic/Portable/CodeGen/OperatorKind.vb

@@ -27,6 +27,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
         [Error] = &H80
     End Enum
 
+    <Flags()>
     Friend Enum BinaryOperatorKind
 
         Add = 1
@@ -60,6 +61,26 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
         CompareText = &H40
         UserDefined = &H80
         [Error] = &H100
+
+        ''' <summary>
+        ''' Built-in binary operators bound in "OperandOfConditionalBranch" mode are marked with
+        ''' this flag by the Binder, which makes them eligible for some optimizations in
+        ''' <see cref="LocalRewriter.VisitNullableIsTrueOperator(BoundNullableIsTrueOperator)"/> 
+        ''' </summary>
+        IsOperandOfConditionalBranch = &H200
+
+        ''' <summary>
+        ''' <see cref="LocalRewriter.AdjustIfOptimizableForConditionalBranch"/> marks built-in binary operators
+        ''' with this flag in order to inform <see cref="LocalRewriter.VisitBinaryOperator"/> that the operator
+        ''' should be a subject for optimization around use of three-valued Boolean logic.
+        ''' The optimization should be applied only when we are absolutely sure that we will "snap" Null to false.
+        ''' That is when we actually have the <see cref="BoundNullableIsTrueOperator"/> as the ancestor and no user defined operators
+        ''' in between. We simply don't know that information during binding because we haven't bound binary operators
+        ''' up the hierarchy yet. So the optimization is triggered by the fact of snapping Null to false
+        ''' (getting to the <see cref="LocalRewriter.VisitNullableIsTrueOperator"/> method) and then we are making
+        ''' sure we don't have anything unexpected in between.
+        ''' </summary>
+        OptimizableForConditionalBranch = &H400
     End Enum
 
 End Namespace

src/Compilers/VisualBasic/Portable/Lowering/AsyncRewriter/AsyncRewriter.AsyncMethodToClassRewriter.Expressions.vb

@@ -164,7 +164,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                 End If
 
                 Dim builder As New SpillBuilder()
-                If rewritten.OperatorKind = BinaryOperatorKind.AndAlso OrElse rewritten.OperatorKind = BinaryOperatorKind.OrElse Then
+                Dim operatorKind As BinaryOperatorKind = rewritten.OperatorKind And BinaryOperatorKind.OpMask
+                Debug.Assert(operatorKind = (rewritten.OperatorKind And Not (BinaryOperatorKind.IsOperandOfConditionalBranch Or BinaryOperatorKind.OptimizableForConditionalBranch)))
+                If operatorKind = BinaryOperatorKind.AndAlso OrElse operatorKind = BinaryOperatorKind.OrElse Then
                     ' NOTE: Short circuit operators need to evaluate the right optionally
                     Dim spilledLeft = SpillValue(left, builder)
 
@@ -172,7 +174,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                     builder.AddLocal(tempLocal)
 
                     builder.AddStatement(
-                        If(rewritten.OperatorKind = BinaryOperatorKind.AndAlso,
+                        If(operatorKind = BinaryOperatorKind.AndAlso,
                            Me.F.If(condition:=spilledLeft,
                                    thenClause:=MakeAssignmentStatement(right, tempLocal, builder),
                                    elseClause:=MakeAssignmentStatement(Me.F.Literal(False), tempLocal)),

src/Compilers/VisualBasic/Portable/Lowering/LocalRewriter/LocalRewriter_BinaryOperators.vb

@@ -84,20 +84,33 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
         Public Overrides Function VisitBinaryOperator(node As BoundBinaryOperator) As BoundNode
             ' Do not blow the stack due to a deep recursion on the left. 
 
-            Dim child As BoundExpression = node.Left
+            Dim optimizeForConditionalBranch As Boolean = (node.OperatorKind And BinaryOperatorKind.OptimizableForConditionalBranch) <> 0
+            Dim optimizeChildForConditionalBranch As Boolean = optimizeForConditionalBranch
+
+            Dim child As BoundExpression = GetLeftOperand(node, optimizeChildForConditionalBranch)
 
             If child.Kind <> BoundKind.BinaryOperator Then
-                Return RewriteBinaryOperatorSimple(node)
+                Return RewriteBinaryOperatorSimple(node, optimizeForConditionalBranch)
             End If
 
-            Dim stack = ArrayBuilder(Of BoundBinaryOperator).GetInstance()
-            stack.Push(node)
+            Dim stack = ArrayBuilder(Of (Binary As BoundBinaryOperator, OptimizeForConditionalBranch As Boolean)).GetInstance()
+            stack.Push((node, optimizeForConditionalBranch))
 
             Dim binary As BoundBinaryOperator = DirectCast(child, BoundBinaryOperator)
 
             Do
-                stack.Push(binary)
-                child = binary.Left
+                If optimizeChildForConditionalBranch Then
+                    Select Case (binary.OperatorKind And BinaryOperatorKind.OpMask)
+                        Case BinaryOperatorKind.AndAlso, BinaryOperatorKind.OrElse
+                            Exit Select
+                        Case Else
+                            optimizeChildForConditionalBranch = False
+                    End Select
+                End If
+
+                stack.Push((binary, optimizeChildForConditionalBranch))
+
+                child = GetLeftOperand(binary, optimizeChildForConditionalBranch)
 
                 If child.Kind <> BoundKind.BinaryOperator Then
                     Exit Do
@@ -109,12 +122,13 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
             Dim left = VisitExpressionNode(child)
 
             Do
-                binary = stack.Pop()
+                Dim tuple As (Binary As BoundBinaryOperator, OptimizeForConditionalBranch As Boolean) = stack.Pop()
+                binary = tuple.Binary
 
-                Dim right = VisitExpressionNode(binary.Right)
+                Dim right = VisitExpression(GetRightOperand(binary, tuple.OptimizeForConditionalBranch))
 
                 If (binary.OperatorKind And BinaryOperatorKind.Lifted) <> 0 Then
-                    left = FinishRewriteOfLiftedIntrinsicBinaryOperator(binary, left, right)
+                    left = FinishRewriteOfLiftedIntrinsicBinaryOperator(binary, left, right, tuple.OptimizeForConditionalBranch)
                 Else
                     left = TransformRewrittenBinaryOperator(binary.Update(binary.OperatorKind, left, right, binary.Checked, binary.ConstantValueOpt, Me.VisitType(binary.Type)))
                 End If
@@ -126,9 +140,28 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
             Return left
         End Function
 
-        Private Function RewriteBinaryOperatorSimple(node As BoundBinaryOperator) As BoundNode
+        Private Shared Function GetLeftOperand(binary As BoundBinaryOperator, ByRef optimizeForConditionalBranch As Boolean) As BoundExpression
+            If optimizeForConditionalBranch AndAlso (binary.OperatorKind And BinaryOperatorKind.OpMask) <> BinaryOperatorKind.OrElse Then
+                Debug.Assert((binary.OperatorKind And BinaryOperatorKind.OpMask) = BinaryOperatorKind.AndAlso)
+                ' If left operand is evaluated to Null, three-valued Boolean logic dictates that the right operand of AndAlso
+                ' should still be evaluated. So, we cannot simply snap the left operand to Boolean.
+                optimizeForConditionalBranch = False
+            End If
+
+            Return binary.Left.GetMostEnclosedParenthesizedExpression()
+        End Function
+
+        Private Shared Function GetRightOperand(binary As BoundBinaryOperator, adjustIfOptimizableForConditionalBranch As Boolean) As BoundExpression
+            If adjustIfOptimizableForConditionalBranch Then
+                Return LocalRewriter.AdjustIfOptimizableForConditionalBranch(binary.Right, Nothing)
+            Else
+                Return binary.Right
+            End If
+        End Function
+
+        Private Function RewriteBinaryOperatorSimple(node As BoundBinaryOperator, optimizeForConditionalBranch As Boolean) As BoundNode
             If (node.OperatorKind And BinaryOperatorKind.Lifted) <> 0 Then
-                Return RewriteLiftedIntrinsicBinaryOperatorSimple(node)
+                Return RewriteLiftedIntrinsicBinaryOperatorSimple(node, optimizeForConditionalBranch)
             End If
 
             Return TransformRewrittenBinaryOperator(DirectCast(MyBase.VisitBinaryOperator(node), BoundBinaryOperator))
@@ -761,15 +794,34 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
             Return result
         End Function
 
-        Private Function RewriteLiftedIntrinsicBinaryOperatorSimple(node As BoundBinaryOperator) As BoundNode
+        Private Function RewriteLiftedIntrinsicBinaryOperatorSimple(node As BoundBinaryOperator, optimizeForConditionalBranch As Boolean) As BoundNode
             Dim left As BoundExpression = VisitExpressionNode(node.Left)
-            Dim right As BoundExpression = VisitExpressionNode(node.Right)
+            Dim right As BoundExpression = VisitExpressionNode(GetRightOperand(node, optimizeForConditionalBranch))
 
-            Return FinishRewriteOfLiftedIntrinsicBinaryOperator(node, left, right)
+            Return FinishRewriteOfLiftedIntrinsicBinaryOperator(node, left, right, optimizeForConditionalBranch)
         End Function
 
-        Private Function FinishRewriteOfLiftedIntrinsicBinaryOperator(node As BoundBinaryOperator, left As BoundExpression, right As BoundExpression) As BoundExpression
+        Private Function FinishRewriteOfLiftedIntrinsicBinaryOperator(node As BoundBinaryOperator, left As BoundExpression, right As BoundExpression, optimizeForConditionalBranch As Boolean) As BoundExpression
             Debug.Assert((node.OperatorKind And BinaryOperatorKind.Lifted) <> 0)
+            Debug.Assert(Not optimizeForConditionalBranch OrElse
+                         (node.OperatorKind And BinaryOperatorKind.OpMask) = BinaryOperatorKind.OrElse OrElse
+                         (node.OperatorKind And BinaryOperatorKind.OpMask) = BinaryOperatorKind.AndAlso)
+
+            Dim leftHasValue = HasValue(left)
+            Dim rightHasValue = HasValue(right)
+
+            Dim leftHasNoValue = HasNoValue(left)
+            Dim rightHasNoValue = HasNoValue(right)
+
+            ' The goal of optimization is to eliminate the need to deal with instances of Nullable(Of Boolean) type as early as possible,
+            ' and, as a result, simplify evaluation of built-in OrElse/AndAlso operators by eliminating the need to use three-valued Boolean logic.
+            ' The optimization is possible because when an entire Boolean Expression is evaluated to Null, that has the same effect as if result
+            ' of evaluation was False. However, we do want to preserve the original order of evaluation, according to language rules. 
+            If optimizeForConditionalBranch AndAlso node.Type.IsNullableOfBoolean() AndAlso left.Type.IsNullableOfBoolean() AndAlso right.Type.IsNullableOfBoolean() AndAlso
+               (leftHasValue OrElse Not Me._inExpressionLambda OrElse (node.OperatorKind And BinaryOperatorKind.OpMask) = BinaryOperatorKind.OrElse) Then
+
+                Return RewriteAndOptimizeLiftedIntrinsicLogicalShortCircuitingOperator(node, left, right, leftHasNoValue, leftHasValue, rightHasNoValue, rightHasValue)
+            End If
 
             If Me._inExpressionLambda Then
                 Return node.Update(node.OperatorKind, left, right, node.Checked, node.ConstantValueOpt, node.Type)
@@ -777,18 +829,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
 
             ' Check for trivial (no nulls, two nulls) Cases
 
-            Dim leftHasNoValue = HasNoValue(left)
-            Dim rightHasNoValue = HasNoValue(right)
-
             '== TWO NULLS
             If leftHasNoValue And rightHasNoValue Then
                 ' return new R?()
                 Return NullableNull(left, node.Type)
             End If
 
-            Dim leftHasValue = HasValue(left)
-            Dim rightHasValue = HasValue(right)
-
             '== NO NULLS
             If leftHasValue And rightHasValue Then
                 ' return new R?(UnliftedOp(left, right))
@@ -855,7 +901,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                                                        leftHasValueExpr,
                                                        temps,
                                                        inits,
-                                                       RightCanChangeLeftLocal(left, right),
+                                                       RightCantChangeLeftLocal(left, right),
                                                        leftHasValue)
 
             ' left is done when right is running, so right cannot change if it is a local
@@ -894,6 +940,121 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
             Return value
         End Function
 
+        ''' <summary>
+        ''' The goal of optimization is to eliminate the need to deal with instances of Nullable(Of Boolean) type as early as possible,
+        ''' and, as a result, simplify evaluation of built-in OrElse/AndAlso operators by eliminating the need to use three-valued Boolean logic.
+        ''' The optimization is possible because when an entire Boolean Expression is evaluated to Null, that has the same effect as if result
+        ''' of evaluation was False. However, we do want to preserve the original order of evaluation, according to language rules. 
+        ''' This method returns an expression that still has Nullable(Of Boolean) type, but that expression is much simpler and can be 
+        ''' further simplified by the consumer.
+        ''' </summary>
+        Private Function RewriteAndOptimizeLiftedIntrinsicLogicalShortCircuitingOperator(node As BoundBinaryOperator,
+                                                                                         left As BoundExpression, right As BoundExpression,
+                                                                                         leftHasNoValue As Boolean, leftHasValue As Boolean,
+                                                                                         rightHasNoValue As Boolean, rightHasValue As Boolean) As BoundExpression
+
+            Debug.Assert(leftHasValue OrElse Not Me._inExpressionLambda OrElse (node.OperatorKind And BinaryOperatorKind.OpMask) = BinaryOperatorKind.OrElse)
+            Dim booleanResult As BoundExpression = Nothing
+
+            If Not Me._inExpressionLambda Then
+                If leftHasNoValue And rightHasNoValue Then
+                    ' return new R?(), the consumer will take care of optimizing it out if possible. 
+                    Return NullableNull(left, node.Type)
+                End If
+
+                If (node.OperatorKind And BinaryOperatorKind.OpMask) = BinaryOperatorKind.OrElse Then
+                    If leftHasNoValue Then
+                        ' There is nothing to evaluate on the left, the result is True only if Right is True
+                        Return right
+                    ElseIf rightHasNoValue Then
+                        ' There is nothing to evaluate on the right, the result is True only if Left is True
+                        Return left
+                    End If
+                Else
+                    Debug.Assert((node.OperatorKind And BinaryOperatorKind.OpMask) = BinaryOperatorKind.AndAlso)
+
+                    If leftHasNoValue Then
+                        ' We can return False in this case. There is nothing to evaluate on the left, but we still need to evaluate the right
+                        booleanResult = EvaluateOperandAndReturnFalse(node, right, rightHasValue)
+                    ElseIf rightHasNoValue Then
+                        ' We can return False in this case. There is nothing to evaluate on the right, but we still need to evaluate the left
+                        booleanResult = EvaluateOperandAndReturnFalse(node, left, leftHasValue)
+                    ElseIf Not leftHasValue Then
+                        ' We cannot tell whether Left is Null or not.
+                        ' For [x AndAlso y] we can produce Boolean result as follows:
+                        '
+                        ' tempX = x
+                        ' (Not tempX.HasValue OrElse tempX.GetValueOrDefault()) AndAlso
+                        ' (y.GetValueOrDefault() AndAlso tempX.HasValue)
+                        '
+                        Dim leftTemp As SynthesizedLocal = Nothing
+                        Dim leftInit As BoundExpression = Nothing
+
+                        ' Right may be a method that takes Left byref - " local AndAlso TakesArgByref(local) "
+                        ' So in general we must capture Left even if it is a local.
+                        Dim capturedLeft = CaptureNullableIfNeeded(left, leftTemp, leftInit, RightCantChangeLeftLocal(left, right))
+
+                        booleanResult = MakeBooleanBinaryExpression(node.Syntax,
+                                            BinaryOperatorKind.AndAlso,
+                                            MakeBooleanBinaryExpression(node.Syntax,
+                                                BinaryOperatorKind.OrElse,
+                                                New BoundUnaryOperator(node.Syntax,
+                                                                       UnaryOperatorKind.Not,
+                                                                       NullableHasValue(capturedLeft),
+                                                                       False,
+                                                                       node.Type.GetNullableUnderlyingType()),
+                                                NullableValueOrDefault(capturedLeft)),
+                                            MakeBooleanBinaryExpression(node.Syntax,
+                                                BinaryOperatorKind.AndAlso,
+                                                NullableValueOrDefault(right),
+                                                NullableHasValue(capturedLeft)))
+
+                        ' if we used temp, put it in a sequence
+                        Debug.Assert((leftTemp Is Nothing) = (leftInit Is Nothing))
+                        If leftTemp IsNot Nothing Then
+                            booleanResult = New BoundSequence(node.Syntax,
+                                                              ImmutableArray.Create(Of LocalSymbol)(leftTemp),
+                                                              ImmutableArray.Create(Of BoundExpression)(leftInit),
+                                                              booleanResult,
+                                                              booleanResult.Type)
+                        End If
+                    End If
+                End If
+            End If
+
+            If booleanResult Is Nothing Then
+                ' UnliftedOp(left.GetValueOrDefault(), right.GetValueOrDefault()))
+                ' For AndAlso, this optimization is valid only when we know that left has value
+                Debug.Assert(leftHasValue OrElse (node.OperatorKind And BinaryOperatorKind.OpMask) = BinaryOperatorKind.OrElse)
+
+                booleanResult = ApplyUnliftedBinaryOp(node, NullableValueOrDefault(left, leftHasValue), NullableValueOrDefault(right, rightHasValue))
+            End If
+
+            ' return new R?(booleanResult), the consumer will take care of optimizing out the creation of this Nullable(Of Boolean) instance, if possible.
+            Return WrapInNullable(booleanResult, node.Type)
+        End Function
+
+        Private Function EvaluateOperandAndReturnFalse(node As BoundBinaryOperator, operand As BoundExpression, operandHasValue As Boolean) As BoundExpression
+            Debug.Assert(node.Type.IsNullableOfBoolean())
+            Debug.Assert(operand.Type.IsNullableOfBoolean())
+
+            Dim result = New BoundLiteral(node.Syntax, ConstantValue.False, node.Type.GetNullableUnderlyingType())
+            Return New BoundSequence(node.Syntax, ImmutableArray(Of LocalSymbol).Empty,
+                                     ImmutableArray.Create(If(operandHasValue, NullableValueOrDefault(operand), operand)),
+                                     result, result.Type)
+        End Function
+
+        Private Function NullableValueOrDefault(operand As BoundExpression, operandHasValue As Boolean) As BoundExpression
+            Debug.Assert(operand.Type.IsNullableOfBoolean())
+
+            If Not Me._inExpressionLambda OrElse operandHasValue Then
+                Return NullableValueOrDefault(operand)
+            Else
+                ' In expression tree this will be shown as Coalesce, which is preferred over a GetValueOrDefault call  
+                Return New BoundNullableIsTrueOperator(operand.Syntax, operand, operand.Type.GetNullableUnderlyingType())
+            End If
+        End Function
+
         Private Function RewriteLiftedBooleanBinaryOperator(node As BoundBinaryOperator,
                                                             left As BoundExpression,
                                                             right As BoundExpression,
@@ -1017,7 +1178,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
             If Not leftHasValue Then
                 ' Right may be a method that takes Left byref - " local And TakesArgByref(local) "
                 ' So in general we must capture Left even if it is a local.
-                capturedLeft = CaptureNullableIfNeeded(left, leftTemp, leftInit, RightCanChangeLeftLocal(left, right))
+                capturedLeft = CaptureNullableIfNeeded(left, leftTemp, leftInit, RightCantChangeLeftLocal(left, right))
             End If
 
             Dim rightTemp As SynthesizedLocal = Nothing
@@ -1160,7 +1321,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                 Return node
             End If
 
-            Return RewriteNullableIsOrIsNotOperator(node.OperatorKind = BinaryOperatorKind.Is, If(left.IsNothingLiteral, right, left), node.Type)
+            Return RewriteNullableIsOrIsNotOperator((node.OperatorKind And BinaryOperatorKind.OpMask) = BinaryOperatorKind.Is, If(left.IsNothingLiteral, right, left), node.Type)
         End Function
 
         Private Function RewriteNullableIsOrIsNotOperator(isIs As Boolean, operand As BoundExpression, resultType As TypeSymbol) As BoundExpression

src/Compilers/VisualBasic/Portable/Lowering/LocalRewriter/LocalRewriter_NullableHelpers.vb

@@ -74,7 +74,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
         ' Right operand could be a method that takes Left operand byref. Ex: " local And TakesArgByref(local) "
         ' So in general we must capture Left even if it is a local.
         ' however in many case we do not need that.
-        Private Function RightCanChangeLeftLocal(left As BoundExpression, right As BoundExpression) As Boolean
+        Private Shared Function RightCantChangeLeftLocal(left As BoundExpression, right As BoundExpression) As Boolean
             ' TODO: in most cases right operand does not change value of the left one
             '       we could be smarter than this.
             Return right.Kind = BoundKind.Local OrElse
@@ -158,14 +158,20 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
 
             ' check if we are not getting value from freshly constructed nullable
             ' no need to wrap/unwrap it then.
-            If expr.Kind = BoundKind.ObjectCreationExpression Then
-                Dim objectCreation = DirectCast(expr, BoundObjectCreationExpression)
+            Select Case expr.Kind
+                Case BoundKind.ObjectCreationExpression
+                    Dim objectCreation = DirectCast(expr, BoundObjectCreationExpression)
 
-                ' passing one argument means we are calling New Nullable<T>(arg)
-                If objectCreation.Arguments.Length = 1 Then
-                    Return objectCreation.Arguments(0)
-                End If
-            End If
+                    ' passing one argument means we are calling New Nullable<T>(arg)
+                    If objectCreation.Arguments.Length = 1 Then
+                        Return objectCreation.Arguments(0)
+                    End If
+                Case BoundKind.Conversion
+                    Dim conversion = DirectCast(expr, BoundConversion)
+                    If IsConversionFromUnderlyingToNullable(conversion) Then
+                        Return conversion.Operand
+                    End If
+            End Select
 
             Dim getValueOrDefaultMethod = GetNullableMethod(expr.Syntax, expr.Type, SpecialMember.System_Nullable_T_GetValueOrDefault)
 
@@ -184,6 +190,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
             Return New BoundBadExpression(expr.Syntax, LookupResultKind.NotReferencable, ImmutableArray(Of Symbol).Empty, ImmutableArray.Create(expr), expr.Type.GetNullableUnderlyingType(), hasErrors:=True)
         End Function
 
+        Private Shared Function IsConversionFromUnderlyingToNullable(conversion As BoundConversion) As Boolean
+            Return (conversion.ConversionKind And (ConversionKind.Widening Or ConversionKind.Nullable Or ConversionKind.UserDefined)) = (ConversionKind.Widening Or ConversionKind.Nullable) AndAlso
+                   conversion.Type.GetNullableUnderlyingType().Equals(conversion.Operand.Type, TypeCompareKind.AllIgnoreOptionsForVB)
+        End Function
+
         Private Function NullableValue(expr As BoundExpression) As BoundExpression
             Debug.Assert(expr.Type.IsNullableType)
 
@@ -322,11 +333,16 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
         Private Shared Function HasValue(expr As BoundExpression) As Boolean
             Debug.Assert(expr.Type.IsNullableType)
 
-            If expr.Kind = BoundKind.ObjectCreationExpression Then
-                Dim objCreation = DirectCast(expr, BoundObjectCreationExpression)
-                ' Nullable<T> has only one ctor with parameters and only that one sets hasValue = true
-                Return objCreation.Arguments.Length <> 0
-            End If
+            Select Case expr.Kind
+                Case BoundKind.ObjectCreationExpression
+                    Dim objCreation = DirectCast(expr, BoundObjectCreationExpression)
+                    ' Nullable<T> has only one ctor with parameters and only that one sets hasValue = true
+                    Return objCreation.Arguments.Length <> 0
+                Case BoundKind.Conversion
+                    If IsConversionFromUnderlyingToNullable(DirectCast(expr, BoundConversion)) Then
+                        Return True
+                    End If
+            End Select
 
             ' by default we do not know
             Return False

src/Compilers/VisualBasic/Portable/Lowering/LocalRewriter/LocalRewriter_UnaryOperators.vb

@@ -15,11 +15,16 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
         Public Overrides Function VisitNullableIsTrueOperator(node As BoundNullableIsTrueOperator) As BoundNode
             Debug.Assert(node.Operand.Type.IsNullableOfBoolean())
 
-            If _inExpressionLambda Then
-                Return node.Update(VisitExpression(node.Operand), node.Type)
+            Dim optimizableForConditionalBranch As Boolean = False
+            Dim operand = VisitExpression(AdjustIfOptimizableForConditionalBranch(node.Operand, optimizableForConditionalBranch))
+
+            If optimizableForConditionalBranch AndAlso HasValue(operand) Then
+                Return NullableValueOrDefault(operand)
             End If
 
-            Dim operand = VisitExpressionNode(node.Operand)
+            If _inExpressionLambda Then
+                Return node.Update(operand, node.Type)
+            End If
 
             If HasNoValue(operand) Then
                 Return New BoundLiteral(node.Syntax, ConstantValue.False, node.Type)
@@ -39,6 +44,35 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
             Return NullableValueOrDefault(operand)
         End Function
 
+        Private Shared Function AdjustIfOptimizableForConditionalBranch(operand As BoundExpression, <Out> ByRef optimizableForConditionalBranch As Boolean) As BoundExpression
+            optimizableForConditionalBranch = False
+
+            Dim current As BoundExpression = operand
+            Do
+                Select Case current.Kind
+                    Case BoundKind.BinaryOperator
+                        Dim binary = DirectCast(current, BoundBinaryOperator)
+                        If (binary.OperatorKind And BinaryOperatorKind.IsOperandOfConditionalBranch) <> 0 Then
+                            Select Case (binary.OperatorKind And BinaryOperatorKind.OpMask)
+                                Case BinaryOperatorKind.AndAlso, BinaryOperatorKind.OrElse
+                                    Debug.Assert((binary.OperatorKind And BinaryOperatorKind.Lifted) <> 0)
+                                    optimizableForConditionalBranch = True
+                                    Return binary.Update(binary.OperatorKind Or BinaryOperatorKind.OptimizableForConditionalBranch,
+                                                         binary.Left, binary.Right, binary.Checked, binary.ConstantValueOpt, binary.Type)
+                            End Select
+                        End If
+
+                        Return operand
+
+                    Case BoundKind.Parenthesized
+                        current = DirectCast(current, BoundParenthesized).Expression
+                    Case Else
+                        Return operand
+                End Select
+            Loop
+
+        End Function
+
         Public Overrides Function VisitUserDefinedUnaryOperator(node As BoundUserDefinedUnaryOperator) As BoundNode
             If _inExpressionLambda Then
                 Return node.Update(node.OperatorKind, VisitExpression(node.UnderlyingExpression), node.Type)

src/Compilers/VisualBasic/Portable/Operations/VisualBasicOperationFactory.vb

@@ -1225,7 +1225,7 @@ Namespace Microsoft.CodeAnalysis.Operations
 
         Friend Function CreateBoundCatchBlockExceptionDeclarationOrExpression(boundCatchBlock As BoundCatchBlock) As IOperation
             If boundCatchBlock.LocalOpt IsNot Nothing AndAlso
-                        boundCatchBlock.ExceptionSourceOpt?.Kind = BoundKind.Local AndAlso
+                        (boundCatchBlock.ExceptionSourceOpt?.Kind = BoundKind.Local).GetValueOrDefault() AndAlso
                         boundCatchBlock.LocalOpt Is DirectCast(boundCatchBlock.ExceptionSourceOpt, BoundLocal).LocalSymbol Then
                 Return New VariableDeclaratorOperation(boundCatchBlock.LocalOpt, initializer:=Nothing, ignoredArguments:=ImmutableArray(Of IOperation).Empty, semanticModel:=_semanticModel, syntax:=boundCatchBlock.ExceptionSourceOpt.Syntax, type:=Nothing, constantValue:=Nothing, isImplicit:=False)
             Else

src/Compilers/VisualBasic/Portable/Operations/VisualBasicOperationFactory_QueryLambdaRewriter.vb

@@ -84,7 +84,7 @@ Namespace Microsoft.CodeAnalysis.Operations
             End Function
 
             Public Overrides Function VisitParameter(node As BoundParameter) As BoundNode
-                If node.ParameterSymbol?.ContainingSymbol.IsQueryLambdaMethod AndAlso Not _uniqueNodes.Add(node) Then
+                If (node.ParameterSymbol?.ContainingSymbol.IsQueryLambdaMethod).GetValueOrDefault() AndAlso Not _uniqueNodes.Add(node) Then
                     Dim wasCompilerGenerated As Boolean = node.WasCompilerGenerated
                     node = New BoundParameter(node.Syntax, node.ParameterSymbol, node.IsLValue, node.SuppressVirtualCalls, node.Type, node.HasErrors)
                     If wasCompilerGenerated Then

src/Compilers/VisualBasic/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.vb

@@ -1280,7 +1280,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols.Metadata.PE
                    specialtype <> SpecialType.System_Enum AndAlso specialtype <> SpecialType.System_MulticastDelegate Then
                     Dim base As TypeSymbol = GetDeclaredBase(Nothing)
 
-                    If base?.SpecialType = SpecialType.None AndAlso base.ContainingAssembly?.IsMissing Then
+                    If base IsNot Nothing AndAlso base.SpecialType = SpecialType.None AndAlso base.ContainingAssembly?.IsMissing Then
                         Dim missingType = TryCast(base, MissingMetadataTypeSymbol.TopLevel)
 
                         If missingType IsNot Nothing AndAlso missingType.Arity = 0 Then

src/Compilers/VisualBasic/Portable/Symbols/NamedTypeSymbol.vb

@@ -1272,8 +1272,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
 
             ' Should this be optimized for perf (caching for VT<0> to VT<7>, etc.)?
             If Not IsUnboundGenericType AndAlso
-                ContainingSymbol?.Kind = SymbolKind.Namespace AndAlso
-                ContainingNamespace?.ContainingNamespace?.IsGlobalNamespace = True AndAlso
+                (ContainingSymbol?.Kind = SymbolKind.Namespace).GetValueOrDefault() AndAlso
+                (ContainingNamespace.ContainingNamespace?.IsGlobalNamespace).GetValueOrDefault() AndAlso
                 Name = TupleTypeSymbol.TupleTypeName AndAlso
                 ContainingNamespace.Name = MetadataHelpers.SystemString Then
 

src/Compilers/VisualBasic/Portable/Symbols/Source/SourceParameterSymbol.vb

@@ -105,7 +105,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
             Get
                 If Me.ContainingSymbol.IsImplicitlyDeclared Then
 
-                    If TryCast(Me.ContainingSymbol, MethodSymbol)?.MethodKind = MethodKind.DelegateInvoke AndAlso
+                    If (TryCast(Me.ContainingSymbol, MethodSymbol)?.MethodKind = MethodKind.DelegateInvoke).GetValueOrDefault() AndAlso
                        Not Me.ContainingType.AssociatedSymbol?.IsImplicitlyDeclared Then
                         Return False
                     End If

src/Compilers/VisualBasic/Portable/Symbols/SymbolExtensions.vb

@@ -436,7 +436,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
 
         <Extension>
         Friend Function ContainingNonLambdaMember(member As Symbol) As Symbol
-            While member?.Kind = SymbolKind.Method AndAlso DirectCast(member, MethodSymbol).MethodKind = MethodKind.AnonymousFunction
+            While (member?.Kind = SymbolKind.Method).GetValueOrDefault() AndAlso DirectCast(member, MethodSymbol).MethodKind = MethodKind.AnonymousFunction
                 member = member.ContainingSymbol
             End While
 

src/Compilers/VisualBasic/Test/CommandLine/CommandLineBreakingChanges.vb

@@ -42,7 +42,7 @@ SRC.VB(7) : warning BC42104: Variable 'x' is used before it has been assigned a
         End Sub
 
         <Fact, WorkItem(530668, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/530668")>
-        Public Sub WarnAsErrorPrecedence2()
+        Public Sub WarnAsErrorPrecedence2_1()
             Dim src As String = Temp.CreateFile().WriteAllText(<text>
 Module M1
     Sub Main
@@ -70,15 +70,43 @@ SRC.VB(6) : error BC42032: Operands of type Object used for operator '&lt;&gt;';
 
         if (a.Something &lt;&gt; 2)
             ~~~~~~~~~~~      
-SRC.VB(6) : error BC42016: Implicit conversion from 'Object' to 'Boolean'.
+</text>.Value.Trim().Replace(vbLf, vbCrLf), tempOut.ReadAllText().Trim().Replace(src, "SRC.VB"))
+        End Sub
 
-        if (a.Something &lt;&gt; 2)
-           ~~~~~~~~~~~~~~~~~~
+        <Fact, WorkItem(530668, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/530668")>
+        Public Sub WarnAsErrorPrecedence2_2()
+            Dim src As String = Temp.CreateFile().WriteAllText(<text>
+Module M1
+    Sub Main
+    End Sub
+    Sub M(a as Object)
+        if a.Something &lt;&gt; 2
+        end if
+    End Sub
+End Module
+</text>.Value).Path
+            Dim tempOut = Temp.CreateFile()
+            Dim output = ProcessUtilities.RunAndGetOutput("cmd", "/C """ & s_basicCompilerExecutable & """ /nologo /preferreduilang:en /optionstrict:custom /nowarn:41008 /warnaserror+ " & src & " > " & tempOut.Path, expectedRetCode:=1)
+            Assert.Equal("", output.Trim())
+
+            'See bug 16673.
+            'In Dev11, /warnaserror+ does not come into effect strangely and the code only reports warnings.
+            'In Roslyn, /warnaserror+ does come into effect and the code reports the warnings as errors.
+
+            Assert.Equal(<text>
+SRC.VB(6) : error BC42017: Late bound resolution; runtime errors could occur.
+
+        if a.Something &lt;&gt; 2
+           ~~~~~~~~~~~     
+SRC.VB(6) : error BC42032: Operands of type Object used for operator '&lt;&gt;'; use the 'IsNot' operator to test object identity.
+
+        if a.Something &lt;&gt; 2
+           ~~~~~~~~~~~      
 </text>.Value.Trim().Replace(vbLf, vbCrLf), tempOut.ReadAllText().Trim().Replace(src, "SRC.VB"))
         End Sub
 
         <Fact, WorkItem(530668, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/530668")>
-        Public Sub WarnAsErrorPrecedence3()
+        Public Sub WarnAsErrorPrecedence3_1()
             Dim src As String = Temp.CreateFile().WriteAllText(<text>
 Module M1
     Sub Main
@@ -106,10 +134,41 @@ SRC.VB(6) : error BC42032: Operands of type Object used for operator '&lt;&gt;';
 
         if (a.Something &lt;&gt; 2)
             ~~~~~~~~~~~      
-SRC.VB(6) : error BC42016: Implicit conversion from 'Object' to 'Boolean'.
+</text>.Value.Trim().Replace(vbLf, vbCrLf), tempOut.ReadAllText().Trim().Replace(src, "SRC.VB"))
 
-        if (a.Something &lt;&gt; 2)
-           ~~~~~~~~~~~~~~~~~~
+            CleanupAllGeneratedFiles(src)
+            CleanupAllGeneratedFiles(tempOut.Path)
+        End Sub
+
+        <Fact, WorkItem(530668, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/530668")>
+        Public Sub WarnAsErrorPrecedence3_2()
+            Dim src As String = Temp.CreateFile().WriteAllText(<text>
+Module M1
+    Sub Main
+    End Sub
+    Sub M(a as Object)
+        if a.Something &lt;&gt; 2
+        end if
+    End Sub
+End Module
+</text>.Value).Path
+            Dim tempOut = Temp.CreateFile()
+            Dim output = ProcessUtilities.RunAndGetOutput("cmd", "/C """ & s_basicCompilerExecutable & """ /nologo /preferreduilang:en /optionstrict:custom /warnaserror-:42025 /warnaserror+ " & src & " > " & tempOut.Path, expectedRetCode:=1)
+            Assert.Equal("", output.Trim())
+
+            'See bug 16673.
+            'In Dev11, /warnaserror+ does not come into effect strangely and the code only reports warnings.
+            'In Roslyn, /warnaserror+ does come into effect and the code reports the warnings as errors.
+
+            Assert.Equal(<text>
+SRC.VB(6) : error BC42017: Late bound resolution; runtime errors could occur.
+
+        if a.Something &lt;&gt; 2
+           ~~~~~~~~~~~     
+SRC.VB(6) : error BC42032: Operands of type Object used for operator '&lt;&gt;'; use the 'IsNot' operator to test object identity.
+
+        if a.Something &lt;&gt; 2
+           ~~~~~~~~~~~      
 </text>.Value.Trim().Replace(vbLf, vbCrLf), tempOut.ReadAllText().Trim().Replace(src, "SRC.VB"))
 
             CleanupAllGeneratedFiles(src)

src/Compilers/VisualBasic/Test/Emit/ExpressionTrees/Results/CheckedCoalesceWithNullableBoolean.txt

@@ -239,27 +239,20 @@ Lambda(
   body {
     Convert(
       Conditional(
-        Coalesce(
-          AndAlso(
-            Convert(
-              Parameter(
-                x
-                type: System.Boolean
-              )
-              Lifted
-              LiftedToNull
-              type: System.Nullable`1[System.Boolean]
-            )
+        AndAlso(
+          Parameter(
+            x
+            type: System.Boolean
+          )
+          Coalesce(
             Parameter(
               y
               type: System.Nullable`1[System.Boolean]
             )
-            Lifted
-            LiftedToNull
-            type: System.Nullable`1[System.Boolean]
-          )
-          Constant(
-            False
+            Constant(
+              False
+              type: System.Boolean
+            )
             type: System.Boolean
           )
           type: System.Boolean

src/Compilers/VisualBasic/Test/Semantic/Diagnostics/DiagnosticAnalyzerTests.vb

@@ -1242,8 +1242,8 @@ End Class
 
             ' Always report diagnostics in generated code, unless explicitly suppressed or we are not even analyzing generated code.
             Dim reportInGeneratedCode = generatedCodeAnalysisFlagsOpt Is Nothing OrElse
-                ((generatedCodeAnalysisFlagsOpt And GeneratedCodeAnalysisFlags.ReportDiagnostics) <> 0 AndAlso
-                 (generatedCodeAnalysisFlagsOpt And GeneratedCodeAnalysisFlags.Analyze) <> 0)
+                ((generatedCodeAnalysisFlagsOpt.GetValueOrDefault() And GeneratedCodeAnalysisFlags.ReportDiagnostics) <> 0 AndAlso
+                 (generatedCodeAnalysisFlagsOpt.GetValueOrDefault() And GeneratedCodeAnalysisFlags.Analyze) <> 0)
 
             If Not isGeneratedCode OrElse reportInGeneratedCode Then
                 Dim diag = Diagnostic(GeneratedCodeAnalyzer.Warning.Id, squiggledText).WithArguments(arguments).WithLocation(line, column)

src/Features/VisualBasic/Portable/ExtractMethod/VisualBasicSelectionResult.vb

@@ -188,7 +188,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.ExtractMethod
             End If
 
             ' use FormattableString if conversion between String And FormattableString
-            If info.Type?.SpecialType = SpecialType.System_String AndAlso
+            If (info.Type?.SpecialType = SpecialType.System_String).GetValueOrDefault() AndAlso
                info.ConvertedType?.IsFormattableString() Then
 
                 Return info.ConvertedType

src/Workspaces/VisualBasic/Portable/Extensions/CastAnalyzer.vb

@@ -217,7 +217,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Extensions
 
             ' A casts to object can always be removed from an expression inside of an interpolation, since it'll be converted to object
             ' in order to call string.Format(...) anyway.
-            If castType?.SpecialType = SpecialType.System_Object AndAlso
+            If (castType?.SpecialType = SpecialType.System_Object).GetValueOrDefault() AndAlso
                 _castNode.WalkUpParentheses().IsParentKind(SyntaxKind.Interpolation) Then
                 Return True
             End If

src/Workspaces/VisualBasic/Portable/Extensions/ExpressionSyntaxExtensions.vb

@@ -1476,7 +1476,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Extensions
             If name.IsKind(SyntaxKind.GenericName) Then
                 If (name.IsParentKind(SyntaxKind.CrefReference)) OrElse ' cref="Nullable(Of T)"
                    (name.IsParentKind(SyntaxKind.QualifiedName) AndAlso name.Parent?.IsParentKind(SyntaxKind.CrefReference)) OrElse ' cref="System.Nullable(Of T)"
-                   (name.IsParentKind(SyntaxKind.QualifiedName) AndAlso name.Parent?.IsParentKind(SyntaxKind.QualifiedName) AndAlso name.Parent?.Parent?.IsParentKind(SyntaxKind.CrefReference)) Then  ' cref="System.Nullable(Of T).Value"
+                   (name.IsParentKind(SyntaxKind.QualifiedName) AndAlso (name.Parent?.IsParentKind(SyntaxKind.QualifiedName)).GetValueOrDefault() AndAlso name.Parent.Parent?.IsParentKind(SyntaxKind.CrefReference)) Then  ' cref="System.Nullable(Of T).Value"
                     ' Unfortunately, unlike in corresponding C# case, we need syntax based checking to detect these cases because of bugs in the VB SemanticModel.
                     ' See https://github.com/dotnet/roslyn/issues/2196, https://github.com/dotnet/roslyn/issues/2197
                     Return False