Semantic model tuple types (#36628)

* Correctly report nullability of tuples in the semantic model:
- Store the original tuple literal rather than just its type
- Visit the original literal without diagnostics only in the nullability walker to understand its type
- Update the generator to allow force visiting fields in the nullable re-writer
- Update type checker to allow checking of converted types too
- Update various tuple tests to be correct
- Add extra tests

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

@@ -72,7 +72,7 @@ internal partial class Binder
                     TupleTypeSymbol.ReportNamesMismatchesIfAny(destination, sourceTuple, diagnostics);
                     source = new BoundConvertedTupleLiteral(
                         sourceTuple.Syntax,
-                        sourceTuple.Type,
+                        sourceTuple,
                         sourceTuple.Arguments,
                         sourceTuple.Type, // same type to keep original element names 
                         sourceTuple.HasErrors).WithSuppression(sourceTuple.IsSuppressed);
@@ -429,7 +429,7 @@ private BoundExpression CreateTupleLiteralConversion(SyntaxNode syntax, BoundTup
 
             BoundExpression result = new BoundConvertedTupleLiteral(
                 sourceTuple.Syntax,
-                sourceTuple.Type,
+                sourceTuple,
                 convertedArguments.ToImmutableAndFree(),
                 targetType).WithSuppression(sourceTuple.IsSuppressed);
 

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

@@ -1527,8 +1527,8 @@
   <Node Name="BoundConvertedTupleLiteral" Base="BoundTupleExpression">
     <!-- Converted tuple must have a type -->
     <Field Name="Type" Type="TypeSymbol" Override="true" Null="disallow"/>
-    <!-- Natural type is preserved for the purpose of semantic model. Can be null -->
-    <Field Name="NaturalTypeOpt" Type="TypeSymbol" Null="allow"/>
+    <!-- Original tuple is preserved for the purpose of semantic model -->
+    <Field Name="SourceTuple" Type="BoundTupleLiteral" SkipInVisitor="ExceptNullabilityRewriter" />
   </Node>
   
   <Node Name="BoundDynamicObjectCreationExpression" Base="BoundExpression">

src/Compilers/CSharp/Portable/Compilation/CSharpSemanticModel.cs

@@ -1955,7 +1955,7 @@ private static bool IsUserDefinedTrueOrFalse(BoundUnaryOperator @operator)
                                 // The bound tree always fully binds tuple literals. From the language point of
                                 // view, however, converted tuple literals represent tuple conversions
                                 // from tuple literal expressions which may or may not have types
-                                type = tupleLiteral.NaturalTypeOpt;
+                                type = tupleLiteral.SourceTuple.Type;
                                 break;
                             }
                     }
@@ -1987,7 +1987,7 @@ private static bool IsUserDefinedTrueOrFalse(BoundUnaryOperator @operator)
                     if (tupleLiteralConversion.Operand.Kind == BoundKind.ConvertedTupleLiteral)
                     {
                         var convertedTuple = (BoundConvertedTupleLiteral)tupleLiteralConversion.Operand;
-                        type = convertedTuple.NaturalTypeOpt;
+                        type = convertedTuple.SourceTuple.Type;
                         nullability = convertedTuple.TopLevelNullability;
                     }
                     else

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

@@ -165,6 +165,12 @@ private void VisitBinaryOperatorChildren(BoundBinaryOperatorBase node)
                     node = child;
                 }
             }
+
+            public override BoundNode VisitConvertedTupleLiteral(BoundConvertedTupleLiteral node)
+            {
+                Visit(node.SourceTuple);
+                return base.VisitConvertedTupleLiteral(node);
+            }
         }
 #endif
     }

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

@@ -4047,6 +4047,10 @@ public override BoundNode VisitTupleLiteral(BoundTupleLiteral node)
 
         public override BoundNode VisitConvertedTupleLiteral(BoundConvertedTupleLiteral node)
         {
+            // Visit the source tuple so that the semantic model can correctly report nullability for it
+            // Disable diagnostics, as we don't want to duplicate any that are produced by visiting the converted literal below
+            VisitWithoutDiagnostics(node.SourceTuple);
+
             VisitTupleExpression(node);
             return null;
         }

