Merge pull request #8944 from genlu/FixCrash

[Ask Mode] Fix IOperation crash caused by passing null for ConstantValue when creating Lite…

src/Compilers/Core/CodeAnalysisTest/Diagnostics/OperationTestAnalyzer.cs

@@ -1709,4 +1709,44 @@ public sealed override void Initialize(AnalysisContext context)
                 OperationKind.PlaceholderExpression);
         }
     }
+    
+
+    public class ForLoopConditionCrashVBTestAnalyzer : DiagnosticAnalyzer
+    {
+        private const string ReliabilityCategory = "Reliability";
+        
+        public static readonly DiagnosticDescriptor ForLoopConditionCrashDescriptor = new DiagnosticDescriptor(
+            "ForLoopConditionCrash",
+            "Ensure ForLoopCondition property doesn't crash",
+            "Ensure ForLoopCondition property doesn't crash",
+            ReliabilityCategory,
+            DiagnosticSeverity.Warning,
+            isEnabledByDefault: true);
+
+        public sealed override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
+        {
+            get { return ImmutableArray.Create(ForLoopConditionCrashDescriptor); }
+        }
+
+        public sealed override void Initialize(AnalysisContext context)
+        {
+            context.RegisterOperationAction(
+                 (operationContext) =>
+                 {
+                     ILoopStatement loop = (ILoopStatement)operationContext.Operation;
+                     if (loop.LoopKind == LoopKind.For)
+                     {
+                         IForLoopStatement forLoop = (IForLoopStatement)loop;
+                         var forCondition = forLoop.Condition;
+
+                         if (forCondition.IsInvalid)
+                         {
+                             // Generate a warning to prove we didn't crash
+                             operationContext.ReportDiagnostic(Diagnostic.Create(ForLoopConditionCrashDescriptor, forLoop.Condition.Syntax.GetLocation()));
+                         }
+                     }
+                 },
+                 OperationKind.LoopStatement);
+        }
+    }
 }
\ No newline at end of file

src/Compilers/Core/Portable/Compilation/Expression.cs

@@ -3,6 +3,7 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Diagnostics;
 using System.Linq;
 
 namespace Microsoft.CodeAnalysis.Semantics
@@ -46,7 +47,7 @@ public static ConstantValue SynthesizeNumeric(ITypeSymbol type, int value)
                 return SynthesizeNumeric(((INamedTypeSymbol)type).EnumUnderlyingType, value);
             }
 
-            return null;
+            return ConstantValue.Bad;
         }
 
         public static BinaryOperationKind DeriveAdditionKind(ITypeSymbol type)
