Avoid race condition in map for TupleElementIndex

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

@@ -4,11 +4,13 @@
 
 #nullable disable
 
+using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Runtime.InteropServices;
 using Microsoft.CodeAnalysis.CSharp.Emit;
+using Microsoft.CodeAnalysis.RuntimeMembers;
 using Microsoft.CodeAnalysis.Symbols;
 using Roslyn.Utilities;
 
@@ -464,15 +466,34 @@ public virtual int TupleElementIndex
             get
             {
                 // wrapped tuple fields already have this information and override this property
-                Debug.Assert(!(this is TupleElementFieldSymbol or TupleErrorFieldSymbol));
+                Debug.Assert(!(this is TupleElementFieldSymbol or TupleErrorFieldSymbol or Retargeting.RetargetingFieldSymbol));
+                if (!ContainingType.IsTupleType)
+                {
+                    return -1;
+                }
 
-                var map = ContainingType.TupleFieldDefinitionsToIndexMap;
-                if (map is object && map.TryGetValue(this.OriginalDefinition, out int index))
+                if (!ContainingType.IsDefinition)
                 {
-                    return index;
+                    return this.OriginalDefinition.TupleElementIndex;
                 }
 
-                return -1;
+                var tupleElementPosition = NamedTypeSymbol.MatchesCanonicalTupleElementName(Name);
+                int arity = ContainingType.Arity;
+                if (tupleElementPosition <= 0 || tupleElementPosition > arity)
+                {
+                    // ex: no "Item2" in 'ValueTuple<T1>'
+                    return -1;
+                }
+                Debug.Assert(tupleElementPosition < NamedTypeSymbol.ValueTupleRestPosition);
+
+                WellKnownMember wellKnownMember = NamedTypeSymbol.GetTupleTypeMember(arity, tupleElementPosition);
+                MemberDescriptor descriptor = WellKnownMembers.GetDescriptor(wellKnownMember);
+                Symbol found = CSharpCompilation.GetRuntimeMember(ImmutableArray.Create<Symbol>(this), descriptor, CSharpCompilation.SpecialMembersSignatureComparer.Instance,
+                    accessWithinOpt: null); // force lookup of public members only
+
+                return found is not null
+                    ? tupleElementPosition - 1
+                    : -1;
             }
         }
 

src/Compilers/CSharp/Portable/Symbols/Retargeting/RetargetingNamedTypeSymbol.cs

@@ -135,20 +135,6 @@ public override ImmutableArray<Symbol> GetMembers(string name)
             return this.RetargetingTranslator.Retarget(_underlyingType.GetMembers(name));
         }
 
-        public override void InitializeTupleFieldDefinitionsToIndexMap()
-        {
-            Debug.Assert(this.IsTupleType);
-            Debug.Assert(this.IsDefinition); // we only store a map for definitions
-
-            var retargetedMap = new SmallDictionary<FieldSymbol, int>(ReferenceEqualityComparer.Instance);
-            foreach ((FieldSymbol field, int index) in _underlyingType.TupleFieldDefinitionsToIndexMap)
-            {
-                retargetedMap.Add(this.RetargetingTranslator.Retarget(field), index);
-            }
-
-            this.TupleData!.SetFieldDefinitionsToIndexMap(retargetedMap);
-        }
-
         internal override IEnumerable<FieldSymbol> GetFieldsToEmit()
         {
             foreach (FieldSymbol f in _underlyingType.GetFieldsToEmit())

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

@@ -447,7 +447,7 @@ internal static int IsTupleElementNameReserved(string name)
                 return 0;
             }
 
-            return matchesCanonicalElementName(name);
+            return MatchesCanonicalTupleElementName(name);
 
             static bool isElementNameForbidden(string name)
             {
@@ -465,25 +465,25 @@ static bool isElementNameForbidden(string name)
                         return false;
                 }
             }
+        }
 
