Address some PROTOTYPE comments (#29607)

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

@@ -1405,7 +1405,6 @@ private ImmutableArray<BoundExpression> BuildArgumentsForErrorRecovery(AnalyzedA
                                 }
                                 else
                                 {
-                                    // PROTOTYPE(NullableReferenceTypes): this TypeSymbolWithAnnotations has bad annotation and context
                                     newArguments[i] = ((OutVariablePendingInference)argument).SetInferredType(TypeSymbolWithAnnotations.Create(candidateType), null);
                                 }
                             }
@@ -1417,7 +1416,6 @@ private ImmutableArray<BoundExpression> BuildArgumentsForErrorRecovery(AnalyzedA
                                 }
                                 else
                                 {
-                                    // PROTOTYPE(NullableReferenceTypes): this TypeSymbolWithAnnotations has bad annotation and context
                                     newArguments[i] = ((BoundDiscardExpression)argument).SetInferredType(TypeSymbolWithAnnotations.Create(candidateType));
                                 }
                             }

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

@@ -1354,7 +1354,6 @@ private BoundExpression InferTypeForDiscardAssignment(BoundDiscardExpression op1
                 diagnostics.Add(ErrorCode.ERR_VoidAssignment, op1.Syntax.Location);
             }
 
-            // PROTOTYPE(NullableReferenceTypes): this TypeSymbolWithAnnotations has bad annotation and context
             return op1.SetInferredType(TypeSymbolWithAnnotations.Create(inferredType));
         }
 

src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs

