Join in state from parameter default value (#47171)

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

@@ -1709,7 +1709,9 @@ private static bool ShouldReportNullableAssignment(TypeWithAnnotations type, Nul
             }
 
             if (value == null ||
-                value.WasCompilerGenerated ||
+                // This prevents us from giving undesired warnings for synthesized arguments for optional parameters,
+                // but allows us to give warnings for synthesized assignments to record properties and for parameter default values at the declaration site.
+                (value.WasCompilerGenerated && assignmentKind == AssignmentKind.Argument) ||
                 !ShouldReportNullableAssignment(targetType, valueType.State))
             {
                 return;
@@ -2150,9 +2152,33 @@ private void EnterParameters()
                 return;
             }
 
+            if (methodSymbol is SynthesizedRecordConstructor)
+            {
+                if (_hasInitialState)
+                {
+                    // A record primary constructor's parameters are entered before analyzing initializers.
+                    // On the second pass, the correct parameter states (potentially modified by initializers)
+                    // are contained in the initial state.
+                    return;
+                }
+            }
+            else if (methodSymbol.IsConstructor())
+            {
+                if (!_hasInitialState)
+                {
+                    // For most constructors, we only enter parameters after analyzing initializers.
+                    return;
+                }
+            }
+
+            // The partial definition part may include optional parameters whose default values we want to simulate assigning at the beginning of the method
+            methodSymbol = methodSymbol.PartialDefinitionPart ?? methodSymbol;
+
             var methodParameters = methodSymbol.Parameters;
             var signatureParameters = (_useDelegateInvokeParameterTypes ? _delegateInvokeMethod! : methodSymbol).Parameters;
 
+            // save a state representing the possibility that parameter default values were not assigned to the parameters.
+            var parameterDefaultsNotAssignedState = State.Clone();
             for (int i = 0; i < methodParameters.Length; i++)
             {
                 var parameter = methodParameters[i];
@@ -2161,6 +2187,7 @@ private void EnterParameters()
                 var parameterType = i >= signatureParameters.Length ? parameter.TypeWithAnnotations : signatureParameters[i].TypeWithAnnotations;
                 EnterParameter(parameter, parameterType);
             }
+            Join(ref State, ref parameterDefaultsNotAssignedState);
         }
 
         private void EnterParameter(ParameterSymbol parameter, TypeWithAnnotations parameterType)
@@ -2181,6 +2208,26 @@ private void EnterParameter(ParameterSymbol parameter, TypeWithAnnotations param
                         valueSlot: -1,
                         isDefaultValue: parameter.ExplicitDefaultConstantValue?.IsNull == true);
                 }
+
+                if (parameter.IsOptional)
+                {
+                    // When a parameter has a default value, we initialize its flow state by simulating an assignment of the default value to the parameter.
+                    var syntax = parameter.GetNonNullSyntaxNode();
+                    var rightSyntax = syntax is ParameterSyntax { Default: { Value: { } value } } ? value : syntax;
+                    var right = GetDefaultParameterValue(rightSyntax, parameter);
+
+                    var parameterAnnotations = GetParameterAnnotations(parameter);
+                    var parameterLValueType = ApplyLValueAnnotations(parameterType, parameterAnnotations);
+
+                    var previous = _disableNullabilityAnalysis;
+                    _disableNullabilityAnalysis = true;
+                    VisitOptionalImplicitConversion(right, parameterLValueType, useLegacyWarnings: true, trackMembers: true, assignmentKind: AssignmentKind.Assignment);
+                    _disableNullabilityAnalysis = previous;
+
+                    // If the LHS has annotations, we perform an additional check for nullable value types
+                    CheckDisallowedNullAssignment(ResultType, parameterAnnotations, right.Syntax.Location);
+                    TrackNullableStateForAssignment(right, parameterType, slot, ResultType, MakeSlot(right));
+                }
             }
         }
 