-            // Returns 3 for "Item3".
-            // Returns -1 otherwise.
-            static int matchesCanonicalElementName(string name)
+        // Returns 3 for "Item3".
+        // Returns -1 otherwise.
+        internal static int MatchesCanonicalTupleElementName(string name)
+        {
+            if (name.StartsWith("Item", StringComparison.Ordinal))
             {
-                if (name.StartsWith("Item", StringComparison.Ordinal))
+                string tail = name.Substring("Item".Length);
+                if (int.TryParse(tail, out int number))
                 {
-                    string tail = name.Substring(4);
-                    if (int.TryParse(tail, out int number))
+                    if (number > 0 && string.Equals(name, TupleMemberName(number), StringComparison.Ordinal))
                     {
-                        if (number > 0 && string.Equals(name, TupleMemberName(number), StringComparison.Ordinal))
-                        {
-                            return number;
-                        }
+                        return number;
                     }
                 }
-
-                return -1;
             }
+
+            return -1;
         }
 
         /// <summary>
@@ -567,36 +567,6 @@ public sealed override ImmutableArray<TypeWithAnnotations> TupleElementTypesWith
         public sealed override ImmutableArray<FieldSymbol> TupleElements
             => IsTupleType ? TupleData!.TupleElements(this) : default;
 
