Merge pull request #13107 from VSadov/tupleDataFlow

Implement definite assignment analysis for tuples

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

@@ -664,6 +664,8 @@ private void NoteWrite(BoundExpression n, BoundExpression value, bool read)
         /// </summary>
         protected int VariableSlot(Symbol symbol, int containingSlot = 0)
         {
+            containingSlot = DescendThroughTupleRestFields(ref symbol, containingSlot, forceContainingSlotsToExist: false);
+
             int slot;
             return (_variableSlot.TryGetValue(new VariableIdentifier(symbol, containingSlot), out slot)) ? slot : -1;
         }
@@ -674,6 +676,9 @@ protected int VariableSlot(Symbol symbol, int containingSlot = 0)
         protected int GetOrCreateSlot(Symbol symbol, int containingSlot = 0)
         {
             if (symbol is RangeVariableSymbol) return -1;
+
+            containingSlot = DescendThroughTupleRestFields(ref symbol, containingSlot, forceContainingSlotsToExist: true);
+
             VariableIdentifier identifier = new VariableIdentifier(symbol, containingSlot);
             int slot;
 
@@ -700,6 +705,54 @@ protected int GetOrCreateSlot(Symbol symbol, int containingSlot = 0)
             return slot;
         }
 
+        /// <summary>
+        /// Descends through Rest fields of a tuple if "symbol" is an extended field
+        /// As a result the "symbol" will be adjusted to be the field of the innermost tuple
+        /// and a corresponding containingSlot is returned.
+        /// Return value -1 indicates a failure which could happen for the following reasons
+        /// a) Rest field does not exist, which could happen in rare error scenarios involving broken ValueTuple types
+        /// b) Rest is not tracked already and forceSlotsToExist is false (otherwise we create slots on demand)
+        /// </summary>
+        private int DescendThroughTupleRestFields(ref Symbol symbol, int containingSlot, bool forceContainingSlotsToExist)
+        {
+            var fieldSymbol = symbol as TupleFieldSymbol;
+            if ((object)fieldSymbol != null)
+            {
+                TypeSymbol containingType = ((TupleTypeSymbol)symbol.ContainingType).UnderlyingNamedType;
+
+                // for tuple fields the variable identifier represents the underlying field
+                symbol = fieldSymbol.TupleUnderlyingField;
+
+                // descend through Rest fields
+                // force corresponding slots if do not exist
+                while (containingType != symbol.ContainingType)
+                {
+                    var restField = containingType.GetMembers(TupleTypeSymbol.RestFieldName).FirstOrDefault() as FieldSymbol;
+                    if ((object)restField == null)
+                    {
+                        return -1;
+                    }
+
+                    if (forceContainingSlotsToExist)
+                    {
+                        containingSlot = GetOrCreateSlot(restField, containingSlot);
+                    }
+                    else
+                    {
+                        if (!_variableSlot.TryGetValue(new VariableIdentifier(restField, containingSlot), out containingSlot))
+                        {
+                            return -1;
+                        }
+                    }
+
+                    containingType = restField.Type.TupleUnderlyingTypeOrSelf();
+                }
+            }
+
+            return containingSlot;
+        }
+
+
         private void Normalize(ref LocalState state)
         {
             int oldNext = state.Assigned.Capacity;

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

@@ -118,8 +118,8 @@ private bool CheckStructInstanceFields(ConsList<NamedTypeSymbol> typesWithMember
 
                 if ((object)field != null)
                 {
-                    var actualFiledType = field.Type;
-                    if (!IsEmptyStructType(actualFiledType, typesWithMembersOfThisType))
+                    var actualFieldType = field.Type;
+                    if (!IsEmptyStructType(actualFieldType, typesWithMembersOfThisType))
                     {
                         return false;
                     }
@@ -167,6 +167,15 @@ private FieldSymbol GetActualInstanceField(Symbol member, NamedTypeSymbol type)
                 {
                     case SymbolKind.Field:
                         var field = (FieldSymbol)member;
+
+                        // Do not report virtual tuple fields.
+                        // They are additional aliases to the fields of the underlying struct or nested extensions.
+                        // and as such are already accounted for via the nonvirtual fields.
+                        if (field.IsVirtualTupleField)
+                        {
+                            return null;
+                        }
+
                         return (field.IsFixed || ShouldIgnoreStructField(field, field.Type)) ? null : field.AsMember(type);
 
                     case SymbolKind.Event:

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

@@ -384,6 +384,17 @@ public virtual bool IsTupleField
             }
         }
 
+        /// <summary>
+        /// Returns True when field symbol is not mapped directly to a field in the underlying tuple struct.
+        /// </summary>
+        internal virtual bool IsVirtualTupleField
+        {
+            get
+            {
+                return false;
+            }
+        }
+
         /// <summary>
         /// If this is a field of a tuple type, return corresponding underlying field from the
         /// tuple underlying type. Otherwise, null. In case of a malformed underlying type

src/Compilers/CSharp/Portable/Symbols/Tuples/TupleFieldSymbol.cs

@@ -15,7 +15,7 @@ internal class TupleFieldSymbol : WrappedFieldSymbol
         protected readonly TupleTypeSymbol _containingTuple;
 
         /// <summary>
-        /// If this field represents a tuple element (including the name match), 
+        /// If this field represents a tuple element, 
         /// id is an index of the element (zero-based).
         /// Otherwise, (-1 - [index in members array]);
         /// </summary>
@@ -24,6 +24,9 @@ internal class TupleFieldSymbol : WrappedFieldSymbol
         public TupleFieldSymbol(TupleTypeSymbol container, FieldSymbol underlyingField, int tupleFieldId)
             : base(underlyingField)
         {
+            Debug.Assert(container.UnderlyingNamedType.Equals(underlyingField.ContainingType, TypeCompareKind.IgnoreDynamicAndTupleNames) || this is TupleVirtualElementFieldSymbol,
+                                            "virtual fields should be represented by " + nameof(TupleVirtualElementFieldSymbol));
+
             _containingTuple = container;
             _tupleFieldId = tupleFieldId;
         }
@@ -37,7 +40,7 @@ public override bool IsTupleField
         }
 
         /// <summary>
-        /// If this field represents a tuple element (including the name match), 
+        /// If this field represents a tuple element, 
         /// id is an index of the element (zero-based).
         /// Otherwise, (-1 - [index in members array]);
         /// </summary>i
@@ -100,22 +103,27 @@ internal override DiagnosticInfo GetUseSiteDiagnostic()
 
         public override sealed int GetHashCode()
         {
-            return Hash.Combine(_containingTuple.GetHashCode(), _tupleFieldId.GetHashCode());
+            return Hash.Combine(
+                Hash.Combine(_containingTuple.GetHashCode(), _tupleFieldId.GetHashCode()),
+                this.Name.GetHashCode());
         }
 
         public override sealed bool Equals(object obj)
         {
-            return Equals(obj as TupleFieldSymbol);
-        }
+            var other = obj as TupleFieldSymbol;
 
-        public bool Equals(TupleFieldSymbol other)
-        {
             if ((object)other == this)
             {
                 return true;
             }
 
-            return (object)other != null && _tupleFieldId == other._tupleFieldId && _containingTuple == other._containingTuple;
+            // note we have to compare both index and name because 
+            // in nameless tuple there could be fields that differ only by index
+            // and in named tupoles there could be fields that differ only by name
+            return (object)other != null && 
+                _tupleFieldId == other.TupleElementIndex && 
+                _containingTuple == other._containingTuple &&
+                this.Name == other.Name;
         }
     }
 