src/Compilers/CSharp/Portable/Generated/BoundNodes.xml.Generated.cs

@@ -5701,25 +5701,26 @@ protected override BoundExpression ShallowClone()
 
     internal sealed partial class BoundConvertedTupleLiteral : BoundTupleExpression
     {
-        public BoundConvertedTupleLiteral(SyntaxNode syntax, TypeSymbol naturalTypeOpt, ImmutableArray<BoundExpression> arguments, TypeSymbol type, bool hasErrors = false)
-            : base(BoundKind.ConvertedTupleLiteral, syntax, arguments, type, hasErrors || arguments.HasErrors())
+        public BoundConvertedTupleLiteral(SyntaxNode syntax, BoundTupleLiteral sourceTuple, ImmutableArray<BoundExpression> arguments, TypeSymbol type, bool hasErrors = false)
+            : base(BoundKind.ConvertedTupleLiteral, syntax, arguments, type, hasErrors || sourceTuple.HasErrors() || arguments.HasErrors())
         {
 
+            Debug.Assert((object)sourceTuple != null, "Field 'sourceTuple' cannot be null (use Null=\"allow\" in BoundNodes.xml to remove this check)");
             Debug.Assert(!arguments.IsDefault, "Field 'arguments' cannot be null (use Null=\"allow\" in BoundNodes.xml to remove this check)");
             Debug.Assert((object)type != null, "Field 'type' cannot be null (use Null=\"allow\" in BoundNodes.xml to remove this check)");
 
-            this.NaturalTypeOpt = naturalTypeOpt;
+            this.SourceTuple = sourceTuple;
         }
 
 
-        public TypeSymbol NaturalTypeOpt { get; }
+        public BoundTupleLiteral SourceTuple { get; }
         public override BoundNode Accept(BoundTreeVisitor visitor) => visitor.VisitConvertedTupleLiteral(this);
 
-        public BoundConvertedTupleLiteral Update(TypeSymbol naturalTypeOpt, ImmutableArray<BoundExpression> arguments, TypeSymbol type)
+        public BoundConvertedTupleLiteral Update(BoundTupleLiteral sourceTuple, ImmutableArray<BoundExpression> arguments, TypeSymbol type)
         {
-            if (!TypeSymbol.Equals(naturalTypeOpt, this.NaturalTypeOpt, TypeCompareKind.ConsiderEverything) || arguments != this.Arguments || !TypeSymbol.Equals(type, this.Type, TypeCompareKind.ConsiderEverything))
+            if (sourceTuple != this.SourceTuple || arguments != this.Arguments || !TypeSymbol.Equals(type, this.Type, TypeCompareKind.ConsiderEverything))
             {
-                var result = new BoundConvertedTupleLiteral(this.Syntax, naturalTypeOpt, arguments, type, this.HasErrors);
+                var result = new BoundConvertedTupleLiteral(this.Syntax, sourceTuple, arguments, type, this.HasErrors);
                 result.CopyAttributes(this);
                 return result;
             }
@@ -5728,7 +5729,7 @@ public BoundConvertedTupleLiteral Update(TypeSymbol naturalTypeOpt, ImmutableArr
 
         protected override BoundExpression ShallowClone()
         {
-            var result = new BoundConvertedTupleLiteral(this.Syntax, this.NaturalTypeOpt, this.Arguments, this.Type, this.HasErrors);
+            var result = new BoundConvertedTupleLiteral(this.Syntax, this.SourceTuple, this.Arguments, this.Type, this.HasErrors);
             result.CopyAttributes(this);
             return result;
         }
@@ -9985,10 +9986,10 @@ public override BoundNode VisitTupleLiteral(BoundTupleLiteral node)
         }
         public override BoundNode VisitConvertedTupleLiteral(BoundConvertedTupleLiteral node)
         {
+            BoundTupleLiteral sourceTuple = node.SourceTuple;
             ImmutableArray<BoundExpression> arguments = this.VisitList(node.Arguments);
-            TypeSymbol naturalTypeOpt = this.VisitType(node.NaturalTypeOpt);
             TypeSymbol type = this.VisitType(node.Type);
-            return node.Update(naturalTypeOpt, arguments, type);
+            return node.Update(sourceTuple, arguments, type);
         }
         public override BoundNode VisitDynamicObjectCreationExpression(BoundDynamicObjectCreationExpression node)
         {
@@ -11467,17 +11468,18 @@ public override BoundNode VisitTupleLiteral(BoundTupleLiteral node)
 
         public override BoundNode VisitConvertedTupleLiteral(BoundConvertedTupleLiteral node)
         {
+            BoundTupleLiteral sourceTuple = (BoundTupleLiteral)this.Visit(node.SourceTuple);
             ImmutableArray<BoundExpression> arguments = this.VisitList(node.Arguments);
             BoundConvertedTupleLiteral updatedNode;
 
             if (_updatedNullabilities.TryGetValue(node, out (NullabilityInfo Info, TypeSymbol Type) infoAndType))
             {
-                updatedNode = node.Update(node.NaturalTypeOpt, arguments, infoAndType.Type);
+                updatedNode = node.Update(sourceTuple, arguments, infoAndType.Type);
                 updatedNode.TopLevelNullability = infoAndType.Info;
             }
             else
             {
-                updatedNode = node.Update(node.NaturalTypeOpt, arguments, node.Type);
+                updatedNode = node.Update(sourceTuple, arguments, node.Type);
             }
             return updatedNode;
         }
@@ -13171,7 +13173,7 @@ public override TreeDumperNode VisitYieldBreakStatement(BoundYieldBreakStatement
         );
         public override TreeDumperNode VisitConvertedTupleLiteral(BoundConvertedTupleLiteral node, object arg) => new TreeDumperNode("convertedTupleLiteral", null, new TreeDumperNode[]
         {
-            new TreeDumperNode("naturalTypeOpt", node.NaturalTypeOpt, null),
+            new TreeDumperNode("sourceTuple", null, new TreeDumperNode[] { Visit(node.SourceTuple, null) }),
             new TreeDumperNode("arguments", null, from x in node.Arguments select Visit(x, null)),
             new TreeDumperNode("type", node.Type, null),
             new TreeDumperNode("isSuppressed", node.IsSuppressed, null)

src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs

@@ -1797,7 +1797,7 @@ internal IOperation CreateBoundTupleLiteralOperation(BoundTupleLiteral boundTupl
 
         internal IOperation CreateBoundConvertedTupleLiteralOperation(BoundConvertedTupleLiteral boundConvertedTupleLiteral, bool createDeclaration = true)
         {
-            return CreateTupleOperation(boundConvertedTupleLiteral, boundConvertedTupleLiteral.NaturalTypeOpt, createDeclaration);
+            return CreateTupleOperation(boundConvertedTupleLiteral, boundConvertedTupleLiteral.SourceTuple.Type, createDeclaration);
         }
 
         internal IOperation CreateTupleOperation(BoundTupleExpression boundTupleExpression, ITypeSymbol naturalType, bool createDeclaration)

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenTupleEqualityTests.cs

@@ -2191,7 +2191,6 @@ public static void Main()
 Operator '!=' cannot be applied to operands of type 'System.ValueTuple<int,int,int>' and 'System.ValueTuple<int,int,int>'");
         }
 
-        // https://github.com/dotnet/roslyn/issues/35010 Support deconstruction assignment
         [Fact]
         public void TestComparisonWithDeconstructionResult()
         {

src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs

@@ -2882,7 +2882,6 @@ .maxstack  1
 }");
         }
 
-        // https://github.com/dotnet/roslyn/issues/35010 Support deconstruction assignment
         [Fact]
         public void DoNotShareInputForMutatingWhenClause()
         {

src/Compilers/CSharp/Test/Symbol/Symbols/Source/NullablePublicAPITests.cs

@@ -979,5 +979,37 @@ class C
             var speculativeTypeInfo = specModel.GetTypeInfo(yReference);
             Assert.Equal(PublicNullableFlowState.NotNull, speculativeTypeInfo.Nullability.FlowState);
         }
+
+        [Fact]
+        public void TupleAssignment()
+        {
+            var source =
+@"
+#pragma warning disable CS0219
+#nullable enable
+class C
+{
+    void M(C? x, C x2)
+    {
+        (C? a, C b) t = (x, x2) /*T:(C? x, C! x2)*/ /*CT:(C? a, C! b)*/;
+        (object a, int b) t2 = (x, (short)0)/*T:(C? x, short)*/ /*CT:(object! a, int b)*/; // 1
+        (object a, int b) t3 = (default, default) /*T:<null>!*/ /*CT:(object! a, int b)*/; // 2
+        (object a, int b) t4 = (default(object), default(int)) /*T:(object?, int)*/ /*CT:(object! a, int b)*/; // 3
+    }
+}";
+            var comp = CreateCompilation(source);
+            comp.VerifyTypes();
+            comp.VerifyDiagnostics(
+                // (9,32): warning CS8619: Nullability of reference types in value of type '(object? x, int)' doesn't match target type '(object a, int b)'.
+                //         (object a, int b) t2 = (x, (short)0)/*T:(C? x, short)*/ /*CT:(object! a, int b)*/; // 1
+                Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "(x, (short)0)").WithArguments("(object? x, int)", "(object a, int b)").WithLocation(9, 32),
+                // (10,32): warning CS8619: Nullability of reference types in value of type '(object?, int)' doesn't match target type '(object a, int b)'.
+                //         (object a, int b) t3 = (default, default) /*T:<null>!*/ /*CT:(object! a, int b)*/; //2
+                Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "(default, default)").WithArguments("(object?, int)", "(object a, int b)").WithLocation(10, 32),
+                // (11,32): warning CS8619: Nullability of reference types in value of type '(object?, int)' doesn't match target type '(object a, int b)'.
+                //         (object a, int b) t4 = (default(object), default(int)) /*T:(object?, int)*/ /*CT:(object! a, int b)*/; // 3
+                Diagnostic(ErrorCode.WRN_NullabilityMismatchInAssignment, "(default(object), default(int))").WithArguments("(object?, int)", "(object a, int b)").WithLocation(11, 32)
+                );
+        }
     }
 }