-        /// <summary>
-        /// For tuple fields that aren't TupleElementFieldSymbol or TupleErrorFieldSymbol, we cache their tuple element index.
-        /// This supports <see cref="FieldSymbol.TupleElementIndex"/>.
-        /// For those fields, we map from their definition to an index.
-        /// </summary>
-        public SmallDictionary<FieldSymbol, int>? TupleFieldDefinitionsToIndexMap
-        {
-            get
-            {
-                if (!IsTupleType)
-                {
-                    return null;
-                }
-
-                if (!IsDefinition)
-                {
-                    return this.OriginalDefinition.TupleFieldDefinitionsToIndexMap;
-                }
-
-                return TupleData!.GetFieldDefinitionsToIndexMap(this);
-            }
-        }
-
-        public virtual void InitializeTupleFieldDefinitionsToIndexMap()
-        {
-            Debug.Assert(this.IsTupleType);
-            Debug.Assert(this.IsDefinition); // we only store a map for definitions
-            _ = this.GetMembers();
-        }
-
         public TMember? GetTupleMemberSymbolForUnderlyingMember<TMember>(TMember? underlyingMemberOpt) where TMember : Symbol
         {
             return IsTupleType ? TupleData!.GetTupleMemberSymbolForUnderlyingMember(underlyingMemberOpt) : null;
@@ -611,10 +581,6 @@ protected ArrayBuilder<Symbol> MakeSynthesizedTupleMembers(ImmutableArray<Symbol
             var elementsMatchedByFields = ArrayBuilder<bool>.GetInstance(elementTypes.Length, fillWithValue: false);
             var members = ArrayBuilder<Symbol>.GetInstance(currentMembers.Length);
 
-            // For tuple fields that aren't TupleElementFieldSymbol or TupleErrorFieldSymbol, we cache/map their tuple element index
-            // corresponding to their definition. We only need to do that for the definition of ValueTuple types.
-            var fieldDefinitionsToIndexMap = IsDefinition ? new SmallDictionary<FieldSymbol, int>(ReferenceEqualityComparer.Instance) : null;
-
             NamedTypeSymbol currentValueTuple = this;
             int currentNestingLevel = 0;
 
@@ -692,7 +658,6 @@ protected ArrayBuilder<Symbol> MakeSynthesizedTupleMembers(ImmutableArray<Symbol
                                     if (IsDefinition)
                                     {
                                         defaultTupleField = field;
-                                        fieldDefinitionsToIndexMap!.Add(field, tupleFieldIndex);
                                     }
                                     else
                                     {
@@ -819,10 +784,6 @@ protected ArrayBuilder<Symbol> MakeSynthesizedTupleMembers(ImmutableArray<Symbol
             }
 
             elementsMatchedByFields.Free();
-            if (fieldDefinitionsToIndexMap is object)
-            {
-                this.TupleData!.SetFieldDefinitionsToIndexMap(fieldDefinitionsToIndexMap);
-            }
             return members;
 
             // Returns the nested type at a certain depth.
@@ -847,11 +808,11 @@ static void collectTargetTupleFields(int arity, ImmutableArray<Symbol> members,
                 for (int i = 0; i < fieldsPerType; i++)
                 {
                     WellKnownMember wellKnownTupleField = GetTupleTypeMember(arity, i + 1);
-                    fieldsForElements.Add((FieldSymbol?)GetWellKnownMemberInType(members, wellKnownTupleField));
+                    fieldsForElements.Add((FieldSymbol?)getWellKnownMemberInType(members, wellKnownTupleField));
                 }
             }
 
-            static Symbol? GetWellKnownMemberInType(ImmutableArray<Symbol> members, WellKnownMember relativeMember)
+            static Symbol? getWellKnownMemberInType(ImmutableArray<Symbol> members, WellKnownMember relativeMember)
             {
                 Debug.Assert(relativeMember >= WellKnownMember.System_ValueTuple_T1__Item1 && relativeMember <= WellKnownMember.System_ValueTuple_TRest__ctor);
 
@@ -954,13 +915,6 @@ internal sealed class TupleExtraData
 
             private ImmutableArray<FieldSymbol> _lazyDefaultElementFields;
 
-            /// <summary>
-            /// For tuple fields that aren't TupleElementFieldSymbol or TupleErrorFieldSymbol, we cache their tuple element index.
-            /// This supports <see cref="FieldSymbol.TupleElementIndex"/>.
-            /// For those fields, we map from their definition to an index.
-            /// </summary>
-            private SmallDictionary<FieldSymbol, int>? _lazyFieldDefinitionsToIndexMap;
-
             private SmallDictionary<Symbol, Symbol>? _lazyUnderlyingDefinitionToMemberMap;
 
             /// <summary>
@@ -1093,26 +1047,6 @@ ImmutableArray<FieldSymbol> collectTupleElementFields(NamedTypeSymbol tuple)
                 }
             }
 
-            internal SmallDictionary<FieldSymbol, int> GetFieldDefinitionsToIndexMap(NamedTypeSymbol tuple)
-            {
-                Debug.Assert(tuple.IsTupleType);
-                Debug.Assert(tuple.IsDefinition); // we only store a map for definitions
-                if (_lazyFieldDefinitionsToIndexMap is null)
-                {
-                    tuple.InitializeTupleFieldDefinitionsToIndexMap();
-                }
-
-                Debug.Assert(_lazyFieldDefinitionsToIndexMap is object);
-                return _lazyFieldDefinitionsToIndexMap;
-            }
-
-            internal void SetFieldDefinitionsToIndexMap(SmallDictionary<FieldSymbol, int> map)
-            {
-                Debug.Assert(map.Keys.All(k => k.IsDefinition));
-                Debug.Assert(map.Values.All(v => v >= 0));
-                Interlocked.CompareExchange(ref _lazyFieldDefinitionsToIndexMap, map, null);
-            }
-
             internal SmallDictionary<Symbol, Symbol> UnderlyingDefinitionToMemberMap
             {
                 get

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

@@ -28562,41 +28562,33 @@ static void verify(FieldSymbol field)
         [Fact]
         public void TupleWithElementNamedWithDefaultName()
         {
-            // Instrumenting test as part of investigating flakiness: https://github.com/dotnet/roslyn/issues/52658
-            for (int i = 0; i < 1000; i++)
-            {
-                string source = @"
+            string source = @"
 class C
 {
     (int Item1, int Item2) M() => throw null;
 }
 ";
-                var comp = CreateCompilation(source);
-                var m = (MethodSymbol)comp.GetMember("C.M");
-                var tuple = m.ReturnType;
-                Assert.Equal("(System.Int32 Item1, System.Int32 Item2)", tuple.ToTestDisplayString());
-                Assert.IsType<ConstructedNamedTypeSymbol>(tuple);
-
-                var item1 = tuple.GetMember<TupleElementFieldSymbol>("Item1");
-                // Instrumenting test as part of investigating flakiness: https://github.com/dotnet/roslyn/issues/52658
-                RoslynDebug.AssertOrFailFast(0 == item1.TupleElementIndex);
-
-                var item1Underlying = item1.TupleUnderlyingField;
-                Assert.IsType<SubstitutedFieldSymbol>(item1Underlying);
-                Assert.Equal("System.Int32 (System.Int32 Item1, System.Int32 Item2).Item1", item1Underlying.ToTestDisplayString());
-                // Instrumenting test as part of investigating flakiness: https://github.com/dotnet/roslyn/issues/52658
-                RoslynDebug.AssertOrFailFast(0 == item1Underlying.TupleElementIndex);
-                Assert.Same(item1Underlying, item1Underlying.TupleUnderlyingField);
-
-                var item2 = tuple.GetMember<TupleElementFieldSymbol>("Item2");
-                // Instrumenting test as part of investigating flakiness: https://github.com/dotnet/roslyn/issues/52658
-                RoslynDebug.AssertOrFailFast(1 == item2.TupleElementIndex);
-                var item2Underlying = item2.TupleUnderlyingField;
-                Assert.IsType<SubstitutedFieldSymbol>(item2Underlying);
-                // Instrumenting test as part of investigating flakiness: https://github.com/dotnet/roslyn/issues/52658
-                RoslynDebug.AssertOrFailFast(1 == item2Underlying.TupleElementIndex);
-                Assert.Same(item2Underlying, item2Underlying.TupleUnderlyingField);
-            }
+            var comp = CreateCompilation(source);
+            var m = (MethodSymbol)comp.GetMember("C.M");
+            var tuple = m.ReturnType;
+            Assert.Equal("(System.Int32 Item1, System.Int32 Item2)", tuple.ToTestDisplayString());
+            Assert.IsType<ConstructedNamedTypeSymbol>(tuple);
+
+            var item1 = tuple.GetMember<TupleElementFieldSymbol>("Item1");
+            Assert.Equal(0, item1.TupleElementIndex);
+
+            var item1Underlying = item1.TupleUnderlyingField;
+            Assert.IsType<SubstitutedFieldSymbol>(item1Underlying);
+            Assert.Equal("System.Int32 (System.Int32 Item1, System.Int32 Item2).Item1", item1Underlying.ToTestDisplayString());
+            Assert.Equal(0, item1Underlying.TupleElementIndex);
+            Assert.Same(item1Underlying, item1Underlying.TupleUnderlyingField);
+
+            var item2 = tuple.GetMember<TupleElementFieldSymbol>("Item2");
+            Assert.Equal(1, item2.TupleElementIndex);
+            var item2Underlying = item2.TupleUnderlyingField;
+            Assert.IsType<SubstitutedFieldSymbol>(item2Underlying);
+            Assert.Equal(1, item2Underlying.TupleElementIndex);
+            Assert.Same(item2Underlying, item2Underlying.TupleUnderlyingField);
         }
 
         [Fact]
@@ -28615,6 +28607,19 @@ class C
                 tuple.ToTestDisplayString());
             Assert.IsType<ConstructedNamedTypeSymbol>(tuple);
 
+            var item1 = tuple.GetMember<TupleElementFieldSymbol>("Item1");
+            Assert.Equal("System.Int32 (System.Int32, System.Int32, System.Int32, System.Int32, System.Int32, System.Int32, System.Int32, System.Int32, System.Int32).Item1",
+                item1.ToTestDisplayString());
+            Assert.Equal(0, item1.TupleElementIndex);
+
+            var item1Underlying = item1.TupleUnderlyingField;
+            Assert.Equal("System.Int32 (System.Int32, System.Int32, System.Int32, System.Int32, System.Int32, System.Int32, System.Int32, System.Int32, System.Int32).Item1",
+                item1Underlying.ToTestDisplayString());
+            Assert.IsType<SubstitutedFieldSymbol>(item1Underlying);
+            Assert.Equal(-1, item1Underlying.TupleElementIndex); // should be zero, tracked by https://github.com/dotnet/roslyn/issues/58499
+
+            Assert.Same(tuple, item1Underlying.ContainingType);
+
             var item8 = tuple.GetMember<TupleElementFieldSymbol>("Item8");
             Assert.Equal("System.Int32 (System.Int32, System.Int32, System.Int32, System.Int32, System.Int32, System.Int32, System.Int32, System.Int32, System.Int32).Item8",
                 item8.ToTestDisplayString());