Overwrite placholder regions temporarily

Author: jjonescz

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

@@ -4480,7 +4480,7 @@ internal SafeContext GetValEscape(BoundExpression expr, SafeContext scopeOfTheCo
                     return scopeOfTheContainingExpression;
 
                 case BoundKind.InterpolatedStringHandlerPlaceholder:
-                    if (_placeholderScopes?.TryGetValue((BoundInterpolatedStringHandlerPlaceholder)expr, out var scope) == true)
+                    if (_placeholderScopes?.TryPeek((BoundInterpolatedStringHandlerPlaceholder)expr, out var scope) == true)
                     {
                         return scope;
                     }
@@ -5710,7 +5710,7 @@ private bool CheckInterpolatedStringHandlerInvocationArgMixingParts(BoundInterpo
             {
                 if (part is BoundCall { Method.Name: BoundInterpolatedString.AppendFormattedMethod } call)
                 {
-                    using var _ = new PlaceholderRegion(this, [((BoundInterpolatedStringHandlerPlaceholder)call.ReceiverOpt, escapeTo)]);
+                    using var _ = new PlaceholderRegion(this, [((BoundInterpolatedStringHandlerPlaceholder)call.ReceiverOpt, escapeTo)], overwriteExistingTemporarily: true);
                     result &= CheckInvocationArgMixing(
                         call.Syntax,
                         MethodInfo.Create(call.Method),

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

@@ -7,6 +7,7 @@
 using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Linq;
+using Microsoft.CodeAnalysis.Collections;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.PooledObjects;
 using Roslyn.Utilities;
@@ -61,7 +62,7 @@ private static bool InUnsafeMethod(Symbol symbol)
         private bool _inUnsafeRegion;
         private SafeContext _localScopeDepth;
         private Dictionary<LocalSymbol, (SafeContext RefEscapeScope, SafeContext ValEscapeScope)>? _localEscapeScopes;
-        private Dictionary<BoundValuePlaceholderBase, SafeContext>? _placeholderScopes;
+        private KeyedStack<BoundValuePlaceholderBase, SafeContext>? _placeholderScopes;
         private SafeContext _patternInputValEscape;
 #if DEBUG
         private const int MaxTrackVisited = 100; // Avoid tracking if too many expressions.
@@ -152,22 +153,24 @@ private ref struct PlaceholderRegion
         {
             private readonly RefSafetyAnalysis _analysis;
             private readonly ArrayBuilder<(BoundValuePlaceholderBase, SafeContext)> _placeholders;
+            private readonly bool _overwriteExistingTemporarily;
 
-            public PlaceholderRegion(RefSafetyAnalysis analysis, ArrayBuilder<(BoundValuePlaceholderBase, SafeContext)> placeholders)
+            public PlaceholderRegion(RefSafetyAnalysis analysis, ArrayBuilder<(BoundValuePlaceholderBase, SafeContext)> placeholders, bool overwriteExistingTemporarily = false)
             {
                 _analysis = analysis;
                 _placeholders = placeholders;
+                _overwriteExistingTemporarily = overwriteExistingTemporarily;
                 foreach (var (placeholder, valEscapeScope) in placeholders)
                 {
-                    _analysis.AddPlaceholderScope(placeholder, valEscapeScope);
+                    _analysis.AddPlaceholderScope(placeholder, valEscapeScope, canExist: overwriteExistingTemporarily);
                 }
             }
 
             public void Dispose()
             {
                 foreach (var (placeholder, _) in _placeholders)
                 {
-                    _analysis.RemovePlaceholderScope(placeholder);
+                    _analysis.RemovePlaceholderScope(placeholder, forcePop: _overwriteExistingTemporarily);
                 }
                 _placeholders.Free();
             }
@@ -189,33 +192,34 @@ private void SetLocalScopes(LocalSymbol local, SafeContext refEscapeScope, SafeC
             AddOrSetLocalScopes(local, refEscapeScope, valEscapeScope);
         }
 
-        private void AddPlaceholderScope(BoundValuePlaceholderBase placeholder, SafeContext valEscapeScope)
+        private void AddPlaceholderScope(BoundValuePlaceholderBase placeholder, SafeContext valEscapeScope, bool canExist = false)
         {
-            Debug.Assert(_placeholderScopes?.ContainsKey(placeholder) != true);
+            Debug.Assert(canExist || _placeholderScopes?.ContainsKey(placeholder) != true);
 
             // Consider not adding the placeholder to the dictionary if the escape scope is
             // CallingMethod, and simply fallback to that value in GetPlaceholderScope().
 
-            _placeholderScopes ??= new Dictionary<BoundValuePlaceholderBase, SafeContext>();
-            _placeholderScopes[placeholder] = valEscapeScope;
+            _placeholderScopes ??= new KeyedStack<BoundValuePlaceholderBase, SafeContext>();
+            _placeholderScopes.Push(placeholder, valEscapeScope);
         }
 
-#pragma warning disable IDE0060
-        private void RemovePlaceholderScope(BoundValuePlaceholderBase placeholder)
+        private void RemovePlaceholderScope(BoundValuePlaceholderBase placeholder, bool forcePop = false)
         {
             Debug.Assert(_placeholderScopes?.ContainsKey(placeholder) == true);
 
             // https://github.com/dotnet/roslyn/issues/65961: Currently, analysis may require subsequent calls
             // to GetRefEscape(), etc. for the same expression so we cannot remove placeholders eagerly.
-            //_placeholderScopes.Remove(placeholder);
+            if (forcePop)
+            {
+                _placeholderScopes?.TryPop(placeholder, out _);
+            }
         }
-#pragma warning restore IDE0060
 
         private SafeContext GetPlaceholderScope(BoundValuePlaceholderBase placeholder)
         {
             Debug.Assert(_placeholderScopes?.ContainsKey(placeholder) == true);
 
-            return _placeholderScopes?.TryGetValue(placeholder, out var scope) == true
+            return _placeholderScopes?.TryPeek(placeholder, out var scope) == true
                 ? scope
                 : SafeContext.CallingMethod;
         }

src/Compilers/Core/Portable/Collections/KeyedStack.cs

@@ -2,21 +2,21 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using System;
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
-using System.Linq;
-using System.Text;
-using System.Threading.Tasks;
-using Microsoft.CodeAnalysis.Text;
 
 namespace Microsoft.CodeAnalysis.Collections
 {
-    internal class KeyedStack<T, R>
+    internal sealed class KeyedStack<T, R>
         where T : notnull
     {
         private readonly Dictionary<T, Stack<R>> _dict = new Dictionary<T, Stack<R>>();
 
+        public bool ContainsKey(T key)
+        {
+            return _dict.ContainsKey(key);
+        }
+
         public void Push(T key, R value)
         {
             Stack<R>? store;
@@ -29,6 +29,18 @@ public void Push(T key, R value)
             store.Push(value);
         }
 
+        public bool TryPeek(T key, [MaybeNullWhen(returnValue: false)] out R value)
+        {
+            if (_dict.TryGetValue(key, out var stack) && stack.Count > 0)
+            {
+                value = stack.Peek();
+                return true;
+            }
+
+            value = default;
+            return false;
+        }
+
         public bool TryPop(T key, [MaybeNullWhen(returnValue: false)] out R value)
         {
             Stack<R>? store;