src/Compilers/Test/Utilities/CSharp/CompilationTestUtils.cs

@@ -324,15 +324,22 @@ internal static void VerifyTypes(this CSharpCompilation compilation, SyntaxTree
                         Assert.NotEqual(CodeAnalysis.NullableAnnotation.NotApplicable, typeInfo.Nullability.Annotation);
                         Assert.NotEqual(CodeAnalysis.NullableFlowState.NotApplicable, typeInfo.Nullability.FlowState);
                         // https://github.com/dotnet/roslyn/issues/35035: After refactoring symboldisplay, we should be able to just call something like typeInfo.Type.ToDisplayString(typeInfo.Nullability.FlowState, TypeWithState.TestDisplayFormat)
-                        return TypeWithState.Create((TypeSymbol)typeInfo.Type, typeInfo.Nullability.FlowState.ToInternalFlowState()).ToTypeWithAnnotations().ToDisplayString(TypeWithAnnotations.TestDisplayFormat);
+                        if (annotation.IsConverted)
+                        {
+                            return TypeWithState.Create((TypeSymbol)typeInfo.ConvertedType, typeInfo.ConvertedNullability.FlowState.ToInternalFlowState()).ToTypeWithAnnotations().ToDisplayString(TypeWithAnnotations.TestDisplayFormat);
+                        }
+                        else
+                        {
+                            return TypeWithState.Create((TypeSymbol)typeInfo.Type, typeInfo.Nullability.FlowState.ToInternalFlowState()).ToTypeWithAnnotations().ToDisplayString(TypeWithAnnotations.TestDisplayFormat);
+                        }
                     });
                 // Consider reporting the correct source with annotations on mismatch.
                 AssertEx.Equal(expectedTypes, actualTypes, message: method.ToTestDisplayString());
             }
 
