Address feedback from Chuck

dotnet · Mar 25, 2021 · 2484dc0 · 2484dc0
1 parent 1b2eb98
commit 2484dc0
Show file tree

Hide file tree

Showing 5 changed files with 60 additions and 29 deletions.
diff --git a/src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs b/src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs
@@ -1032,7 +1032,7 @@ internal Cci.IFieldReference Translate(
             Debug.Assert(fieldSymbol.IsDefinitionOrDistinct());
             Debug.Assert(!fieldSymbol.IsVirtualTupleField &&
                 (object)(fieldSymbol.TupleUnderlyingField ?? fieldSymbol) == fieldSymbol &&
-                !(fieldSymbol is TupleErrorFieldSymbol), "tuple fields should be rewritten to underlying by now");
+                fieldSymbol is not TupleErrorFieldSymbol, "tuple fields should be rewritten to underlying by now");
 
             if (!fieldSymbol.IsDefinition)
             {

diff --git a/src/Compilers/CSharp/Portable/Symbols/FieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/FieldSymbol.cs
@@ -405,7 +405,7 @@ public virtual bool IsDefaultTupleElement
         {
             get
             {
-                Debug.Assert(!(this is TupleElementFieldSymbol) && !(this is TupleErrorFieldSymbol));
+                Debug.Assert(!(this is TupleElementFieldSymbol or TupleErrorFieldSymbol));
                 return TupleElementIndex >= 0;
             }
         }
@@ -414,7 +414,7 @@ public virtual bool IsExplicitlyNamedTupleElement
         {
             get
             {
-                Debug.Assert(!(this is TupleElementFieldSymbol) && !(this is TupleErrorFieldSymbol));
+                Debug.Assert(!(this is TupleElementFieldSymbol or TupleErrorFieldSymbol));
                 return false;
             }
         }
@@ -464,15 +464,12 @@ public virtual int TupleElementIndex
             get
             {
                 // wrapped tuple fields already have this information and override this property
-                Debug.Assert(!(this is TupleElementFieldSymbol) && !(this is TupleErrorFieldSymbol));
+                Debug.Assert(!(this is TupleElementFieldSymbol or TupleErrorFieldSymbol));
 
-                if (ContainingType.IsTupleType)
+                var map = ContainingType.TupleFieldDefinitionsToIndexMap;
+                if (map is object && map.TryGetValue(this.OriginalDefinition, out int index))
                 {
-                    var map = ContainingType.TupleFieldDefinitionsToIndexMap;
-                    if (map is object && map.TryGetValue(this.OriginalDefinition, out int index))
-                    {
-                        return index;
-                    }
+                    return index;
                 }
 
                 return -1;

diff --git a/src/Compilers/CSharp/Portable/Symbols/Tuples/TupleFieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Tuples/TupleFieldSymbol.cs
@@ -96,12 +96,7 @@ public override Symbol? AssociatedSymbol
         {
             get
             {
-                if (!TypeSymbol.Equals(_underlyingField.ContainingType, _containingTuple.TupleUnderlyingType, TypeCompareKind.ConsiderEverything))
-                {
-                    return null;
-                }
-
-                return _containingTuple.GetTupleMemberSymbolForUnderlyingMember(_underlyingField.AssociatedSymbol);
+                return null;
             }
         }
 

diff --git a/src/Compilers/CSharp/Portable/Symbols/Tuples/TupleTypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Tuples/TupleTypeSymbol.cs
@@ -596,7 +596,7 @@ public virtual void InitializeTupleFieldDefinitionsToIndexMap()
             _ = this.GetMembers();
         }
 
-        public TMember? GetTupleMemberSymbolForUnderlyingMember<TMember>(TMember underlyingMemberOpt) where TMember : Symbol?
+        public TMember? GetTupleMemberSymbolForUnderlyingMember<TMember>(TMember? underlyingMemberOpt) where TMember : Symbol
         {
             return IsTupleType ? TupleData!.GetTupleMemberSymbolForUnderlyingMember(underlyingMemberOpt) : null;
         }
@@ -656,7 +656,7 @@ protected ArrayBuilder<Symbol> AddOrWrapTupleMembers(ImmutableArray<Symbol> curr
                                 }
 
                                 var providedName = elementNames.IsDefault ? null : elementNames[tupleFieldIndex];
-                                ImmutableArray<Location> locations = getElementLocations(ref elementLocations, tupleFieldIndex);
+                                ImmutableArray<Location> locations = getElementLocations(in elementLocations, tupleFieldIndex);
 
                                 var defaultName = TupleMemberName(tupleFieldIndex + 1);
                                 // if provided name does not match the default one,
@@ -685,7 +685,7 @@ protected ArrayBuilder<Symbol> AddOrWrapTupleMembers(ImmutableArray<Symbol> curr
                                     if (IsDefinition)
                                     {
                                         defaultTupleField = field;
-                                        fieldDefinitionsToIndexMap!.Add(field.OriginalDefinition, tupleFieldIndex);
+                                        fieldDefinitionsToIndexMap!.Add(field, tupleFieldIndex);
                                     }
                                     else
                                     {
@@ -890,7 +890,7 @@ static ImmutableArray<Symbol> getOriginalFields(ImmutableArray<Symbol> members)
                 return fields.ToImmutableAndFree();
             }
 
-            static ImmutableArray<Location> getElementLocations(ref ImmutableArray<Location?> elementLocations, int tupleFieldIndex)
+            static ImmutableArray<Location> getElementLocations(in ImmutableArray<Location?> elementLocations, int tupleFieldIndex)
             {
                 if (elementLocations.IsDefault)
                 {
@@ -1175,7 +1175,7 @@ SmallDictionary<Symbol, Symbol> computeDefinitionToMemberMap()
                 }
             }
 
-            public TMember? GetTupleMemberSymbolForUnderlyingMember<TMember>(TMember underlyingMemberOpt) where TMember : Symbol?
+            public TMember? GetTupleMemberSymbolForUnderlyingMember<TMember>(TMember? underlyingMemberOpt) where TMember : Symbol
             {
                 if (underlyingMemberOpt is null)
                 {

diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenTupleTest.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenTupleTest.cs
@@ -21839,16 +21839,16 @@ public class B
                 );
 
             var methodM = comp.GetMember<MethodSymbol>("A.M");
-            Assert.Equal("RetargetingMethodSymbol", methodM.GetType().Name);
+            Assert.IsType<RetargetingMethodSymbol>(methodM);
 
             Assert.Equal("(System.Int32, System.Int32)[missing]", methodM.ReturnTypeWithAnnotations.ToTestDisplayString());
-            Assert.Equal("ConstructedErrorTypeSymbol", methodM.ReturnType.GetType().Name);
-            Assert.Equal("TopLevel", methodM.ReturnType.OriginalDefinition.GetType().Name);
+            Assert.IsType<ConstructedErrorTypeSymbol>(methodM.ReturnType);
+            Assert.IsType<MissingMetadataTypeSymbol.TopLevel>(methodM.ReturnType.OriginalDefinition);
             Assert.True(methodM.ReturnType.IsTupleType);
             Assert.True(methodM.ReturnType.IsErrorType());
             foreach (var item in methodM.ReturnType.TupleElements)
             {
-                Assert.Equal("TupleErrorFieldSymbol", item.GetType().Name);
+                Assert.IsType<TupleErrorFieldSymbol>(item);
                 Assert.False(item.IsExplicitlyNamedTupleElement);
             }
         }
@@ -21879,16 +21879,16 @@ public class B
                 );
 
             var methodM = comp.GetMember<MethodSymbol>("A.M");
-            Assert.Equal("RetargetingMethodSymbol", methodM.GetType().Name);
+            Assert.IsType<RetargetingMethodSymbol>(methodM);
 
             Assert.Equal("(System.Int32 Item1, System.Int32 Bob)[missing]", methodM.ReturnTypeWithAnnotations.ToTestDisplayString());
-            Assert.Equal("ConstructedErrorTypeSymbol", methodM.ReturnType.GetType().Name);
-            Assert.Equal("TopLevel", methodM.ReturnType.OriginalDefinition.GetType().Name);
+            Assert.IsType<ConstructedErrorTypeSymbol>(methodM.ReturnType);
+            Assert.IsType<MissingMetadataTypeSymbol.TopLevel>(methodM.ReturnType.OriginalDefinition);
             Assert.True(methodM.ReturnType.IsTupleType);
             Assert.True(methodM.ReturnType.IsErrorType());
             foreach (var item in methodM.ReturnType.TupleElements)
             {
-                Assert.Equal("TupleErrorFieldSymbol", item.GetType().Name);
+                Assert.IsType<TupleErrorFieldSymbol>(item);
                 Assert.True(item.IsExplicitlyNamedTupleElement);
             }
         }
