diff --git a/src/Compilers/CSharp/Portable/Lowering/BoundTreeToDifferentEnclosingContextRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/BoundTreeToDifferentEnclosingContextRewriter.cs index f4874b9a49159..2496aa0422e52 100644 --- a/src/Compilers/CSharp/Portable/Lowering/BoundTreeToDifferentEnclosingContextRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/BoundTreeToDifferentEnclosingContextRewriter.cs @@ -23,6 +23,9 @@ internal abstract class BoundTreeToDifferentEnclosingContextRewriter : BoundTree // though its containing method is not correct because the code is moved into another method) private readonly Dictionary localMap = new Dictionary(); + //to handle type changes (e.g. type parameters) we need to update placeholders + private readonly Dictionary _placeholderMap = new Dictionary(); + // A mapping for types in the original method to types in its replacement. This is mainly necessary // when the original method was generic, as type parameters in the original method are mapping into // type parameters of the resulting class. @@ -114,6 +117,31 @@ protected BoundBlock VisitBlock(BoundBlock node, bool removeInstrumentation) return TypeMap.SubstituteType(type).Type; } + public override BoundNode VisitAwaitableInfo(BoundAwaitableInfo node) + { + var awaitablePlaceholder = node.AwaitableInstancePlaceholder; + if (awaitablePlaceholder is null) + { + return node; + } + + var rewrittenPlaceholder = awaitablePlaceholder.Update(VisitType(awaitablePlaceholder.Type)); + _placeholderMap.Add(awaitablePlaceholder, rewrittenPlaceholder); + + var getAwaiter = (BoundExpression?)this.Visit(node.GetAwaiter); + var isCompleted = VisitPropertySymbol(node.IsCompleted); + var getResult = VisitMethodSymbol(node.GetResult); + + _placeholderMap.Remove(awaitablePlaceholder); + + return node.Update(rewrittenPlaceholder, node.IsDynamic, getAwaiter, isCompleted, getResult); + } + + public override BoundNode VisitAwaitableValuePlaceholder(BoundAwaitableValuePlaceholder node) + { + return _placeholderMap[node]; + } + protected override BoundBinaryOperator.UncommonData? VisitBinaryOperatorData(BoundBinaryOperator node) { // Local rewriter should have already rewritten interpolated strings into their final form of calls and gotos @@ -142,6 +170,24 @@ protected BoundBlock VisitBlock(BoundBlock node, bool removeInstrumentation) VisitType(node.Type)); } + [return: NotNullIfNotNull(nameof(property))] + public override PropertySymbol? VisitPropertySymbol(PropertySymbol? property) + { + if (property is null) + { + return null; + } + if (property.ContainingType.IsAnonymousType) + { + //at this point we expect that the code is lowered and that getters of anonymous types are accessed + //only via their corresponding get-methods (see VisitMethodSymbol) + throw ExceptionUtilities.Unreachable(); + } + return ((PropertySymbol)property.OriginalDefinition) + .AsMember((NamedTypeSymbol)TypeMap.SubstituteType(property.ContainingType).AsTypeSymbolOnly()) + ; + } + [return: NotNullIfNotNull(nameof(method))] public override MethodSymbol? VisitMethodSymbol(MethodSymbol? method) { diff --git a/src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs index a870c8678eb7f..85e48d715dd47 100644 --- a/src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs @@ -35,8 +35,6 @@ internal abstract partial class MethodToClassRewriter : BoundTreeToDifferentEncl protected readonly BindingDiagnosticBag Diagnostics; protected readonly VariableSlotAllocator? slotAllocator; - private readonly Dictionary _placeholderMap; - protected MethodToClassRewriter(VariableSlotAllocator? slotAllocator, TypeCompilationState compilationState, BindingDiagnosticBag diagnostics) { Debug.Assert(compilationState != null); @@ -46,7 +44,6 @@ protected MethodToClassRewriter(VariableSlotAllocator? slotAllocator, TypeCompil this.CompilationState = compilationState; this.Diagnostics = diagnostics; this.slotAllocator = slotAllocator; - this._placeholderMap = new Dictionary(); } /// @@ -254,31 +251,6 @@ private bool TryGetHoistedField(Symbol variable, [NotNullWhen(true)] out FieldSy return false; } - public override BoundNode VisitAwaitableInfo(BoundAwaitableInfo node) - { - var awaitablePlaceholder = node.AwaitableInstancePlaceholder; - if (awaitablePlaceholder is null) - { - return node; - } - - var rewrittenPlaceholder = awaitablePlaceholder.Update(VisitType(awaitablePlaceholder.Type)); - _placeholderMap.Add(awaitablePlaceholder, rewrittenPlaceholder); - - var getAwaiter = (BoundExpression?)this.Visit(node.GetAwaiter); - var isCompleted = VisitPropertySymbol(node.IsCompleted); - var getResult = VisitMethodSymbol(node.GetResult); - - _placeholderMap.Remove(awaitablePlaceholder); - - return node.Update(rewrittenPlaceholder, node.IsDynamic, getAwaiter, isCompleted, getResult); - } - - public override BoundNode VisitAwaitableValuePlaceholder(BoundAwaitableValuePlaceholder node) - { - return _placeholderMap[node]; - } - public override BoundNode VisitAssignmentOperator(BoundAssignmentOperator node) { BoundExpression originalLeft = node.Left; diff --git a/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/MethodToStateMachineRewriter.cs b/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/MethodToStateMachineRewriter.cs index 4221e9de09565..3671e459416df 100644 --- a/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/MethodToStateMachineRewriter.cs +++ b/src/Compilers/CSharp/Portable/Lowering/StateMachineRewriter/MethodToStateMachineRewriter.cs @@ -505,11 +505,18 @@ private void FreeReusableHoistedField(StateMachineFieldSymbol field) fields.Add(field); } - private BoundExpression HoistRefInitialization(SynthesizedLocal local, BoundAssignmentOperator node) + private BoundExpression HoistRefInitialization(LocalSymbol local, BoundAssignmentOperator node) { + Debug.Assert( + local switch + { + TypeSubstitutedLocalSymbol tsl => tsl.UnderlyingLocalSymbol, + _ => local + } is SynthesizedLocal + ); Debug.Assert(local.SynthesizedKind == SynthesizedLocalKind.Spill || (local.SynthesizedKind == SynthesizedLocalKind.ForEachArray && local.Type.HasInlineArrayAttribute(out _) && local.Type.TryGetInlineArrayElementField() is object)); - Debug.Assert(local.SyntaxOpt != null); + Debug.Assert(local.GetDeclaratorSyntax() != null); #pragma warning disable format Debug.Assert(local.SynthesizedKind switch { @@ -856,7 +863,7 @@ public override BoundNode VisitAssignmentOperator(BoundAssignmentOperator node) // We have an assignment to a variable that has not yet been assigned a proxy. // So we assign the proxy before translating the assignment. - return HoistRefInitialization((SynthesizedLocal)leftLocal, node); + return HoistRefInitialization(leftLocal, node); } /// diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/TypeSubstitutedLocalSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/TypeSubstitutedLocalSymbol.cs index 7de14256bbd2c..0e5cceb260337 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Synthesized/TypeSubstitutedLocalSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Synthesized/TypeSubstitutedLocalSymbol.cs @@ -24,7 +24,15 @@ public TypeSubstitutedLocalSymbol(LocalSymbol originalVariable, TypeWithAnnotati Debug.Assert(containingSymbol != null); Debug.Assert(containingSymbol.DeclaringCompilation is not null); - _originalVariable = originalVariable; + //avoid double wrapping + //this can happen e.g. if a local is substituted in ExtensionMethodBodyRewriter + //and later it is substituted for example in ClosureConversion again + _originalVariable = originalVariable switch + { + TypeSubstitutedLocalSymbol tsl => tsl._originalVariable, + var l => l + }; + _type = type; _containingSymbol = containingSymbol; } @@ -123,6 +131,8 @@ internal override ReadOnlyBindingDiagnostic GetConstantValueDiag return _originalVariable.GetConstantValueDiagnostics(boundInitValue); } + internal LocalSymbol UnderlyingLocalSymbol => _originalVariable; + internal override LocalSymbol WithSynthesizedLocalKindAndSyntax( SynthesizedLocalKind kind, SyntaxNode syntax #if DEBUG diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/AwaitExpressionTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/AwaitExpressionTests.cs index c47850beca419..1ff6927f6275c 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/AwaitExpressionTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/AwaitExpressionTests.cs @@ -119,6 +119,122 @@ async void Goo(Task t) Assert.Equal("System.Boolean System.Runtime.CompilerServices.TaskAwaiter.IsCompleted { get; }", info.IsCompletedProperty.ToTestDisplayString()); } + [Fact] + [WorkItem("https://github.com/dotnet/roslyn/issues/76999")] + public void TestAwaitHoistedRef() + { + var src = """ + using System.Threading.Tasks; + + public sealed class RefHolder + { + private T _t; + public ref T Get() => ref _t; + } + + public static class App + { + public static void Do() + { + var res = new RefHolder(); + M().Wait(); + async Task M() + { + res.Get() = await Task.FromResult(default(T)); + } + } + } + """; + + var comp = CreateCompilation(src); + comp.VerifyEmitDiagnostics( + // (17,13): error CS8178: A reference returned by a call to 'RefHolder.Get()' cannot be preserved across 'await' or 'yield' boundary. + // res.Get() = await Task.FromResult(default(T)); + Diagnostic(ErrorCode.ERR_RefReturningCallAndAwait, "res.Get()").WithArguments("RefHolder.Get()").WithLocation(17, 13) + ); + } + + [Fact] + [WorkItem("https://github.com/dotnet/roslyn/issues/76999")] + public void TestAwaitHoistedRef_InNewExtensionContainer() + { + var src = """ + using System.Threading.Tasks; + + public sealed class RefHolder + { + private T _t; + public ref T Get() => ref _t; + } + + public static class App + { + extension(int) + { + public static void Do() + { + var res = new RefHolder(); + M().Wait(); + async Task M() + { + res.Get() = await Task.FromResult(default(T)); + } + } + } + } + """; + + var comp = CreateCompilation(src); + comp.VerifyEmitDiagnostics( + // (19,17): error CS8178: A reference returned by a call to 'RefHolder.Get()' cannot be preserved across 'await' or 'yield' boundary. + // res.Get() = await Task.FromResult(default(T)); + Diagnostic(ErrorCode.ERR_RefReturningCallAndAwait, "res.Get()").WithArguments("RefHolder.Get()").WithLocation(19, 17) + ); + } + + [Fact] + [WorkItem("https://github.com/dotnet/roslyn/issues/76999")] + public void TestAwaitHoistedRef2() + { + var src = """ + using System; + using System.Collections.Generic; + using System.Linq; + using System.Threading.Tasks; + + public sealed class ValuesHolder + { + private readonly T[] _values = new T[10]; + + public ref T this[int type] => ref _values[type]; + } + + public static class App + { + public static async Task> Do() + { + var res = new ValuesHolder(); + + var taskGroup = new List>>(); + + await Task.WhenAll(taskGroup.Select(async kv => + { + res[0] = await kv.Value; + })); + + return res; + } + } + """; + + var comp = CreateCompilation(src); + comp.VerifyEmitDiagnostics( + // (23,4): error CS8178: A reference returned by a call to 'ValuesHolder.this[int].get' cannot be preserved across 'await' or 'yield' boundary. + // res[0] = await kv.Value; + Diagnostic(ErrorCode.ERR_RefReturningCallAndAwait, "res[0]").WithArguments("ValuesHolder.this[int].get").WithLocation(23, 4) + ); + } + [Fact] [WorkItem(1084696, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/1084696")] public void TestAwaitInfo2()