@@ -4997,11 +5044,7 @@ void markMembersAsNotNull(int receiverSlot, TypeSymbol type, string memberName,
                     {
                         var annotations = GetParameterAnnotations(parameter);
 
-                        _defaultValuesOpt ??= PooledDictionary<(SyntaxNode, ParameterSymbol), BoundExpression>.GetInstance();
-                        if (!_defaultValuesOpt.TryGetValue((syntax, parameter), out var argument))
-                        {
-                            _defaultValuesOpt[(syntax, parameter)] = argument = LocalRewriter.GetDefaultParameterValue(syntax, parameter, enableCallerInfo: ThreeState.True, localRewriter: null, _binder, Diagnostics);
-                        }
+                        BoundExpression argument = GetDefaultParameterValue(syntax, parameter);
                         resultsBuilder.Add(VisitArgumentEvaluate(argument, RefKind.None, annotations));
                         argumentsBuilder.Add(argument);
                         argsToParamsBuilder?.Add(i);
@@ -5028,6 +5071,17 @@ void markMembersAsNotNull(int receiverSlot, TypeSymbol type, string memberName,
             }
         }
 
+        private BoundExpression GetDefaultParameterValue(SyntaxNode syntax, ParameterSymbol parameter)
+        {
+            _defaultValuesOpt ??= PooledDictionary<(SyntaxNode, ParameterSymbol), BoundExpression>.GetInstance();
+            if (!_defaultValuesOpt.TryGetValue((syntax, parameter), out var argument))
+            {
+                _defaultValuesOpt[(syntax, parameter)] = argument = LocalRewriter.GetDefaultParameterValue(syntax, parameter, enableCallerInfo: ThreeState.True, localRewriter: null, _binder, Diagnostics);
+            }
+
+            return argument;
+        }
+
         private VisitArgumentResult VisitArgumentEvaluate(BoundExpression argument, RefKind refKind, FlowAnalysisAnnotations annotations)
         {
             Debug.Assert(!IsConditionalState);

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

@@ -1454,6 +1454,10 @@ private BoundExpression GetDefaultParameterValue(SyntaxNode syntax, ParameterSym
                 defaultValue = MakeConversionNode(defaultValue, parameterType, @checked: false, acceptFailingConversion: true);
             }
 
+            // CONSIDER: it feels like determining the exact value that will be emitted for a
+            // parameter default value is something that should be figured out at binding time, not in lowering.
+            defaultValue = defaultValue.WithSuppression(syntax.IsKind(SyntaxKind.SuppressNullableWarningExpression));
+
             return defaultValue;
 
             BoundExpression MakeConversionNode(BoundExpression operand, TypeSymbol type, bool @checked, bool acceptFailingConversion = false)

src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs

@@ -278,39 +278,10 @@ protected ConstantValue MakeDefaultExpression(DiagnosticBag diagnostics, Binder
                 }
             }
 
-            if (parameterType.Type.IsReferenceType &&
-                parameterType.NullableAnnotation.IsNotAnnotated() &&
-                convertedExpression.ConstantValue?.IsNull == true &&
-                !suppressNullableWarning(convertedExpression) &&
-                DeclaringCompilation.LanguageVersion >= MessageID.IDS_FeatureNullableReferenceTypes.RequiredVersion())
-            {
-                diagnostics.Add(ErrorCode.WRN_NullAsNonNullable, parameterSyntax.Default.Value.Location);
-            }
-
             // represent default(struct) by a Null constant:
             var value = convertedExpression.ConstantValue ?? ConstantValue.Null;
             VerifyParamDefaultValueMatchesAttributeIfAny(value, defaultSyntax.Value, diagnostics);
             return value;
-
-            bool suppressNullableWarning(BoundExpression expr)
-            {
-                while (true)
-                {
-                    if (expr.IsSuppressed)
-                    {
-                        return true;
-                    }
-
-                    switch (expr.Kind)
-                    {
-                        case BoundKind.Conversion:
-                            expr = ((BoundConversion)expr).Operand;
-                            break;
-                        default:
-                            return false;
-                    }
-                }
-            }
         }
 
         public override string MetadataName

src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs

@@ -7944,35 +7944,60 @@ public void WithExpr_NullableAnalysis_07()
 #nullable enable
 using System.Diagnostics.CodeAnalysis;
 