@@ -28006,6 +28006,45 @@ void verify(string name, string display, bool isTupleElement)
             }
         }
 
+        [Fact]
+        public void TestValueTuple2Definition_WithExtraAutoProperty()
+        {
+            var source = @"
+namespace System
+{
+    public struct ValueTuple<T1, T2>
+    {
+        public T1 Item1;
+        public T2 Item2;
+        public string Property { get; set; }
+    }
+}";
+            var comp = CreateCompilation(source, targetFramework: TargetFramework.Mscorlib40, assemblyName: "customValueTuple");
+            comp.VerifyDiagnostics();
+            var verifier = CompileAndVerify(comp, symbolValidator: verifyModule, sourceSymbolValidator: verifyModule);
+
+            static void verifyModule(ModuleSymbol module)
+            {
+                var namedType = (NamedTypeSymbol)module.GlobalNamespace.GetMember<NamespaceSymbol>("System").GetMembers("ValueTuple").Single();
+                var isSourceSymbol = namedType.ContainingModule is SourceModuleSymbol;
+
+                Assert.Equal("(T1, T2)", namedType.ToTestDisplayString());
+                var fields = namedType.GetMembers().OfType<FieldSymbol>();
+                AssertEx.SetEqual(new[] { "T1 (T1, T2).Item1", "T2 (T1, T2).Item2", "System.String (T1, T2).<Property>k__BackingField" }, fields.ToTestDisplayStrings());
+                var backingField = namedType.GetField("<Property>k__BackingField");
+                if (isSourceSymbol)
+                {
+                    Assert.IsType<SynthesizedBackingFieldSymbol>(backingField);
+                    Assert.Equal("System.String (T1, T2).Property { get; set; }", backingField.AssociatedSymbol.ToTestDisplayString());
+                }
+                else
+                {
+                    Assert.IsType<PEFieldSymbol>(backingField);
+                    Assert.Null(backingField.AssociatedSymbol);
+                }
+            }
+        }
+
         [Fact]
         public void SwitchWithNamedTuple()
         {