@@ -185,18 +193,31 @@ public override Symbol AssociatedSymbol
     }
 
     /// <summary>
-    /// Represents an element field of a tuple type (such as (int a, byte b).a, or (int a, byte b).b)
-    /// that is backed by a real field with a different name within the tuple underlying type.
+    /// Represents an element field of a tuple type that is not backed by a real field 
+    /// with the same name within the tuple underlying type.
+    /// 
+    /// Examples
+    ///     // alias to Item1 with a different name
+    ///     (int a, byte b).a                           
+    ///
+    ///     // not backed directly by the underlying type
+    ///     (int i1, int i2, int i3, int i4, int i5, int i6, int i7, int i8).i8
+    ///     
+    /// NOTE: For any virtual element, there is a nonvirtual way to access the same underlying field.
+    ///       In scenarios where we need to enumerate actual fields of a struct, 
+    ///       virtual fields should be ignored.
     /// </summary>
-    internal sealed class TupleRenamedElementFieldSymbol : TupleElementFieldSymbol
+    internal sealed class TupleVirtualElementFieldSymbol : TupleElementFieldSymbol
     {
         private readonly string _name;
 
-        public TupleRenamedElementFieldSymbol(TupleTypeSymbol container, FieldSymbol underlyingField, string name, int tupleElementOrdinal, Location location)
+        public TupleVirtualElementFieldSymbol(TupleTypeSymbol container, FieldSymbol underlyingField, string name, int tupleElementOrdinal, Location location)
             : base(container, underlyingField, tupleElementOrdinal, location)
         {
             Debug.Assert(name != null);
-            Debug.Assert(name != underlyingField.Name);
+            Debug.Assert(name != underlyingField.Name || !container.UnderlyingNamedType.Equals(underlyingField.ContainingType, TypeCompareKind.IgnoreDynamicAndTupleNames),
+                                "fields that map directly to underlying should not be represented by " + nameof(TupleVirtualElementFieldSymbol));
+
             _name = name;
         }
 
@@ -223,5 +244,13 @@ public override Symbol AssociatedSymbol
                 return null;
             }
         }