@@ -1366,9 +1366,9 @@ private static bool HasIdentityConversionInternal(TypeSymbol type1, TypeSymbol t
             Debug.Assert((object)type1 != null);
             Debug.Assert((object)type2 != null);
 
-            // PROTOTYPE(NullableReferenceTypes): If two types differ only by nullability and
+            // https://github.com/dotnet/roslyn/issues/27961: If two types differ only by nullability and
             // one has IsNullable unset and the other doesn't, which do we choose in inference?
-            // See test TypeInference_04.
+            // See NullableReferenceTypesTests.TypeInference_04.
             var compareKind = includeNullability ?
                 TypeCompareKind.AllIgnoreOptions | TypeCompareKind.CompareNullableModifiersForReferenceTypes | TypeCompareKind.UnknownNullableModifierMatchesAny :
                 TypeCompareKind.AllIgnoreOptions;

src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/MethodTypeInference.cs

@@ -2474,7 +2474,7 @@ private static TypeSymbolWithAnnotations Fix(
                     // Make a copy; don't modify the collection as we're iterating it.
                     foreach (var candidate in initialCandidates)
                     {
-                        if (!bound.Equals(candidate, TypeCompareKind.AllAspects))
+                        if (!bound.Equals(candidate, TypeCompareKind.CompareNullableModifiersForReferenceTypes))
                         {
                             if (!ImplicitConversionExists(bound, candidate, ref useSiteDiagnostics, conversions, includeNullability))
                             {
@@ -2498,7 +2498,7 @@ private static TypeSymbolWithAnnotations Fix(
                 {
                     foreach (var candidate in initialCandidates)
                     {
-                        if (!bound.Equals(candidate, TypeCompareKind.AllAspects))
+                        if (!bound.Equals(candidate, TypeCompareKind.CompareNullableModifiersForReferenceTypes))
                         {
                             if (!ImplicitConversionExists(candidate, bound, ref useSiteDiagnostics, conversions, includeNullability))
                             {
@@ -2524,7 +2524,7 @@ private static TypeSymbolWithAnnotations Fix(
             {
                 foreach (var candidate2 in initialCandidates)
                 {
-                    if (!candidate.Equals(candidate2, TypeCompareKind.AllAspects) &&
+                    if (!candidate.Equals(candidate2, TypeCompareKind.CompareNullableModifiersForReferenceTypes) &&
                         !ImplicitConversionExists(candidate2, candidate, ref useSiteDiagnostics, conversions, includeNullability))
                     {
                         goto OuterBreak;
@@ -2546,7 +2546,7 @@ private static TypeSymbolWithAnnotations Fix(
                     Debug.Assert(!(best.IsObjectType() && candidate.IsDynamic()));
                     Debug.Assert(!(best.IsDynamic() && candidate.IsObjectType()));
 
-                    best = Merge(best, candidate, conversions.CorLibrary);
+                    best = MergeNullability(MergeTupleNames(MergeDynamic(best, candidate, conversions.CorLibrary), candidate), candidate);
                 }
                 else
                 {
@@ -2580,12 +2580,6 @@ internal static TypeSymbol Merge(TypeSymbol first, TypeSymbol second, AssemblySy
             return MergeNullability(MergeTupleNames(MergeDynamic(firstWithAnnotations, secondWithAnnotations, corLibrary), secondWithAnnotations), secondWithAnnotations).TypeSymbol;
         }
 
-        // PROTOTYPE(NullableReferenceTypes): Remove this overload.
-        internal static TypeSymbolWithAnnotations Merge(TypeSymbolWithAnnotations first, TypeSymbolWithAnnotations second, AssemblySymbol corLibrary)
-        {
-            return MergeNullability(MergeTupleNames(MergeDynamic(first, second, corLibrary), second), second);
-        }
-
         /// <summary>
         /// Returns first or a modified version of first with merged dynamic flags from both types.
         /// </summary>
@@ -2678,7 +2672,6 @@ private static bool ImplicitConversionExists(TypeSymbolWithAnnotations sourceWit
                 return false;
             }
 
-            // PROTOTYPE(NullableReferenceTypes): Pass includeNullability to ClassifyImplicitConversionFromType.
             return conversions.ClassifyImplicitConversionFromType(source, destination, ref useSiteDiagnostics).Exists;
         }
 

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

@@ -1734,7 +1734,6 @@
   </Node>
 
   <!-- Node only used during nullability flow analysis to represent an expression with an updated nullability -->
-  <!-- PROTOTYPE(NullableReferenceTypes): Derive from BoundValuePlaceholderBase. -->
   <Node Name="BoundExpressionWithNullability" Base="BoundExpression">
     <Field Name="Expression" Type="BoundExpression" Null="disallow"/>
     <Field Name="Type" Type="TypeSymbol" Override="true" Null="allow"/> <!-- We use null Type for placeholders representing out vars -->

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

@@ -28,8 +28,8 @@ internal sealed partial class NullableWalker : DataFlowPassBase<NullableWalker.L
         /// </summary>
         internal sealed class VariableState
         {
-            // PROTOTYPE(NullableReferenceTypes): Reference the collections directly from the original
-            // NullableWalker ratherthan coping the collections. (Items are added to the collections
+            // Consider referencing the collections directly from the original NullableWalker
+            // rather than coping the collections. (Items are added to the collections
             // but never replaced so the collections are lazily populated but otherwise immutable.)
             internal readonly ImmutableDictionary<VariableIdentifier, int> VariableSlot;
             internal readonly ImmutableArray<VariableIdentifier> VariableBySlot;
@@ -1013,7 +1013,7 @@ public override BoundNode VisitLocalDeclaration(BoundLocalDeclaration node)
         protected override BoundExpression VisitExpressionWithoutStackGuard(BoundExpression node)
         {
             Debug.Assert(!IsConditionalState);
-            _resultType = _invalidType; // PROTOTYPE(NullableReferenceTypes): Move to `Visit` method?
+            _resultType = _invalidType;
             var result = base.VisitExpressionWithoutStackGuard(node);
 #if DEBUG
             // Verify Visit method set _result.
@@ -1376,7 +1376,7 @@ private TypeSymbolWithAnnotations InferResultNullability(BinaryOperatorKind oper
                     // PROTOTYPE(NullableReferenceTypes): Conversions: Lifted operator
                     return TypeSymbolWithAnnotations.Create(resultType);
                 }
-                // PROTOTYPE(NullableReferenceTypes): Update method based on operand types.
+                // Update method based on operand types: see https://github.com/dotnet/roslyn/issues/29605.
                 if ((object)methodOpt != null && methodOpt.ParameterCount == 2)
                 {
                     return methodOpt.ReturnType;
@@ -1418,7 +1418,7 @@ private TypeSymbolWithAnnotations InferResultNullability(BinaryOperatorKind oper
         protected override void AfterLeftChildHasBeenVisited(BoundBinaryOperator binary)
         {
             Debug.Assert(!IsConditionalState);
-            //if (this.State.Reachable) // PROTOTYPE(NullableReferenceTypes): Consider reachability?
+            //if (this.State.Reachable) // Consider reachability: see https://github.com/dotnet/roslyn/issues/28798
             {
                 TypeSymbolWithAnnotations leftType = _resultType;
                 bool warnOnNullReferenceArgument = (binary.OperatorKind.IsUserDefined() && (object)binary.MethodOpt != null && binary.MethodOpt.ParameterCount == 2);
@@ -1897,7 +1897,7 @@ public override BoundNode VisitCall(BoundCall node)
             {
                 VisitRvalue(receiverOpt);
                 CheckPossibleNullReceiver(receiverOpt);
-                // PROTOTYPE(NullableReferenceTypes): Update method based on inferred receiver type.
+                // Update method based on inferred receiver type: see https://github.com/dotnet/roslyn/issues/29605.
             }
 
             // PROTOTYPE(NullableReferenceTypes): Can we handle some error cases?
@@ -1919,7 +1919,7 @@ public override BoundNode VisitCall(BoundCall node)
                 ReplayReadsAndWrites(localFunc, node.Syntax, writes: true);
             }
 
-            //if (this.State.Reachable) // PROTOTYPE(NullableReferenceTypes): Consider reachability?
+            //if (this.State.Reachable) // Consider reachability: see https://github.com/dotnet/roslyn/issues/28798
             {
                 _resultType = method.ReturnType;
             }
@@ -3041,7 +3041,7 @@ private TypeSymbolWithAnnotations ApplyConversion(
                             useLegacyWarnings,
                             assignmentKind);
 
-                        // PROTOTYPE(NullableReferenceTypes): Update method based on operandType
+                        // Update method based on operandType: see https://github.com/dotnet/roslyn/issues/29605.
                         // (see NullableReferenceTypesTests.ImplicitConversions_07).
                         var methodOpt = conversion.Method;
                         Debug.Assert((object)methodOpt != null);
@@ -3215,7 +3215,7 @@ public override BoundNode VisitMethodGroup(BoundMethodGroup node)
                 CheckPossibleNullReceiver(receiverOpt);
             }
 
-            //if (this.State.Reachable) // PROTOTYPE(NullableReferenceTypes): Consider reachability?
+            //if (this.State.Reachable) // Consider reachability: see https://github.com/dotnet/roslyn/issues/28798
             {
                 _resultType = default;
             }
@@ -3326,9 +3326,9 @@ private static bool UseLegacyWarnings(BoundExpression expr)
 
         public override BoundNode VisitDeconstructionAssignmentOperator(BoundDeconstructionAssignmentOperator node)
         {
-            // PROTOTYPE(NullableReferenceTypes): Assign each of the deconstructed values.
+            // https://github.com/dotnet/roslyn/issues/29618: Assign each of the deconstructed values,
+            // and handle deconstruction conversion for node.Right.
             VisitLvalue(node.Left);
-            // PROTOTYPE(NullableReferenceTypes): Handle deconstruction conversion node.Right.
             VisitRvalue(node.Right.Operand);
             SetResult(node);
             return null;
@@ -3439,7 +3439,7 @@ public override BoundNode VisitCompoundAssignmentOperator(BoundCompoundAssignmen
             TypeSymbolWithAnnotations resultType;
             Debug.Assert(!IsConditionalState);
 
-            //if (this.State.Reachable) // PROTOTYPE(NullableReferenceTypes): Consider reachability?
+            //if (this.State.Reachable) // Consider reachability: see https://github.com/dotnet/roslyn/issues/28798
             {
                 TypeSymbolWithAnnotations leftOnRightType = GetAdjustedResult(leftType, MakeSlot(node.Left));
 
@@ -3602,7 +3602,7 @@ private void VisitMemberAccess(BoundExpression receiverOpt, Symbol member, bool
         {
             Debug.Assert(!IsConditionalState);
 
-            //if (this.State.Reachable) // PROTOTYPE(NullableReferenceTypes): Consider reachability?
+            //if (this.State.Reachable) // Consider reachability: see https://github.com/dotnet/roslyn/issues/28798
             {
                 VisitRvalue(receiverOpt);
 
@@ -3719,7 +3719,7 @@ public override BoundNode VisitUnaryOperator(BoundUnaryOperator node)
             var result = base.VisitUnaryOperator(node);
             TypeSymbolWithAnnotations resultType = default;
 
-            // PROTOTYPE(NullableReferenceTypes): Update method based on inferred operand type.
+            // Update method based on inferred operand type: see https://github.com/dotnet/roslyn/issues/29605.
             if (node.OperatorKind.IsUserDefined())
             {
                 if (node.OperatorKind.IsLifted())
@@ -3779,7 +3779,7 @@ private TypeSymbolWithAnnotations InferResultNullability(BoundUserDefinedConditi
                 // PROTOTYPE(NullableReferenceTypes): Conversions: Lifted operator
                 return TypeSymbolWithAnnotations.Create(node.Type);
             }
-            // PROTOTYPE(NullableReferenceTypes): Update method based on inferred operand types.
+            // Update method based on inferred operand types: see https://github.com/dotnet/roslyn/issues/29605.
             if ((object)node.LogicalOperator != null && node.LogicalOperator.ParameterCount == 2)
             {
                 return node.LogicalOperator.ReturnType;
@@ -3793,7 +3793,7 @@ private TypeSymbolWithAnnotations InferResultNullability(BoundUserDefinedConditi
         protected override void AfterLeftChildOfBinaryLogicalOperatorHasBeenVisited(BoundExpression node, BoundExpression right, bool isAnd, bool isBool, ref LocalState leftTrue, ref LocalState leftFalse)
         {
             Debug.Assert(!IsConditionalState);
-            //if (this.State.Reachable) // PROTOTYPE(NullableReferenceTypes): Consider reachability?
+            //if (this.State.Reachable) // Consider reachability: see https://github.com/dotnet/roslyn/issues/28798
             {
                 TypeSymbolWithAnnotations leftType = _resultType;
                 // PROTOTYPE(NullableReferenceTypes): Update operator methods based on inferred operand types.
@@ -3872,7 +3872,7 @@ public override BoundNode VisitAwaitExpression(BoundAwaitExpression node)
             }
             else
             {
-                // PROTOTYPE(NullableReferenceTypes): Update method based on inferred receiver type.
+                // Update method based on inferred receiver type: see https://github.com/dotnet/roslyn/issues/29605.
                 _resultType = node.GetResult.ReturnType;
             }
             return result;
@@ -3939,7 +3939,7 @@ public override BoundNode VisitAsOperator(BoundAsOperator node)
         {
             var result = base.VisitAsOperator(node);
 
-            //if (this.State.Reachable) // PROTOTYPE(NullableReferenceTypes): Consider reachability?
+            //if (this.State.Reachable) // Consider reachability: see https://github.com/dotnet/roslyn/issues/28798
             {
                 bool? isNullable = null;
                 if (!node.Type.IsValueType)
@@ -3982,7 +3982,7 @@ public override BoundNode VisitSuppressNullableWarningExpression(BoundSuppressNu
         {
             base.VisitSuppressNullableWarningExpression(node);
 
-            //if (this.State.Reachable) // PROTOTYPE(NullableReferenceTypes): Consider reachability?
+            //if (this.State.Reachable) // Consider reachability: see https://github.com/dotnet/roslyn/issues/28798
             {
                 _resultType = _resultType.IsNull ? default : _resultType.WithTopLevelNonNullabilityForReferenceTypes();
             }
@@ -4018,7 +4018,7 @@ public override BoundNode VisitLiteral(BoundLiteral node)
             var result = base.VisitLiteral(node);
 
             Debug.Assert(!IsConditionalState);
-            //if (this.State.Reachable) // PROTOTYPE(NullableReferenceTypes): Consider reachability?
+            //if (this.State.Reachable) // Consider reachability: see https://github.com/dotnet/roslyn/issues/28798
             {
                 var constant = node.ConstantValue;
 

src/Compilers/CSharp/Portable/Symbols/NamedTypeSymbol.cs

@@ -843,7 +843,8 @@ internal override TypeSymbol SetUnknownNullabilityForReferenceTypes()
         /// parameters in the type.</param>
         public NamedTypeSymbol Construct(params TypeSymbol[] typeArguments)
         {
-            return ConstructWithoutModifiers(typeArguments.AsImmutableOrNull(), false, nonNullTypesContext: null);
+            // PROTOTYPE(NullableReferenceTypes): We should fix the callers to pass an explicit context.
+            return ConstructWithoutModifiers(typeArguments.AsImmutableOrNull(), false, NonNullTypesFalseContext.Instance);
         }
 
         /// <summary>
@@ -865,7 +866,7 @@ public NamedTypeSymbol Construct(INonNullTypesContext nonNullTypesContext, param
         public NamedTypeSymbol Construct(ImmutableArray<TypeSymbol> typeArguments, INonNullTypesContext nonNullTypesContext = null)
         {
             // PROTOTYPE(NullableReferenceTypes): We should fix the callers to pass an explicit context.
-            return ConstructWithoutModifiers(typeArguments, false, nonNullTypesContext);
+            return ConstructWithoutModifiers(typeArguments, false, nonNullTypesContext ?? NonNullTypesFalseContext.Instance);
         }
 
         /// <summary>
@@ -875,7 +876,7 @@ public NamedTypeSymbol Construct(ImmutableArray<TypeSymbol> typeArguments, INonN
         public NamedTypeSymbol Construct(IEnumerable<TypeSymbol> typeArguments)
         {
             // PROTOTYPE(NullableReferenceTypes): We should fix the callers to pass an explicit context.
-            return ConstructWithoutModifiers(typeArguments.AsImmutableOrNull(), false, nonNullTypesContext: null);
+            return ConstructWithoutModifiers(typeArguments.AsImmutableOrNull(), false, NonNullTypesFalseContext.Instance);
         }
 
         /// <summary>