@@ -104,7 +105,7 @@ public ConditionalChoice(IOperation condition, IOperation ifTrue, IOperation ifF
 
         public OperationKind Kind => OperationKind.ConditionalChoiceExpression;
 
-        public bool IsInvalid => Condition == null || Condition.IsInvalid || IfTrueValue == null || IfTrueValue.IsInvalid || IfFalseValue == null || IfFalseValue.IsInvalid;
+        public bool IsInvalid => Condition == null || Condition.IsInvalid || IfTrueValue == null || IfTrueValue.IsInvalid || IfFalseValue == null || IfFalseValue.IsInvalid || Type == null;
 
         public Optional<object> ConstantValue => default(Optional<object>);
 
@@ -301,6 +302,7 @@ internal class Literal : ILiteralExpression
 
         public Literal(ConstantValue value, ITypeSymbol resultType, SyntaxNode syntax)
         {
+            Debug.Assert(value != null, "value can't be null");
             _value = value;
             this.Type = resultType;
             this.Syntax = syntax;
@@ -312,7 +314,7 @@ public Literal(ConstantValue value, ITypeSymbol resultType, SyntaxNode syntax)
 
         public OperationKind Kind => OperationKind.LiteralExpression;
 
-        public bool IsInvalid => false;
+        public bool IsInvalid => _value.IsBad;
 
         public Optional<object> ConstantValue => new Optional<object>(_value.Value);
 
@@ -354,7 +356,12 @@ public Binary(BinaryOperationKind binaryOperationKind, IOperation left, IOperati
 
         public OperationKind Kind => OperationKind.BinaryOperatorExpression;
 
-        public bool IsInvalid => LeftOperand == null || LeftOperand.IsInvalid || RightOperand == null || RightOperand.IsInvalid;
+        public bool IsInvalid => LeftOperand == null 
+                                || LeftOperand.IsInvalid 
+                                || RightOperand == null
+                                || RightOperand.IsInvalid 
+                                || BinaryOperationKind == BinaryOperationKind.Invalid 
+                                || Type == null;
 
         public Optional<object> ConstantValue => default(Optional<object>);
 

src/Compilers/VisualBasic/Portable/BoundTree/Statement.vb

@@ -628,7 +628,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
                         Dim limitValue As IOperation = If(BoundFor.LimitValue.IsConstant, DirectCast(BoundFor.LimitValue, IOperation), New Temporary(SyntheticLocalKind.ForLoopLimitValue, BoundFor, BoundFor.LimitValue))
                         Dim controlVariable As BoundExpression = BoundFor.ControlVariable
 
-                        Dim booleanType As ITypeSymbol = controlVariable.ExpressionSymbol.DeclaringCompilation.GetSpecialType(SpecialType.System_Boolean)
+                        ' controlVariable can be a BoundBadExpression in case of error
+                        Dim booleanType As ITypeSymbol = controlVariable.ExpressionSymbol?.DeclaringCompilation.GetSpecialType(SpecialType.System_Boolean)
 
                         Dim operators As BoundForToUserDefinedOperators = Me.OperatorsOpt
                         If operators IsNot Nothing Then
@@ -645,7 +646,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
 
                                 Dim stepValue As IOperation = New Temporary(SyntheticLocalKind.ForLoopStepValue, BoundFor, BoundFor.StepValue)
                                 Dim stepRelationalCode As BinaryOperationKind = DeriveBinaryOperationKind(BinaryOperatorKind.GreaterThanOrEqual, BoundFor.StepValue)
-                                Dim stepCondition As IOperation = New Binary(stepRelationalCode, stepValue, New BoundLiteral(Nothing, Semantics.Expression.SynthesizeNumeric(stepValue.Type, 0), BoundFor.StepValue.Type), booleanType, BoundFor.StepValue.Syntax)
+                                Dim stepCondition As IOperation = New Binary(stepRelationalCode,
+                                                                             stepValue,
+                                                                             New Literal(Semantics.Expression.SynthesizeNumeric(stepValue.Type, 0), BoundFor.StepValue.Type, BoundFor.StepValue.Syntax),
+                                                                             booleanType,
+                                                                             BoundFor.StepValue.Syntax)
 
                                 Dim positiveStepRelationalCode As BinaryOperationKind = DeriveBinaryOperationKind(BinaryOperatorKind.LessThanOrEqual, controlVariable)
                                 Dim positiveStepCondition As IOperation = New Binary(positiveStepRelationalCode, controlVariable, limitValue, booleanType, limitValue.Syntax)

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

@@ -1654,5 +1654,75 @@ End Class
                 Diagnostic(ConditionalAccessOperationTestAnalyzer.ConditionalAccessOperationDescriptor.Id, "Field1?.M0(Nothing)").WithLocation(32, 9),
                 Diagnostic(ConditionalAccessOperationTestAnalyzer.ConditionalAccessInstanceOperationDescriptor.Id, "Field1?.M0(Nothing)").WithLocation(32, 9))
         End Sub
+
+        <WorkItem(8955, "https://github.com/dotnet/roslyn/issues/8955")>
+        <Fact>
+        Public Sub ForToLoopConditionCrashVisualBasic()
+            Dim source = <compilation>
+                             <file name="c.vb">
+                                 <![CDATA[
+Imports System   
+
+Module M1
+    Class C1(Of t)
+        Shared Widening Operator CType(ByVal p1 As C1(Of t)) As Integer
+            Return 1
+        End Operator
+        Shared Widening Operator CType(ByVal p1 As Integer) As C1(Of t)
+            Return Nothing
+        End Operator
+        Shared Operator -(ByVal p1 As C1(Of t), ByVal p2 As C1(Of t)) As C1(Of Short)
+            Return Nothing
+        End Operator
+        Shared Operator +(ByVal p1 As C1(Of t), ByVal p2 As C1(Of t)) As C1(Of Integer)
+            Return Nothing
+        End Operator
+    End Class
+    Sub foo()
+        For i As C1(Of Integer) = 1 To 10
+        Next
+    End Sub
+End Module
+     
+Module M2
+    ReadOnly Property Moo As Integer
+        Get
+            Return 1
+        End Get
+    End Property
+
+    WriteOnly Property Boo As integer
+        Set(value As integer)
+
+        End Set
+    End Property
+
+    Sub Main()
+        For Moo = 1 to Moo step Moo
+        Next
+
+        For Boo = 1 to Boo step Boo
+        Next
+    End Sub
+End Module
+]]>
+                             </file>
+                         </compilation>
+
+            Dim comp = CompilationUtils.CreateCompilationWithMscorlibAndVBRuntime(source)
+            comp.VerifyDiagnostics(
+                Diagnostic(ERRID.ERR_LoopControlMustNotBeProperty, "Moo").WithLocation(38, 13),
+                Diagnostic(ERRID.ERR_LoopControlMustNotBeProperty, "Boo").WithLocation(41, 13),
+                Diagnostic(ERRID.ERR_NoGetProperty1, "Boo").WithArguments("Boo").WithLocation(41, 24),
+                Diagnostic(ERRID.ERR_NoGetProperty1, "Boo").WithArguments("Boo").WithLocation(41, 33),
+                Diagnostic(ERRID.ERR_UnacceptableForLoopOperator2, "For i As C1(Of Integer) = 1 To 10").WithArguments("Public Shared Operator -(p1 As M1.C1(Of Integer), p2 As M1.C1(Of Integer)) As M1.C1(Of Short)", "M1.C1(Of Integer)").WithLocation(19, 9),
+                Diagnostic(ERRID.ERR_ForLoopOperatorRequired2, "For i As C1(Of Integer) = 1 To 10").WithArguments("M1.C1(Of Integer)", "<=").WithLocation(19, 9),
+                Diagnostic(ERRID.ERR_ForLoopOperatorRequired2, "For i As C1(Of Integer) = 1 To 10").WithArguments("M1.C1(Of Integer)", ">=").WithLocation(19, 9),
+                Diagnostic(ERRID.HDN_UnusedImportStatement, "Imports System").WithLocation(1, 1))
+            comp.VerifyAnalyzerDiagnostics({New ForLoopConditionCrashVBTestAnalyzer}, Nothing, Nothing, False,
+                Diagnostic(ForLoopConditionCrashVBTestAnalyzer.ForLoopConditionCrashDescriptor.Id, "Moo").WithLocation(38, 24),
+                Diagnostic(ForLoopConditionCrashVBTestAnalyzer.ForLoopConditionCrashDescriptor.Id, "Boo").WithLocation(41, 24),
+                Diagnostic(ForLoopConditionCrashVBTestAnalyzer.ForLoopConditionCrashDescriptor.Id, "10").WithLocation(19, 40))
+        End Sub
     End Class
 End Namespace