+
+        internal override bool IsVirtualTupleField
+        {
+            get
+            {
+                return true;
+            }
+        }
     }
 }

src/Compilers/CSharp/Portable/Symbols/Tuples/TupleTypeSymbol.cs

@@ -44,6 +44,7 @@ internal sealed class TupleTypeSymbol : WrappedNamedTypeSymbol
 
         internal const int RestPosition = 8; // The Rest field is in 8th position
         internal const string TupleTypeName = "ValueTuple";
+        internal const string RestFieldName = "Rest";
 
         private TupleTypeSymbol(Location locationOpt, NamedTypeSymbol underlyingType, ImmutableArray<Location> elementLocations, ImmutableArray<string> elementNames, ImmutableArray<TypeSymbol> elementTypes)
             : this(locationOpt == null ? ImmutableArray<Location>.Empty : ImmutableArray.Create(locationOpt),
@@ -555,6 +556,15 @@ internal static int IsElementNameReserved(string name)
                 return 0;
             }
 
+            return MatchesCanonicalElementName(name);
+        }
+
+        /// <summary>
+        /// Returns 3 for "Item3".
+        /// Returns -1 otherwise.
+        /// </summary>
+        private static int MatchesCanonicalElementName(string name)
+        {
             if (name.StartsWith("Item", StringComparison.Ordinal))
             {
                 string tail = name.Substring(4);
@@ -723,13 +733,13 @@ private ImmutableArray<FieldSymbol> CollectTupleElementFields()
                     continue;
                 }
 
-                int index = (member as TupleFieldSymbol)?.TupleElementIndex ??
-                            ((TupleErrorFieldSymbol)member).TupleElementIndex;
+                var field = (FieldSymbol)member;
+                var index = field.TupleElementIndex;
 
-                if (index >= 0)
+                if (index >= 0 && index == MatchesCanonicalElementName(field.Name) - 1)
                 {
                     Debug.Assert((object)builder[index] == null);
-                    builder[index] = (FieldSymbol)member;
+                    builder[index] = field;
                 }
             }
 
@@ -813,21 +823,15 @@ private ImmutableArray<Symbol> CreateMembers()
                                     if (namesOfVirtualFields[tupleFieldIndex] != defaultName)
                                     {
                                         // The name given doesn't match default name Item8, etc.
-                                        members.Add(new TupleRenamedElementFieldSymbol(this, fieldSymbol, defaultName, -members.Count - 1, null));
+                                        // Add a field with default name and make it virtual since we are not at the top level
+                                        members.Add(new TupleVirtualElementFieldSymbol(this, fieldSymbol, defaultName, tupleFieldIndex, null));
                                     }
 
                                     // Add a field with the given name
                                     var location = _elementLocations.IsDefault ? null : _elementLocations[tupleFieldIndex];
+                                    members.Add(new TupleVirtualElementFieldSymbol(this, fieldSymbol, namesOfVirtualFields[tupleFieldIndex],
+                                                        tupleFieldIndex, location));
 
-                                    if (field.Name == namesOfVirtualFields[tupleFieldIndex])
-                                    {
-                                        members.Add(new TupleElementFieldSymbol(this, fieldSymbol, tupleFieldIndex, location));
-                                    }
-                                    else
-                                    {
-                                        members.Add(new TupleRenamedElementFieldSymbol(this, fieldSymbol, namesOfVirtualFields[tupleFieldIndex],
-                                                                                                tupleFieldIndex, location));
-                                    }
                                 }
                                 else if (field.Name == namesOfVirtualFields[tupleFieldIndex])
                                 {
@@ -837,12 +841,12 @@ private ImmutableArray<Symbol> CreateMembers()
                                 else
                                 {
                                     // Add a field with default name
-                                    members.Add(new TupleFieldSymbol(this, fieldSymbol, -members.Count - 1));
+                                    members.Add(new TupleFieldSymbol(this, fieldSymbol, tupleFieldIndex));
 
                                     // Add a field with the given name
                                     if (namesOfVirtualFields[tupleFieldIndex] != null)
                                     {
-                                        members.Add(new TupleRenamedElementFieldSymbol(this, fieldSymbol, namesOfVirtualFields[tupleFieldIndex], tupleFieldIndex,
+                                        members.Add(new TupleVirtualElementFieldSymbol(this, fieldSymbol, namesOfVirtualFields[tupleFieldIndex], tupleFieldIndex,
                                                                                                 _elementLocations.IsDefault ? null : _elementLocations[tupleFieldIndex]));
                                     }
                                 }