-record B([AllowNull] string X)
+record B([AllowNull] string X) // 1
 {
     static void M1(B b)
     {
         b.X.ToString();
-        b = b with { X = null }; // ok
-        b.X.ToString(); // ok
+        b = b with { X = null }; // 2
+        b.X.ToString(); // 3
         b = new B((string?)null);
-        b.X.ToString(); // ok
+        b.X.ToString();
     }
 }";
-            // We should have a way to propagate attributes on
-            // positional parameters to the corresponding properties.
-            // https://github.com/dotnet/roslyn/issues/44691
             var comp = CreateCompilation(new[] { src, AllowNullAttributeDefinition });
             comp.VerifyDiagnostics(
+                // (5,10): warning CS8601: Possible null reference assignment.
+                // record B([AllowNull] string X) // 1
+                Diagnostic(ErrorCode.WRN_NullReferenceAssignment, "[AllowNull] string X").WithLocation(5, 10),
                 // (10,26): warning CS8625: Cannot convert null literal to non-nullable reference type.
-                //         b = b with { X = null }; // ok
+                //         b = b with { X = null }; // 2
                 Diagnostic(ErrorCode.WRN_NullAsNonNullable, "null").WithLocation(10, 26),
                 // (11,9): warning CS8602: Dereference of a possibly null reference.
-                //         b.X.ToString(); // ok
+                //         b.X.ToString(); // 3
                 Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(11, 9));
         }
 
-        [Fact]
+        [Fact, WorkItem(44691, "https://github.com/dotnet/roslyn/issues/44691")]
         public void WithExpr_NullableAnalysis_08()
         {
             var src = @"
 #nullable enable
+using System.Diagnostics.CodeAnalysis;
+
+record B([property: AllowNull][AllowNull] string X)
+{
+    static void M1(B b)
+    {
+        b.X.ToString();
+        b = b with { X = null };
+        b.X.ToString(); // 1
+        b = new B((string?)null);
+        b.X.ToString();
+    }
+}";
+            var comp = CreateCompilation(new[] { src, AllowNullAttributeDefinition });
+            comp.VerifyDiagnostics(
+                // (11,9): warning CS8602: Dereference of a possibly null reference.
+                //         b.X.ToString(); // 1
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "b.X").WithLocation(11, 9));
+        }
+
+        [Fact]
+        public void WithExpr_NullableAnalysis_09()
+        {
+            var src = @"
+#nullable enable
 record B(string? X, string? Y)
 {
     static void M1(B b1)
@@ -8008,7 +8033,7 @@ static void M1(B b1)
         }
 
         [Fact]
-        public void WithExpr_NullableAnalysis_09()
+        public void WithExpr_NullableAnalysis_10()
         {
             var src = @"
 #nullable enable
@@ -8032,7 +8057,7 @@ static void M1(B b1)
         }
 
         [Fact]
-        public void WithExpr_NullableAnalysis_10()
+        public void WithExpr_NullableAnalysis_11()
         {
             var src = @"
 #nullable enable

src/Compilers/Core/Portable/Symbols/Attributes/CommonAttributeData.cs

@@ -158,12 +158,12 @@ static bool namespaceMatch(INamespaceSymbolInternal container, string namespaceN
         /// For user defined attributes VB allows duplicate named arguments and uses the last value.
         /// Dev11 reports an error for pseudo-custom attributes when emitting metadata. We don't.
         /// </remarks>
-        internal T? DecodeNamedArgument<T>(string name, SpecialType specialType, T defaultValue = default)
+        internal T? DecodeNamedArgument<T>(string name, SpecialType specialType, T? defaultValue = default)
         {
             return DecodeNamedArgument<T>(CommonNamedArguments, name, specialType, defaultValue);
         }
 
-        private static T? DecodeNamedArgument<T>(ImmutableArray<KeyValuePair<string, TypedConstant>> namedArguments, string name, SpecialType specialType, T defaultValue = default)
+        private static T? DecodeNamedArgument<T>(ImmutableArray<KeyValuePair<string, TypedConstant>> namedArguments, string name, SpecialType specialType, T? defaultValue = default)
         {
             int index = IndexOfNamedArgument(namedArguments, name);
             return index >= 0 ? namedArguments[index].Value.DecodeValue<T>(specialType) : defaultValue;