-            ImmutableArray<(ExpressionSyntax Expression, string Text)> getAnnotations()
+            ImmutableArray<(ExpressionSyntax Expression, string Text, bool IsConverted)> getAnnotations()
             {
-                var builder = ArrayBuilder<(ExpressionSyntax, string)>.GetInstance();
+                var builder = ArrayBuilder<(ExpressionSyntax, string, bool)>.GetInstance();
                 foreach (var token in root.DescendantTokens())
                 {
                     foreach (var trivia in token.TrailingTrivia)
@@ -340,15 +347,18 @@ internal static void VerifyTypes(this CSharpCompilation compilation, SyntaxTree
                         if (trivia.Kind() == SyntaxKind.MultiLineCommentTrivia)
                         {
                             var text = trivia.ToFullString();
-                            const string prefix = "/*T:";
+                            const string typePrefix = "/*T:";
+                            const string convertedPrefix = "/*CT:";
                             const string suffix = "*/";
-                            if (text.StartsWith(prefix) && text.EndsWith(suffix))
+                            bool startsWithTypePrefix = text.StartsWith(typePrefix);
+                            if (text.EndsWith(suffix) && (startsWithTypePrefix || text.StartsWith(convertedPrefix)))
                             {
+                                var prefix = startsWithTypePrefix ? typePrefix : convertedPrefix;
                                 var expr = getEnclosingExpression(token);
                                 Assert.True(expr != null, $"VerifyTypes could not find a matching expression for annotation '{text}'.");
 
                                 var content = text.Substring(prefix.Length, text.Length - prefix.Length - suffix.Length);
-                                builder.Add((expr, content));
+                                builder.Add((expr, content, !startsWithTypePrefix));
                             }
                         }
                     }

src/Tools/Source/CompilerGeneratorTools/Source/BoundTreeGenerator/BoundNodeClassWriter.cs

@@ -1493,7 +1493,7 @@ private void WriteNullabilityRewriter()
                             foreach (var field in AllNodeOrNodeListFields(node))
                             {
                                 hadField = true;
-                                WriteNodeVisitCall(field);
+                                WriteNodeVisitCall(field, forceVisit: VisitFieldOnlyInNullabilityRewriter(field));
                             }
 
                             if (hadField)
@@ -1705,7 +1705,13 @@ private static bool IsPropertyOverrides(Field f)
 
         private static bool SkipInVisitor(Field f)
         {
-            return string.Compare(f.SkipInVisitor, "true", true) == 0;
+            return string.Compare(f.SkipInVisitor, "true", true) == 0
+                || VisitFieldOnlyInNullabilityRewriter(f);
+        }
+
+        private static bool VisitFieldOnlyInNullabilityRewriter(Field f)
+        {
+            return string.Compare(f.SkipInVisitor, "ExceptNullabilityRewriter", true) == 0;
         }
 
         private static bool SkipInNullabilityRewriter(Node n)
@@ -1777,12 +1783,12 @@ private string GetVisitFunctionDeclaration(string nodeName, bool isOverride)
             }
         }
 
-        private void WriteNodeVisitCall(Field field)
+        private void WriteNodeVisitCall(Field field, bool forceVisit = false)
         {
             switch (_targetLang)
             {
                 case TargetLanguage.CSharp:
-                    if (SkipInVisitor(field))
+                    if (SkipInVisitor(field) && !forceVisit)
                     {
                         WriteLine($"{field.Type} {ToCamelCase(field.Name)} = node.{field.Name};");
                     }