From 80aaae6bbc896debf6c584a8b37633e5962dc83c Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Thu, 21 Nov 2024 16:52:58 +0100 Subject: [PATCH 01/22] Avoid reusing temps whose refs might be captured --- .../CSharp/Portable/CodeGen/CodeGenerator.cs | 43 +- .../CodeGen/CodeGenerator_RefSafety.cs | 158 ++++ .../CSharp/Portable/CodeGen/EmitExpression.cs | 64 +- .../CSharp/Portable/CodeGen/EmitStatement.cs | 2 + .../LocalRewriter/LocalRewriter_Call.cs | 11 +- ...ocalRewriter_CompoundAssignmentOperator.cs | 2 +- .../Lowering/SyntheticBoundNodeFactory.cs | 2 +- .../Emit/CodeGen/CodeGenInParametersTests.cs | 19 +- .../Semantics/CollectionExpressionTests.cs | 46 +- .../Test/Emit3/Semantics/InlineArrayTests.cs | 103 ++- .../Semantic/Semantics/RefEscapingTests.cs | 869 ++++++++++++++++++ .../Symbol/Symbols/IndexedPropertyTests.cs | 142 +-- .../Core/Portable/CodeGen/LocalSlotManager.cs | 49 +- 13 files changed, 1326 insertions(+), 184 deletions(-) create mode 100644 src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs diff --git a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator.cs b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator.cs index ce7045499e411..bb3e6489064c4 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator.cs @@ -38,7 +38,10 @@ internal sealed partial class CodeGenerator // There are scenarios where rvalues need to be passed to ref/in parameters // in such cases the values must be spilled into temps and retained for the entirety of // the most encompassing expression. + // If a ref to the temp could escape, it is retained for the most encompassing block. private ArrayBuilder _expressionTemps; + private ArrayBuilder _blockTemps; + private bool _tempRefsMightEscape; // not 0 when in a protected region with a handler. private int _tryNestingLevel; @@ -504,35 +507,47 @@ private TextSpan EmitSequencePoint(SyntaxTree syntaxTree, TextSpan span) private void AddExpressionTemp(LocalDefinition temp) { - // in some cases like stack locals, there is no slot allocated. - if (temp == null) + if (_tempRefsMightEscape) { - return; + AddBlockTemp(temp); } + else + { + AddTemp(ref _expressionTemps, temp); + } + } + + private void ReleaseExpressionTemps() => ReleaseTemps(_expressionTemps); - ArrayBuilder exprTemps = _expressionTemps; - if (exprTemps == null) + private void AddBlockTemp(LocalDefinition temp) => AddTemp(ref _blockTemps, temp); + + private void ReleaseBlockTemps() => ReleaseTemps(_blockTemps); + + private static void AddTemp(ref ArrayBuilder temps, LocalDefinition temp) + { + if (temp == null) { - exprTemps = ArrayBuilder.GetInstance(); - _expressionTemps = exprTemps; + return; } - Debug.Assert(!exprTemps.Contains(temp)); - exprTemps.Add(temp); + temps ??= ArrayBuilder.GetInstance(); + + Debug.Assert(!temps.Contains(temp)); + temps.Add(temp); } - private void ReleaseExpressionTemps() + private void ReleaseTemps(ArrayBuilder temps) { - if (_expressionTemps?.Count > 0) + if (temps?.Count > 0) { // release in reverse order to keep same temps on top of the temp stack if possible - for (int i = _expressionTemps.Count - 1; i >= 0; i--) + for (int i = temps.Count - 1; i >= 0; i--) { - var temp = _expressionTemps[i]; + var temp = temps[i]; FreeTemp(temp); } - _expressionTemps.Clear(); + temps.Clear(); } } } diff --git a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs new file mode 100644 index 0000000000000..62bbb62e8d47a --- /dev/null +++ b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs @@ -0,0 +1,158 @@ +// Licensed to the .NET Foundation under one or more agreements. +// 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.Collections.Immutable; +using System.Diagnostics; +using Microsoft.CodeAnalysis.CSharp.Symbols; + +namespace Microsoft.CodeAnalysis.CSharp.CodeGen; + +internal partial class CodeGenerator +{ + private static bool MightEscapeTemporaryRefs(BoundCall node, bool used, AddressKind? receiverAddressKind) + { + return MightEscapeTemporaryRefs( + used: used, + returnType: node.Type, + returnRefKind: node.Method.RefKind, + receiverType: !node.Method.RequiresInstanceReceiver ? null : node.ReceiverOpt?.Type, + receiverScope: node.Method.TryGetThisParameter(out var thisParameter) ? thisParameter?.EffectiveScope : null, + receiverAddressKind: receiverAddressKind, + isReceiverReadOnly: node.Method.IsEffectivelyReadOnly, + parameters: node.Method.Parameters, + arguments: node.Arguments, + argsToParamsOpt: node.ArgsToParamsOpt, + expanded: node.Expanded); + } + + private static bool MightEscapeTemporaryRefs(BoundObjectCreationExpression node, bool used) + { + return MightEscapeTemporaryRefs( + used: used, + returnType: node.Type, + returnRefKind: RefKind.None, + receiverType: null, + receiverScope: null, + receiverAddressKind: null, + isReceiverReadOnly: false, + parameters: node.Constructor.Parameters, + arguments: node.Arguments, + argsToParamsOpt: node.ArgsToParamsOpt, + expanded: node.Expanded); + } + + private static bool MightEscapeTemporaryRefs(BoundFunctionPointerInvocation node, bool used) + { + FunctionPointerMethodSymbol method = node.FunctionPointer.Signature; + return MightEscapeTemporaryRefs( + used: used, + returnType: node.Type, + returnRefKind: method.RefKind, + receiverType: null, + receiverScope: null, + receiverAddressKind: null, + isReceiverReadOnly: false, + parameters: method.Parameters, + arguments: node.Arguments, + argsToParamsOpt: default, + expanded: false); + } + + private static bool MightEscapeTemporaryRefs( + bool used, + TypeSymbol returnType, + RefKind returnRefKind, + TypeSymbol? receiverType, + ScopedKind? receiverScope, + AddressKind? receiverAddressKind, + bool isReceiverReadOnly, + ImmutableArray parameters, + ImmutableArray arguments, + ImmutableArray argsToParamsOpt, + bool expanded) + { + Debug.Assert(receiverAddressKind is null || receiverType is not null); + + int writableRefs = 0; + int readonlyRefs = 0; + + if (used && (returnRefKind != RefKind.None || returnType.IsRefLikeOrAllowsRefLikeType())) + { + writableRefs++; + } + + if (receiverType is not null) + { + receiverScope ??= ScopedKind.None; + if (receiverAddressKind is { } a && !IsAnyReadOnly(a) && receiverScope == ScopedKind.None) + { + writableRefs++; + } + else if (receiverType.IsRefLikeOrAllowsRefLikeType() && receiverScope != ScopedKind.ScopedValue) + { + if (isReceiverReadOnly || receiverType.IsReadOnly) + { + readonlyRefs++; + } + else + { + writableRefs++; + } + } + else if (receiverAddressKind != null && receiverScope == ScopedKind.None) + { + readonlyRefs++; + } + } + + if (shouldReturnTrue(writableRefs, readonlyRefs)) + { + return true; + } + + for (var arg = 0; arg < arguments.Length; arg++) + { + var parameter = Binder.GetCorrespondingParameter( + arg, + parameters, + argsToParamsOpt, + expanded); + + if (parameter is not null) + { + if (parameter.RefKind.IsWritableReference() && parameter.EffectiveScope == ScopedKind.None) + { + writableRefs++; + } + else if (parameter.Type.IsRefLikeOrAllowsRefLikeType() && parameter.EffectiveScope != ScopedKind.ScopedValue) + { + if (parameter.Type.IsReadOnly || !parameter.RefKind.IsWritableReference()) + { + readonlyRefs++; + } + else + { + writableRefs++; + } + } + else if (parameter.RefKind != RefKind.None && parameter.EffectiveScope == ScopedKind.None) + { + readonlyRefs++; + } + } + + if (shouldReturnTrue(writableRefs, readonlyRefs)) + { + return true; + } + } + + return false; + + static bool shouldReturnTrue(int writableRefs, int readonlyRefs) + { + return writableRefs > 0 && (writableRefs + readonlyRefs) > 1; + } + } +} diff --git a/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs b/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs index b9f065e75ff1d..f58773a366c2f 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs @@ -943,7 +943,7 @@ private void EmitSideEffects(BoundSequence sequence) } } - private void EmitArguments(ImmutableArray arguments, ImmutableArray parameters, ImmutableArray argRefKindsOpt) + private void EmitArguments(ImmutableArray arguments, ImmutableArray parameters, ImmutableArray argRefKindsOpt, bool mightEscapeTemporaryRefs) { // We might have an extra argument for the __arglist() of a varargs method. Debug.Assert(arguments.Length == parameters.Length || @@ -953,11 +953,19 @@ private void EmitArguments(ImmutableArray arguments, ImmutableA Debug.Assert(argRefKindsOpt.IsDefault || argRefKindsOpt.Length == arguments.Length || (argRefKindsOpt.Length == arguments.Length - 1 && arguments is [.., BoundArgListOperator]), "if we have argRefKinds, we should have one for each argument"); + var oldTempRefsMightEscape = _tempRefsMightEscape; + if (mightEscapeTemporaryRefs) + { + _tempRefsMightEscape = true; + } + for (int i = 0; i < arguments.Length; i++) { RefKind argRefKind = GetArgumentRefKind(arguments, parameters, argRefKindsOpt, i); EmitArgument(arguments[i], argRefKind); } + + _tempRefsMightEscape = oldTempRefsMightEscape; } /// @@ -1660,7 +1668,11 @@ private void EmitStaticCallExpression(BoundCall call, UseKind useKind) Debug.Assert(method.IsStatic); - EmitArguments(arguments, method.Parameters, call.ArgumentRefKindsOpt); + EmitArguments( + arguments, + method.Parameters, + call.ArgumentRefKindsOpt, + mightEscapeTemporaryRefs: MightEscapeTemporaryRefs(call, used: useKind != UseKind.Unused, receiverAddressKind: null)); int stackBehavior = GetCallStackBehavior(method, arguments); if (method.IsAbstract || method.IsVirtual) @@ -1689,6 +1701,7 @@ private void EmitInstanceCallExpression(BoundCall call, UseKind useKind) AddressKind? addressKind; bool box; LocalDefinition tempOpt; + bool mightEscapeTemporaryRefs; if (receiverIsInstanceCall(call, out BoundCall nested)) { @@ -1704,7 +1717,7 @@ private void EmitInstanceCallExpression(BoundCall call, UseKind useKind) } callKind = determineEmitReceiverStrategy(call, out addressKind, out box); - emitReceiver(call, callKind, addressKind, box, out tempOpt); + emitReceiver(call, callKind, addressKind, box, out tempOpt, out mightEscapeTemporaryRefs); while (calls.Count != 0) { @@ -1754,7 +1767,11 @@ private void EmitInstanceCallExpression(BoundCall call, UseKind useKind) } } - emitArgumentsAndCallEpilogue(call, callKind, receiverUseKind); + emitArgumentsAndCallEpilogue(call, callKind, receiverUseKind, + mightEscapeTemporaryRefs: MightEscapeTemporaryRefs( + call, + used: useKind != UseKind.Unused, + receiverAddressKind: receiverUseKind != UseKind.UsedAsAddress ? null : addressKind)); FreeOptTemp(tempOpt); tempOpt = null; @@ -1811,10 +1828,10 @@ private void EmitInstanceCallExpression(BoundCall call, UseKind useKind) else { callKind = determineEmitReceiverStrategy(call, out addressKind, out box); - emitReceiver(call, callKind, addressKind, box, out tempOpt); + emitReceiver(call, callKind, addressKind, box, out tempOpt, out mightEscapeTemporaryRefs); } - emitArgumentsAndCallEpilogue(call, callKind, useKind); + emitArgumentsAndCallEpilogue(call, callKind, useKind, mightEscapeTemporaryRefs); FreeOptTemp(tempOpt); return; @@ -1911,11 +1928,12 @@ CallKind determineEmitReceiverStrategy(BoundCall call, out AddressKind? addressK } [MethodImpl(MethodImplOptions.NoInlining)] - void emitReceiver(BoundCall call, CallKind callKind, AddressKind? addressKind, bool box, out LocalDefinition tempOpt) + void emitReceiver(BoundCall call, CallKind callKind, AddressKind? addressKind, bool box, out LocalDefinition tempOpt, out bool mightEscapeTemporaryRefs) { var receiver = call.ReceiverOpt; var receiverType = receiver.Type; tempOpt = null; + mightEscapeTemporaryRefs = MightEscapeTemporaryRefs(call, used: useKind != UseKind.Unused, receiverAddressKind: addressKind); if (addressKind is null) { @@ -1932,12 +1950,18 @@ void emitReceiver(BoundCall call, CallKind callKind, AddressKind? addressKind, b Debug.Assert(!receiverType.IsVerifierReference()); tempOpt = EmitReceiverRef(receiver, addressKind.GetValueOrDefault()); + if (mightEscapeTemporaryRefs) + { + AddBlockTemp(tempOpt); + tempOpt = null; + } + emitGenericReceiverCloneIfNecessary(call, callKind, ref tempOpt); } } [MethodImpl(MethodImplOptions.NoInlining)] - void emitArgumentsAndCallEpilogue(BoundCall call, CallKind callKind, UseKind useKind) + void emitArgumentsAndCallEpilogue(BoundCall call, CallKind callKind, UseKind useKind, bool mightEscapeTemporaryRefs) { var method = call.Method; var receiver = call.ReceiverOpt; @@ -1991,7 +2015,11 @@ void emitArgumentsAndCallEpilogue(BoundCall call, CallKind callKind, UseKind use } var arguments = call.Arguments; - EmitArguments(arguments, method.Parameters, call.ArgumentRefKindsOpt); + EmitArguments( + arguments, + method.Parameters, + call.ArgumentRefKindsOpt, + mightEscapeTemporaryRefs: mightEscapeTemporaryRefs); int stackBehavior = GetCallStackBehavior(method, arguments); switch (callKind) { @@ -2446,7 +2474,11 @@ private void EmitObjectCreationExpression(BoundObjectCreationExpression expressi } // none of the above cases, so just create an instance - EmitArguments(expression.Arguments, constructor.Parameters, expression.ArgumentRefKindsOpt); + EmitArguments( + expression.Arguments, + constructor.Parameters, + expression.ArgumentRefKindsOpt, + mightEscapeTemporaryRefs: MightEscapeTemporaryRefs(expression, used)); var stackAdjustment = GetObjCreationStackBehavior(expression); _builder.EmitOpCode(ILOpCode.Newobj, stackAdjustment); @@ -2704,7 +2736,11 @@ private bool TryInPlaceCtorCall(BoundExpression target, BoundObjectCreationExpre Debug.Assert(temp == null, "in-place ctor target should not create temps"); var constructor = objCreation.Constructor; - EmitArguments(objCreation.Arguments, constructor.Parameters, objCreation.ArgumentRefKindsOpt); + EmitArguments( + objCreation.Arguments, + constructor.Parameters, + objCreation.ArgumentRefKindsOpt, + mightEscapeTemporaryRefs: MightEscapeTemporaryRefs(objCreation, used)); // -2 to adjust for consumed target address and not produced value. var stackAdjustment = GetObjCreationStackBehavior(objCreation) - 2; _builder.EmitOpCode(ILOpCode.Call, stackAdjustment); @@ -4027,7 +4063,11 @@ private void EmitCalli(BoundFunctionPointerInvocation ptrInvocation, UseKind use } FunctionPointerMethodSymbol method = ptrInvocation.FunctionPointer.Signature; - EmitArguments(ptrInvocation.Arguments, method.Parameters, ptrInvocation.ArgumentRefKindsOpt); + EmitArguments( + ptrInvocation.Arguments, + method.Parameters, + ptrInvocation.ArgumentRefKindsOpt, + mightEscapeTemporaryRefs: MightEscapeTemporaryRefs(ptrInvocation, used: useKind != UseKind.Unused)); var stackBehavior = GetCallStackBehavior(ptrInvocation.FunctionPointer.Signature, ptrInvocation.Arguments); if (temp is object) diff --git a/src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs b/src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs index 107a8f02f2481..9a5bfd0428440 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs @@ -673,6 +673,8 @@ private void EmitBlock(BoundBlock block) { EmitUninstrumentedBlock(block); } + + ReleaseBlockTemps(); } private void EmitInstrumentedBlock(BoundBlockInstrumentation instrumentation, BoundBlock block) diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs index 021a5509ae2d7..2620087347fa7 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs @@ -1112,7 +1112,7 @@ private ImmutableArray MakeArguments( if (isComReceiver) { - RewriteArgumentsForComCall(parameters, actualArguments, refKinds, temps); + RewriteArgumentsForComCall(parameters, actualArguments, refKinds); } // * The refkind map is now filled out to match the arguments. @@ -1599,8 +1599,7 @@ static BoundExpression mergeArgumentAndSideEffect(BoundExpression argument, Arra private void RewriteArgumentsForComCall( ImmutableArray parameters, BoundExpression[] actualArguments, //already re-ordered to match parameters - ArrayBuilder argsRefKindsBuilder, - ArrayBuilder temporariesBuilder) + ArrayBuilder argsRefKindsBuilder) { Debug.Assert(actualArguments != null); Debug.Assert(actualArguments.Length == parameters.Length); @@ -1640,13 +1639,11 @@ private void RewriteArgumentsForComCall( actualArguments[argIndex] = new BoundSequence( argument.Syntax, - locals: ImmutableArray.Empty, - sideEffects: ImmutableArray.Create(boundAssignmentToTemp), + locals: [boundTemp.LocalSymbol], + sideEffects: [boundAssignmentToTemp], value: boundTemp, type: boundTemp.Type); argsRefKindsBuilder[argIndex] = RefKind.Ref; - - temporariesBuilder.Add(boundTemp.LocalSymbol); } } diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_CompoundAssignmentOperator.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_CompoundAssignmentOperator.cs index 6b87177c37ffb..4f72e87fbc03a 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_CompoundAssignmentOperator.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_CompoundAssignmentOperator.cs @@ -370,7 +370,7 @@ private BoundIndexerAccess TransformIndexerAccessContinued( if (indexer.ContainingType.IsComImport) { - RewriteArgumentsForComCall(parameters, actualArguments, refKinds, temps); + RewriteArgumentsForComCall(parameters, actualArguments, refKinds); } Debug.Assert(actualArguments.All(static arg => arg is not null)); diff --git a/src/Compilers/CSharp/Portable/Lowering/SyntheticBoundNodeFactory.cs b/src/Compilers/CSharp/Portable/Lowering/SyntheticBoundNodeFactory.cs index 637a3a8da0e85..79c3f09cd7b3f 100644 --- a/src/Compilers/CSharp/Portable/Lowering/SyntheticBoundNodeFactory.cs +++ b/src/Compilers/CSharp/Portable/Lowering/SyntheticBoundNodeFactory.cs @@ -887,7 +887,7 @@ public BoundCall Call(BoundExpression? receiver, MethodSymbol method, ImmutableA return new BoundCall( Syntax, receiver, initialBindingReceiverIsSubjectToCloning: ThreeState.Unknown, method, args, argumentNamesOpt: default(ImmutableArray), argumentRefKindsOpt: refKinds, isDelegateCall: false, expanded: false, invokedAsExtensionMethod: false, - argsToParamsOpt: ImmutableArray.Empty, defaultArguments: default(BitVector), resultKind: LookupResultKind.Viable, type: method.ReturnType) + argsToParamsOpt: default, defaultArguments: default(BitVector), resultKind: LookupResultKind.Viable, type: method.ReturnType) { WasCompilerGenerated = true }; } diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenInParametersTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenInParametersTests.cs index 1f87e26e45231..b34b097c130ad 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenInParametersTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenInParametersTests.cs @@ -861,7 +861,8 @@ static ref readonly int M(in int x) // Code size 72 (0x48) .maxstack 3 .locals init (int V_0, - int V_1) + int V_1, + int V_2) IL_0000: ldc.i4.s 42 IL_0002: stloc.0 IL_0003: ldloca.s V_0 @@ -870,22 +871,22 @@ .locals init (int V_0, IL_000b: call ""void System.Console.WriteLine(int)"" IL_0010: newobj ""Program..ctor()"" IL_0015: ldc.i4.5 - IL_0016: stloc.0 - IL_0017: ldloca.s V_0 + IL_0016: stloc.1 + IL_0017: ldloca.s V_1 IL_0019: ldc.i4.6 - IL_001a: stloc.1 - IL_001b: ldloca.s V_1 + IL_001a: stloc.2 + IL_001b: ldloca.s V_2 IL_001d: call ""int Program.this[in int, in int].get"" IL_0022: call ""void System.Console.WriteLine(int)"" IL_0027: ldc.i4.s 42 - IL_0029: stloc.0 - IL_002a: ldloca.s V_0 + IL_0029: stloc.1 + IL_002a: ldloca.s V_1 IL_002c: call ""ref readonly int Program.M(in int)"" IL_0031: ldind.i4 IL_0032: call ""void System.Console.WriteLine(int)"" IL_0037: ldc.i4.s 42 - IL_0039: stloc.0 - IL_003a: ldloca.s V_0 + IL_0039: stloc.2 + IL_003a: ldloca.s V_2 IL_003c: call ""ref readonly int Program.M(in int)"" IL_0041: ldind.i4 IL_0042: call ""void System.Console.WriteLine(int)"" diff --git a/src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests.cs b/src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests.cs index c2350f829f9eb..1e75957eea944 100644 --- a/src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests.cs +++ b/src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests.cs @@ -9554,7 +9554,8 @@ .locals init (System.ReadOnlySpan V_0, //y System.ReadOnlySpan V_3, int V_4, int[] V_5, - System.Span V_6) + System.Span V_6, + System.Span V_7) IL_0000: ldtoken ".__StaticArrayInitTypeSize=8_Align=4 .34FB5C825DE7CA4AEA6E712F19D439C1DA0C92C37B423936C5F618545CA4FA1F4" IL_0005: call "System.ReadOnlySpan System.Runtime.CompilerServices.RuntimeHelpers.CreateSpan(System.RuntimeFieldHandle)" IL_000a: stloc.0 @@ -9590,8 +9591,8 @@ .locals init (System.ReadOnlySpan V_0, //y IL_0056: ldloca.s V_3 IL_0058: ldloc.s V_5 IL_005a: newobj "System.Span..ctor(int[])" - IL_005f: stloc.s V_6 - IL_0061: ldloca.s V_6 + IL_005f: stloc.s V_7 + IL_0061: ldloca.s V_7 IL_0063: ldloc.s V_4 IL_0065: ldloca.s V_3 IL_0067: call "int System.ReadOnlySpan.Length.get" @@ -11607,7 +11608,8 @@ .locals init (System.Collections.Generic.List V_0, T[] V_2, System.Span V_3, System.Span V_4, - System.Span V_5) + System.Span V_5, + System.Span V_6) IL_0000: ldarg.0 IL_0001: ldarg.1 IL_0002: stloc.0 @@ -11643,8 +11645,8 @@ .locals init (System.Collections.Generic.List V_0, IL_004e: ldloca.s V_4 IL_0050: ldloc.2 IL_0051: newobj "System.Span..ctor(T[])" - IL_0056: stloc.s V_5 - IL_0058: ldloca.s V_5 + IL_0056: stloc.s V_6 + IL_0058: ldloca.s V_6 IL_005a: ldloc.1 IL_005b: ldloca.s V_4 IL_005d: call "int System.Span.Length.get" @@ -33775,7 +33777,8 @@ .locals init (System.ReadOnlySpan V_0, //li1 System.ReadOnlySpan V_3, int V_4, int[] V_5, - System.Span V_6) + System.Span V_6, + System.Span V_7) IL_0000: ldtoken ".__StaticArrayInitTypeSize=12_Align=4 .4636993D3E1DA4E9D6B8F87B79E8F7C6D018580D52661950EABC3845C5897A4D4" IL_0005: call "System.ReadOnlySpan System.Runtime.CompilerServices.RuntimeHelpers.CreateSpan(System.RuntimeFieldHandle)" IL_000a: stloc.0 @@ -33817,8 +33820,8 @@ .locals init (System.ReadOnlySpan V_0, //li1 IL_006d: ldloca.s V_3 IL_006f: ldloc.s V_5 IL_0071: newobj "System.Span..ctor(int[])" - IL_0076: stloc.s V_6 - IL_0078: ldloca.s V_6 + IL_0076: stloc.s V_7 + IL_0078: ldloca.s V_7 IL_007a: ldloc.s V_4 IL_007c: ldloca.s V_3 IL_007e: call "int System.ReadOnlySpan.Length.get" @@ -34383,7 +34386,8 @@ .locals init (Base[] V_0, int V_3, Base[] V_4, System.ReadOnlySpan V_5, - System.Span V_6) + System.Span V_6, + System.Span V_7) IL_0000: ldc.i4.1 IL_0001: newarr "Derived" IL_0006: dup @@ -34435,8 +34439,8 @@ .locals init (Base[] V_0, IL_006a: ldloca.s V_5 IL_006c: ldloc.s V_4 IL_006e: newobj "System.Span..ctor(Base[])" - IL_0073: stloc.s V_6 - IL_0075: ldloca.s V_6 + IL_0073: stloc.s V_7 + IL_0075: ldloca.s V_7 IL_0077: ldloc.3 IL_0078: ldloca.s V_5 IL_007a: call "int System.ReadOnlySpan.Length.get" @@ -35049,7 +35053,8 @@ .locals init (int[] V_0, int[] V_3, System.ReadOnlySpan V_4, System.ReadOnlySpan V_5, - System.Span V_6) + System.Span V_6, + System.Span V_7) IL_0000: ldc.i4.2 IL_0001: newarr "int" IL_0006: dup @@ -35101,8 +35106,8 @@ .locals init (int[] V_0, IL_005f: ldloca.s V_5 IL_0061: ldloc.3 IL_0062: newobj "System.Span..ctor(int[])" - IL_0067: stloc.s V_6 - IL_0069: ldloca.s V_6 + IL_0067: stloc.s V_7 + IL_0069: ldloca.s V_7 IL_006b: ldloc.2 IL_006c: ldloca.s V_5 IL_006e: call "int System.ReadOnlySpan.Length.get" @@ -35330,7 +35335,8 @@ .locals init (int V_0, System.ReadOnlySpan V_6, System.Runtime.CompilerServices.TaskAwaiter V_7, System.Span V_8, - System.Exception V_9) + System.Span V_9, + System.Exception V_10) IL_0000: ldarg.0 IL_0001: ldfld "int C.
d__0.<>1__state" IL_0006: stloc.0 @@ -35425,8 +35431,8 @@ .locals init (int V_0, IL_00ea: ldarg.0 IL_00eb: ldfld "int[] C.
d__0.<>7__wrap4" IL_00f0: newobj "System.Span..ctor(int[])" - IL_00f5: stloc.s V_8 - IL_00f7: ldloca.s V_8 + IL_00f5: stloc.s V_9 + IL_00f7: ldloca.s V_9 IL_00f9: ldloc.s V_4 IL_00fb: ldloca.s V_6 IL_00fd: call "int System.ReadOnlySpan.Length.get" @@ -35505,13 +35511,13 @@ .locals init (int V_0, } catch System.Exception { - IL_01c5: stloc.s V_9 + IL_01c5: stloc.s V_10 IL_01c7: ldarg.0 IL_01c8: ldc.i4.s -2 IL_01ca: stfld "int C.
d__0.<>1__state" IL_01cf: ldarg.0 IL_01d0: ldflda "System.Runtime.CompilerServices.AsyncTaskMethodBuilder C.
d__0.<>t__builder" - IL_01d5: ldloc.s V_9 + IL_01d5: ldloc.s V_10 IL_01d7: call "void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetException(System.Exception)" IL_01dc: leave.s IL_01f1 } diff --git a/src/Compilers/CSharp/Test/Emit3/Semantics/InlineArrayTests.cs b/src/Compilers/CSharp/Test/Emit3/Semantics/InlineArrayTests.cs index 795ef8e73fb65..ac248191c94d6 100644 --- a/src/Compilers/CSharp/Test/Emit3/Semantics/InlineArrayTests.cs +++ b/src/Compilers/CSharp/Test/Emit3/Semantics/InlineArrayTests.cs @@ -7339,13 +7339,14 @@ static async Task FromResult(T r) verifier.VerifyIL("Program.d__2.System.Runtime.CompilerServices.IAsyncStateMachine.MoveNext", @" { - // Code size 208 (0xd0) + // Code size 209 (0xd1) .maxstack 3 .locals init (int V_0, int V_1, System.Runtime.CompilerServices.TaskAwaiter V_2, System.Span V_3, - System.Exception V_4) + System.Span V_4, + System.Exception V_5) IL_0000: ldarg.0 IL_0001: ldfld ""int Program.d__2.<>1__state"" IL_0006: stloc.0 @@ -7377,7 +7378,7 @@ .locals init (int V_0, IL_0041: ldloca.s V_2 IL_0043: ldarg.0 IL_0044: call ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.AwaitUnsafeOnCompleted, Program.d__2>(ref System.Runtime.CompilerServices.TaskAwaiter, ref Program.d__2)"" - IL_0049: leave IL_00cf + IL_0049: leave IL_00d0 IL_004e: ldarg.0 IL_004f: ldfld ""System.Runtime.CompilerServices.TaskAwaiter Program.d__2.<>u__1"" IL_0054: stloc.2 @@ -7402,36 +7403,36 @@ .locals init (int V_0, IL_0087: ldc.i4.0 IL_0088: ldloc.1 IL_0089: call ""System.Span System.Span.Slice(int, int)"" - IL_008e: stloc.3 - IL_008f: ldloca.s V_3 - IL_0091: ldc.i4.0 - IL_0092: call ""ref int System.Span.this[int].get"" - IL_0097: ldc.i4.s 111 - IL_0099: stind.i4 - IL_009a: ldarg.0 - IL_009b: ldnull - IL_009c: stfld ""C Program.d__2.<>7__wrap1"" - IL_00a1: leave.s IL_00bc + IL_008e: stloc.s V_4 + IL_0090: ldloca.s V_4 + IL_0092: ldc.i4.0 + IL_0093: call ""ref int System.Span.this[int].get"" + IL_0098: ldc.i4.s 111 + IL_009a: stind.i4 + IL_009b: ldarg.0 + IL_009c: ldnull + IL_009d: stfld ""C Program.d__2.<>7__wrap1"" + IL_00a2: leave.s IL_00bd } catch System.Exception { - IL_00a3: stloc.s V_4 - IL_00a5: ldarg.0 - IL_00a6: ldc.i4.s -2 - IL_00a8: stfld ""int Program.d__2.<>1__state"" - IL_00ad: ldarg.0 - IL_00ae: ldflda ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder Program.d__2.<>t__builder"" - IL_00b3: ldloc.s V_4 - IL_00b5: call ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetException(System.Exception)"" - IL_00ba: leave.s IL_00cf + IL_00a4: stloc.s V_5 + IL_00a6: ldarg.0 + IL_00a7: ldc.i4.s -2 + IL_00a9: stfld ""int Program.d__2.<>1__state"" + IL_00ae: ldarg.0 + IL_00af: ldflda ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder Program.d__2.<>t__builder"" + IL_00b4: ldloc.s V_5 + IL_00b6: call ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetException(System.Exception)"" + IL_00bb: leave.s IL_00d0 } - IL_00bc: ldarg.0 - IL_00bd: ldc.i4.s -2 - IL_00bf: stfld ""int Program.d__2.<>1__state"" - IL_00c4: ldarg.0 - IL_00c5: ldflda ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder Program.d__2.<>t__builder"" - IL_00ca: call ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetResult()"" - IL_00cf: ret + IL_00bd: ldarg.0 + IL_00be: ldc.i4.s -2 + IL_00c0: stfld ""int Program.d__2.<>1__state"" + IL_00c5: ldarg.0 + IL_00c6: ldflda ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder Program.d__2.<>t__builder"" + IL_00cb: call ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetResult()"" + IL_00d0: ret } "); } @@ -7482,7 +7483,8 @@ .locals init (int V_0, int V_2, System.Runtime.CompilerServices.TaskAwaiter V_3, System.Span V_4, - System.Exception V_5) + System.Span V_5, + System.Exception V_6) IL_0000: ldarg.0 IL_0001: ldfld ""int Program.d__2.<>1__state"" IL_0006: stloc.0 @@ -7547,8 +7549,8 @@ .locals init (int V_0, IL_0098: ldloc.2 IL_0099: sub IL_009a: call ""System.Span System.Span.Slice(int, int)"" - IL_009f: stloc.s V_4 - IL_00a1: ldloca.s V_4 + IL_009f: stloc.s V_5 + IL_00a1: ldloca.s V_5 IL_00a3: ldc.i4.0 IL_00a4: call ""ref int System.Span.this[int].get"" IL_00a9: ldc.i4.s 111 @@ -7560,13 +7562,13 @@ .locals init (int V_0, } catch System.Exception { - IL_00b5: stloc.s V_5 + IL_00b5: stloc.s V_6 IL_00b7: ldarg.0 IL_00b8: ldc.i4.s -2 IL_00ba: stfld ""int Program.d__2.<>1__state"" IL_00bf: ldarg.0 IL_00c0: ldflda ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder Program.d__2.<>t__builder"" - IL_00c5: ldloc.s V_5 + IL_00c5: ldloc.s V_6 IL_00c7: call ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetException(System.Exception)"" IL_00cc: leave.s IL_00e1 } @@ -7630,7 +7632,8 @@ .locals init (int V_0, System.Runtime.CompilerServices.TaskAwaiter V_5, System.Index V_6, System.Span V_7, - System.Exception V_8) + System.Span V_8, + System.Exception V_9) IL_0000: ldarg.0 IL_0001: ldfld ""int Program.d__2.<>1__state"" IL_0006: stloc.0 @@ -7711,8 +7714,8 @@ .locals init (int V_0, IL_00cc: ldloc.3 IL_00cd: ldloc.s V_4 IL_00cf: call ""System.Span System.Span.Slice(int, int)"" - IL_00d4: stloc.s V_7 - IL_00d6: ldloca.s V_7 + IL_00d4: stloc.s V_8 + IL_00d6: ldloca.s V_8 IL_00d8: ldc.i4.0 IL_00d9: call ""ref int System.Span.this[int].get"" IL_00de: ldc.i4.s 111 @@ -7724,13 +7727,13 @@ .locals init (int V_0, } catch System.Exception { - IL_00ea: stloc.s V_8 + IL_00ea: stloc.s V_9 IL_00ec: ldarg.0 IL_00ed: ldc.i4.s -2 IL_00ef: stfld ""int Program.d__2.<>1__state"" IL_00f4: ldarg.0 IL_00f5: ldflda ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder Program.d__2.<>t__builder"" - IL_00fa: ldloc.s V_8 + IL_00fa: ldloc.s V_9 IL_00fc: call ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetException(System.Exception)"" IL_0101: leave.s IL_0116 } @@ -7795,7 +7798,8 @@ .locals init (int V_0, int V_2, System.Runtime.CompilerServices.TaskAwaiter V_3, System.Span V_4, - System.Exception V_5) + System.Span V_5, + System.Exception V_6) IL_0000: ldarg.0 IL_0001: ldfld ""int Program.d__2.<>1__state"" IL_0006: stloc.0 @@ -7872,8 +7876,8 @@ .locals init (int V_0, IL_00c5: ldarg.0 IL_00c6: ldfld ""int Program.d__2.<>7__wrap2"" IL_00cb: call ""System.Span System.Span.Slice(int, int)"" - IL_00d0: stloc.s V_4 - IL_00d2: ldloca.s V_4 + IL_00d0: stloc.s V_5 + IL_00d2: ldloca.s V_5 IL_00d4: ldarg.0 IL_00d5: ldfld ""int Program.d__2.<>7__wrap3"" IL_00da: call ""ref int System.Span.this[int].get"" @@ -7886,13 +7890,13 @@ .locals init (int V_0, } catch System.Exception { - IL_00ea: stloc.s V_5 + IL_00ea: stloc.s V_6 IL_00ec: ldarg.0 IL_00ed: ldc.i4.s -2 IL_00ef: stfld ""int Program.d__2.<>1__state"" IL_00f4: ldarg.0 IL_00f5: ldflda ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder Program.d__2.<>t__builder"" - IL_00fa: ldloc.s V_5 + IL_00fa: ldloc.s V_6 IL_00fc: call ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetException(System.Exception)"" IL_0101: leave.s IL_0116 } @@ -7961,7 +7965,8 @@ .locals init (int V_0, System.Index V_6, System.Runtime.CompilerServices.TaskAwaiter V_7, System.ReadOnlySpan V_8, - System.Exception V_9) + System.ReadOnlySpan V_9, + System.Exception V_10) IL_0000: ldarg.0 IL_0001: ldfld ""int Program.d__1.<>1__state"" IL_0006: stloc.0 @@ -8051,8 +8056,8 @@ .locals init (int V_0, IL_00ee: ldarg.0 IL_00ef: ldfld ""int Program.d__1.<>7__wrap2"" IL_00f4: call ""System.ReadOnlySpan System.ReadOnlySpan.Slice(int, int)"" - IL_00f9: stloc.s V_8 - IL_00fb: ldloca.s V_8 + IL_00f9: stloc.s V_9 + IL_00fb: ldloca.s V_9 IL_00fd: ldloc.s V_5 IL_00ff: call ""ref readonly int System.ReadOnlySpan.this[int].get"" IL_0104: ldind.i4 @@ -8061,13 +8066,13 @@ .locals init (int V_0, } catch System.Exception { - IL_0108: stloc.s V_9 + IL_0108: stloc.s V_10 IL_010a: ldarg.0 IL_010b: ldc.i4.s -2 IL_010d: stfld ""int Program.d__1.<>1__state"" IL_0112: ldarg.0 IL_0113: ldflda ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder Program.d__1.<>t__builder"" - IL_0118: ldloc.s V_9 + IL_0118: ldloc.s V_10 IL_011a: call ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetException(System.Exception)"" IL_011f: leave.s IL_0135 } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs index c0c092edba74b..7ce2e394ec670 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs @@ -10216,6 +10216,875 @@ static R F2() Diagnostic(ErrorCode.ERR_RefReturnLvalueExpected, "1").WithLocation(18, 22)); } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_Escapes() + { + var source = """ + var r1 = new R(111); + var r2 = new R(222); + Report(r1.F, r2.F); + + static void Report(int x, int y) => System.Console.WriteLine($"{x} {y}"); + + ref struct R(in int x) + { + public ref readonly int F = ref x; + } + """; + CompileAndVerify(source, + expectedOutput: RefFieldTests.IncludeExpectedOutput("111 222"), + targetFramework: TargetFramework.Net70, + verify: Verification.FailsPEVerify) + .VerifyDiagnostics() + // Needs two int temps. + .VerifyIL("", """ + { + // Code size 43 (0x2b) + .maxstack 2 + .locals init (R V_0, //r2 + int V_1, + int V_2) + IL_0000: ldc.i4.s 111 + IL_0002: stloc.1 + IL_0003: ldloca.s V_1 + IL_0005: newobj "R..ctor(in int)" + IL_000a: ldc.i4 0xde + IL_000f: stloc.2 + IL_0010: ldloca.s V_2 + IL_0012: newobj "R..ctor(in int)" + IL_0017: stloc.0 + IL_0018: ldfld "ref readonly int R.F" + IL_001d: ldind.i4 + IL_001e: ldloc.0 + IL_001f: ldfld "ref readonly int R.F" + IL_0024: ldind.i4 + IL_0025: call "void Program.<
$>g__Report|0_0(int, int)" + IL_002a: ret + } + """); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_CannotEscape_NonRefStruct() + { + var source = """ + var r1 = new R(111); + var r2 = new R(222); + + struct R(in int x) + { + public readonly int F = x; + } + """; + CompileAndVerify(source) + .VerifyDiagnostics() + // One int temp is enough. + .VerifyIL("", """ + { + // Code size 26 (0x1a) + .maxstack 1 + .locals init (int V_0) + IL_0000: ldc.i4.s 111 + IL_0002: stloc.0 + IL_0003: ldloca.s V_0 + IL_0005: newobj "R..ctor(in int)" + IL_000a: pop + IL_000b: ldc.i4 0xde + IL_0010: stloc.0 + IL_0011: ldloca.s V_0 + IL_0013: newobj "R..ctor(in int)" + IL_0018: pop + IL_0019: ret + } + """); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_CannotEscape_UnusedResult() + { + var source = """ + new R(111); + new R(222); + + ref struct R + { + public R(in int x) { } + } + """; + CompileAndVerify(source) + .VerifyDiagnostics() + // One int temp is enough. + .VerifyIL("", """ + { + // Code size 26 (0x1a) + .maxstack 1 + .locals init (int V_0) + IL_0000: ldc.i4.s 111 + IL_0002: stloc.0 + IL_0003: ldloca.s V_0 + IL_0005: newobj "R..ctor(in int)" + IL_000a: pop + IL_000b: ldc.i4 0xde + IL_0010: stloc.0 + IL_0011: ldloca.s V_0 + IL_0013: newobj "R..ctor(in int)" + IL_0018: pop + IL_0019: ret + } + """); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_CannotEscape_ScopedParameter() + { + var source = """ + var r1 = new R(111); + var r2 = new R(222); + Use(r1, r2); + + static void Use(R x, R y) { } + + ref struct R + { + public R(scoped in int x) { } + } + """; + CompileAndVerify(source) + .VerifyDiagnostics() + // One int temp is enough. + .VerifyIL("", """ + { + // Code size 31 (0x1f) + .maxstack 2 + .locals init (R V_0, //r2 + int V_1) + IL_0000: ldc.i4.s 111 + IL_0002: stloc.1 + IL_0003: ldloca.s V_1 + IL_0005: newobj "R..ctor(scoped in int)" + IL_000a: ldc.i4 0xde + IL_000f: stloc.1 + IL_0010: ldloca.s V_1 + IL_0012: newobj "R..ctor(scoped in int)" + IL_0017: stloc.0 + IL_0018: ldloc.0 + IL_0019: call "void Program.<
$>g__Use|0_0(R, R)" + IL_001e: ret + } + """); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_Escapes_NestedBlock() + { + var source = """ + { + var r1 = new R(111); + var r2 = new R(222); + Report(r1.F, r2.F); + } + { + var r1 = new R(333); + var r2 = new R(444); + Report(r1.F, r2.F); + } + + static void Report(int x, int y) => System.Console.Write($"{x} {y} "); + + ref struct R(in int x) + { + public ref readonly int F = ref x; + } + """; + CompileAndVerify(source, + expectedOutput: RefFieldTests.IncludeExpectedOutput("111 222 333 444"), + targetFramework: TargetFramework.Net70, + verify: Verification.FailsPEVerify) + .VerifyDiagnostics() + // Two int and two R temps are enough. + .VerifyIL("", """ + { + // Code size 88 (0x58) + .maxstack 2 + .locals init (R V_0, //r2 + int V_1, + int V_2, + R V_3) //r2 + IL_0000: ldc.i4.s 111 + IL_0002: stloc.1 + IL_0003: ldloca.s V_1 + IL_0005: newobj "R..ctor(in int)" + IL_000a: ldc.i4 0xde + IL_000f: stloc.2 + IL_0010: ldloca.s V_2 + IL_0012: newobj "R..ctor(in int)" + IL_0017: stloc.0 + IL_0018: ldfld "ref readonly int R.F" + IL_001d: ldind.i4 + IL_001e: ldloc.0 + IL_001f: ldfld "ref readonly int R.F" + IL_0024: ldind.i4 + IL_0025: call "void Program.<
$>g__Report|0_0(int, int)" + IL_002a: ldc.i4 0x14d + IL_002f: stloc.1 + IL_0030: ldloca.s V_1 + IL_0032: newobj "R..ctor(in int)" + IL_0037: ldc.i4 0x1bc + IL_003c: stloc.2 + IL_003d: ldloca.s V_2 + IL_003f: newobj "R..ctor(in int)" + IL_0044: stloc.3 + IL_0045: ldfld "ref readonly int R.F" + IL_004a: ldind.i4 + IL_004b: ldloc.3 + IL_004c: ldfld "ref readonly int R.F" + IL_0051: ldind.i4 + IL_0052: call "void Program.<
$>g__Report|0_0(int, int)" + IL_0057: ret + } + """); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_CannotEscape_ComCall() + { + var source = """ + using System.Runtime.InteropServices; + + I i = new C(); + i.M(111); + i.M(222); + + [ComImport, Guid("96A2DE64-6D44-4DA5-BBA4-25F5F07E0E6B")] + interface I + { + void M(ref int i); + } + + class C : I + { + void I.M(ref int i) { } + } + """; + CompileAndVerify(source) + .VerifyDiagnostics() + // One int temp is enough. + .VerifyIL("", """ + { + // Code size 30 (0x1e) + .maxstack 3 + .locals init (int V_0) + IL_0000: newobj "C..ctor()" + IL_0005: dup + IL_0006: ldc.i4.s 111 + IL_0008: stloc.0 + IL_0009: ldloca.s V_0 + IL_000b: callvirt "void I.M(ref int)" + IL_0010: ldc.i4 0xde + IL_0015: stloc.0 + IL_0016: ldloca.s V_0 + IL_0018: callvirt "void I.M(ref int)" + IL_001d: ret + } + """); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_Escapes_ComCall() + { + var source = """ + using System.Diagnostics.CodeAnalysis; + using System.Runtime.InteropServices; + + I i = new C(); + var r1 = i.M(111); + var r2 = i.M(222); + Report(r1.F, r2.F); + + static void Report(int x, int y) => System.Console.WriteLine($"{x} {y}"); + + [ComImport, Guid("96A2DE64-6D44-4DA5-BBA4-25F5F07E0E6B")] + interface I + { + R M([UnscopedRef] ref int i); + } + + class C : I + { + R I.M([UnscopedRef] ref int i) + { + var r = new R(); + r.F = ref i; + return r; + } + } + + ref struct R + { + public ref int F; + } + """; + CompileAndVerify(source, + expectedOutput: RefFieldTests.IncludeExpectedOutput("111 222"), + targetFramework: TargetFramework.Net70, + verify: Verification.Fails) + .VerifyDiagnostics() + // Needs two int temps. + .VerifyIL("", """ + { + // Code size 51 (0x33) + .maxstack 3 + .locals init (R V_0, //r1 + R V_1, //r2 + int V_2, + int V_3) + IL_0000: newobj "C..ctor()" + IL_0005: dup + IL_0006: ldc.i4.s 111 + IL_0008: stloc.2 + IL_0009: ldloca.s V_2 + IL_000b: callvirt "R I.M(ref int)" + IL_0010: stloc.0 + IL_0011: ldc.i4 0xde + IL_0016: stloc.3 + IL_0017: ldloca.s V_3 + IL_0019: callvirt "R I.M(ref int)" + IL_001e: stloc.1 + IL_001f: ldloc.0 + IL_0020: ldfld "ref int R.F" + IL_0025: ldind.i4 + IL_0026: ldloc.1 + IL_0027: ldfld "ref int R.F" + IL_002c: ldind.i4 + IL_002d: call "void Program.<
$>g__Report|0_0(int, int)" + IL_0032: ret + } + """); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_NoTemp() + { + var source = """ + var x1 = 111; + var r1 = new R(x1); + var x2 = 222; + var r2 = new R(x2); + Report(r1.F, r2.F); + + static void Report(int x, int y) => System.Console.WriteLine($"{x} {y}"); + + ref struct R(in int x) + { + public ref readonly int F = ref x; + } + """; + CompileAndVerify(source, + expectedOutput: RefFieldTests.IncludeExpectedOutput("111 222"), + targetFramework: TargetFramework.Net70, + verify: Verification.FailsPEVerify) + .VerifyDiagnostics() + // No int temps needed. + .VerifyIL("", """ + { + // Code size 43 (0x2b) + .maxstack 2 + .locals init (int V_0, //x1 + int V_1, //x2 + R V_2) //r2 + IL_0000: ldc.i4.s 111 + IL_0002: stloc.0 + IL_0003: ldloca.s V_0 + IL_0005: newobj "R..ctor(in int)" + IL_000a: ldc.i4 0xde + IL_000f: stloc.1 + IL_0010: ldloca.s V_1 + IL_0012: newobj "R..ctor(in int)" + IL_0017: stloc.2 + IL_0018: ldfld "ref readonly int R.F" + IL_001d: ldind.i4 + IL_001e: ldloc.2 + IL_001f: ldfld "ref readonly int R.F" + IL_0024: ldind.i4 + IL_0025: call "void Program.<
$>g__Report|0_0(int, int)" + IL_002a: ret + } + """); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_Escapes_RValue() + { + var source = """ + var r1 = new R(F1()); + var r2 = new R(F2()); + Report(r1.F, r2.F); + + static int F1() => 111; + static int F2() => 222; + static void Report(int x, int y) => System.Console.WriteLine($"{x} {y}"); + + ref struct R(in int x) + { + public ref readonly int F = ref x; + } + """; + CompileAndVerify(source, + expectedOutput: RefFieldTests.IncludeExpectedOutput("111 222"), + targetFramework: TargetFramework.Net70, + verify: Verification.FailsPEVerify) + .VerifyDiagnostics() + // Needs two int temps. + .VerifyIL("", """ + { + // Code size 46 (0x2e) + .maxstack 2 + .locals init (R V_0, //r2 + int V_1, + int V_2) + IL_0000: call "int Program.<
$>g__F1|0_0()" + IL_0005: stloc.1 + IL_0006: ldloca.s V_1 + IL_0008: newobj "R..ctor(in int)" + IL_000d: call "int Program.<
$>g__F2|0_1()" + IL_0012: stloc.2 + IL_0013: ldloca.s V_2 + IL_0015: newobj "R..ctor(in int)" + IL_001a: stloc.0 + IL_001b: ldfld "ref readonly int R.F" + IL_0020: ldind.i4 + IL_0021: ldloc.0 + IL_0022: ldfld "ref readonly int R.F" + IL_0027: ldind.i4 + IL_0028: call "void Program.<
$>g__Report|0_2(int, int)" + IL_002d: ret + } + """); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_Escapes_Assignment() + { + var source = """ + var r1 = new R(100); + r1 = new R(101); + var r2 = new R(200); + r2 = new R(201); + Report(r1.F, r2.F); + + static void Report(int x, int y) => System.Console.WriteLine($"{x} {y}"); + + ref struct R(in int x) + { + public ref readonly int F = ref x; + } + """; + CompileAndVerify(source, + expectedOutput: RefFieldTests.IncludeExpectedOutput("101 201"), + targetFramework: TargetFramework.Net70, + verify: Verification.FailsPEVerify) + .VerifyDiagnostics() + // Needs at least two int temps. + .VerifyIL("", """ + { + // Code size 68 (0x44) + .maxstack 2 + .locals init (R V_0, //r2 + int V_1, + int V_2, + int V_3) + IL_0000: ldc.i4.s 100 + IL_0002: stloc.1 + IL_0003: ldloca.s V_1 + IL_0005: newobj "R..ctor(in int)" + IL_000a: pop + IL_000b: ldc.i4.s 101 + IL_000d: stloc.1 + IL_000e: ldloca.s V_1 + IL_0010: newobj "R..ctor(in int)" + IL_0015: ldc.i4 0xc8 + IL_001a: stloc.2 + IL_001b: ldloca.s V_2 + IL_001d: newobj "R..ctor(in int)" + IL_0022: stloc.0 + IL_0023: ldc.i4 0xc9 + IL_0028: stloc.3 + IL_0029: ldloca.s V_3 + IL_002b: newobj "R..ctor(in int)" + IL_0030: stloc.0 + IL_0031: ldfld "ref readonly int R.F" + IL_0036: ldind.i4 + IL_0037: ldloc.0 + IL_0038: ldfld "ref readonly int R.F" + IL_003d: ldind.i4 + IL_003e: call "void Program.<
$>g__Report|0_0(int, int)" + IL_0043: ret + } + """); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_Escapes_PatternMatch() + { + var source = """ + if (new R(111) is { } r1 && new R(222) is { } r2) + { + Report(r1.F, r2.F); + } + + static void Report(int x, int y) => System.Console.WriteLine($"{x} {y}"); + + ref struct R(in int x) + { + public ref readonly int F = ref x; + } + """; + CompileAndVerify(source, + expectedOutput: RefFieldTests.IncludeExpectedOutput("111 222"), + targetFramework: TargetFramework.Net70, + verify: Verification.FailsPEVerify) + .VerifyDiagnostics() + // Needs two int temps. + .VerifyIL("", """ + { + // Code size 45 (0x2d) + .maxstack 2 + .locals init (R V_0, //r1 + R V_1, //r2 + int V_2, + int V_3) + IL_0000: ldc.i4.s 111 + IL_0002: stloc.2 + IL_0003: ldloca.s V_2 + IL_0005: newobj "R..ctor(in int)" + IL_000a: stloc.0 + IL_000b: ldc.i4 0xde + IL_0010: stloc.3 + IL_0011: ldloca.s V_3 + IL_0013: newobj "R..ctor(in int)" + IL_0018: stloc.1 + IL_0019: ldloc.0 + IL_001a: ldfld "ref readonly int R.F" + IL_001f: ldind.i4 + IL_0020: ldloc.1 + IL_0021: ldfld "ref readonly int R.F" + IL_0026: ldind.i4 + IL_0027: call "void Program.<
$>g__Report|0_0(int, int)" + IL_002c: ret + } + """); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_Escapes_Initializer() + { + var source = """ + var r1 = new R() { F = ref M(111) }; + var r2 = new R() { F = ref M(222) }; + Report(r1.F, r2.F); + + static void Report(int x, int y) => System.Console.WriteLine($"{x} {y}"); + + static ref readonly int M(in int x) => ref x; + + ref struct R + { + public ref readonly int F; + } + """; + CompileAndVerify(source, + expectedOutput: RefFieldTests.IncludeExpectedOutput("111 222"), + targetFramework: TargetFramework.Net70, + verify: Verification.Fails) + .VerifyDiagnostics(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_Escapes_ViaOutParameter() + { + var source = """ + scoped R r1, r2; + M(111, out r1); + M(222, out r2); + Report(r1.F, r2.F); + + static void Report(int x, int y) => System.Console.WriteLine($"{x} {y}"); + + static void M(in int x, out R r) => r = new R(x); + + ref struct R(in int x) + { + public ref readonly int F = ref x; + } + """; + CompileAndVerify(source, + expectedOutput: RefFieldTests.IncludeExpectedOutput("111 222"), + targetFramework: TargetFramework.Net70, + verify: Verification.FailsPEVerify) + .VerifyDiagnostics(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_CannotEscape_ViaOutDiscard() + { + var source = """ + M(111, out _); + M(222, out _); + + static void M(in int x, out R r) => r = default; + + ref struct R; + """; + CompileAndVerify(source) + .VerifyDiagnostics() + // One int temp would be enough. But currently the emit layer does not see the argument is a discard. + .VerifyIL("", """ + { + // Code size 28 (0x1c) + .maxstack 2 + .locals init (R V_0, + int V_1, + int V_2) + IL_0000: ldc.i4.s 111 + IL_0002: stloc.1 + IL_0003: ldloca.s V_1 + IL_0005: ldloca.s V_0 + IL_0007: call "void Program.<
$>g__M|0_0(in int, out R)" + IL_000c: ldc.i4 0xde + IL_0011: stloc.2 + IL_0012: ldloca.s V_2 + IL_0014: ldloca.s V_0 + IL_0016: call "void Program.<
$>g__M|0_0(in int, out R)" + IL_001b: ret + } + """); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_Escapes_InstanceMethod() + { + var source = """ + using System.Diagnostics.CodeAnalysis; + + scoped var r1 = new R(); + scoped var r2 = new R(); + r1.Set(111); + r2.Set(222); + Report(r1.F, r2.F); + + static void Report(int x, int y) => System.Console.WriteLine($"{x} {y}"); + + ref struct R + { + public ref readonly int F; + + public void Set([UnscopedRef] in int x) => F = ref x; + } + """; + CompileAndVerify(source, + expectedOutput: RefFieldTests.IncludeExpectedOutput("111 222"), + targetFramework: TargetFramework.Net70, + verify: Verification.FailsPEVerify) + .VerifyDiagnostics(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_Escapes_FunctionPointer() + { + var source = """ + unsafe + { + delegate* f = &M; + var r1 = f(111); + var r2 = f(222); + Report(r1.F, r2.F); + } + + static void Report(int x, int y) => System.Console.WriteLine($"{x} {y}"); + + static R M(in int x) => new R(in x); + + ref struct R(ref readonly int x) + { + public ref readonly int F = ref x; + } + """; + CompileAndVerify(source, + expectedOutput: RefFieldTests.IncludeExpectedOutput("111 222"), + options: TestOptions.UnsafeReleaseExe, + targetFramework: TargetFramework.Net70, + verify: Verification.Fails) + .VerifyDiagnostics(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_Escapes_RefAssignment() + { + var source = """ + ref readonly int x = ref 111.M(); + ref readonly int y = ref 222.M(); + + Report(x, y); + + static void Report(int x, int y) => System.Console.WriteLine($"{x} {y}"); + + static class E + { + public static ref readonly int M(this in int x) => ref x; + } + """; + CompileAndVerify(source, + expectedOutput: RefFieldTests.IncludeExpectedOutput("111 222"), + targetFramework: TargetFramework.Net70, + verify: Verification.Fails) + .VerifyDiagnostics(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_Escapes_Receiver() + { + var source = """ + using System.Diagnostics.CodeAnalysis; + + class C + { + static S M1() => new S { F = 111 }; + + static void M2(ref int x, ref int y) => System.Console.WriteLine($"{x} {y}"); + + static void Main() + { + M2(ref M1().Ref(), ref new S { F = 222 }.Ref()); + } + } + + struct S + { + public int F; + + [UnscopedRef] public ref int Ref() => ref F; + } + """; + CompileAndVerify(source, + expectedOutput: RefFieldTests.IncludeExpectedOutput("111 222"), + targetFramework: TargetFramework.Net70, + verify: Verification.Fails) + .VerifyDiagnostics(); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_Escapes_Receiver_ReadOnlyContext_01() + { + var source = """ + using System.Diagnostics.CodeAnalysis; + + new S { F = 111 }.M3(); + + struct S + { + public int F; + + [UnscopedRef] public ref int Ref() => ref F; + + public readonly void M3() + { + M2(ref Ref(), ref new S { F = 222 }.Ref()); + } + + static void M2(ref int x, ref int y) => System.Console.WriteLine($"{x} {y}"); + } + """; + CompileAndVerify(source, + expectedOutput: RefFieldTests.IncludeExpectedOutput("111 222"), + targetFramework: TargetFramework.Net70, + verify: Verification.Fails) + .VerifyDiagnostics( + // (13,16): warning CS8656: Call to non-readonly member 'S.Ref()' from a 'readonly' member results in an implicit copy of 'this'. + // M2(ref Ref(), ref new S { F = 222 }.Ref()); + Diagnostic(ErrorCode.WRN_ImplicitCopyInReadOnlyMember, "Ref").WithArguments("S.Ref()", "this").WithLocation(13, 16)); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_CannotEscape_Receiver_ReadOnlyContext_02() + { + var source = """ + using System.Diagnostics.CodeAnalysis; + + static S M1() => new S { F = 111 }; + + M1().M3(); + + struct S + { + public int F; + + [UnscopedRef] public readonly ref readonly int Ref() => ref F; + + public readonly void M3() + { + M2(in Ref(), new S { F = 222 }); + } + + static void M2(in int x, S y) => System.Console.WriteLine($"{x} {y.F}"); + } + """; + CompileAndVerify(source, + expectedOutput: RefFieldTests.IncludeExpectedOutput("111 222"), + targetFramework: TargetFramework.Net70, + verify: Verification.Fails) + .VerifyDiagnostics() + // One S temp is enough. + .VerifyIL("S.M3", """ + { + // Code size 33 (0x21) + .maxstack 3 + .locals init (S V_0) + IL_0000: ldarg.0 + IL_0001: call "readonly ref readonly int S.Ref()" + IL_0006: ldloca.s V_0 + IL_0008: initobj "S" + IL_000e: ldloca.s V_0 + IL_0010: ldc.i4 0xde + IL_0015: stfld "int S.F" + IL_001a: ldloc.0 + IL_001b: call "void S.M2(in int, S)" + IL_0020: ret + } + """); + } + + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_Escapes_Receiver_ReadOnlyContext_03() + { + var source = """ + using System.Diagnostics.CodeAnalysis; + + class C + { + static S M1() => new S { F = 111 }; + + static void M2(in int x, in int y) => System.Console.WriteLine($"{x} {y}"); + + static void Main() + { + M2(in M1().Ref(), in new S { F = 222 }.Ref()); + } + } + + public struct S + { + public int F; + + [UnscopedRef] public readonly ref readonly int Ref() => ref F; + } + """; + CompileAndVerify(source, + expectedOutput: RefFieldTests.IncludeExpectedOutput("111 222"), + targetFramework: TargetFramework.Net70, + verify: Verification.Fails) + .VerifyDiagnostics(); + } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/72873")] public void Utf8Addition() { diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/IndexedPropertyTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/IndexedPropertyTests.cs index 1556bf4fddc92..1b47af24e69c8 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/IndexedPropertyTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/IndexedPropertyTests.cs @@ -412,22 +412,23 @@ .locals init (int V_0) //value // Code size 33 (0x21) .maxstack 4 .locals init (IA V_0, - int V_1, - int V_2) + int V_1, + int V_2, + int V_3) IL_0000: ldarg.0 IL_0001: stloc.0 IL_0002: call ""int[] B.F()"" IL_0007: ldc.i4.0 IL_0008: ldelem.i4 - IL_0009: stloc.2 + IL_0009: stloc.1 IL_000a: ldloc.0 - IL_000b: ldloc.2 - IL_000c: stloc.1 - IL_000d: ldloca.s V_1 + IL_000b: ldloc.1 + IL_000c: stloc.2 + IL_000d: ldloca.s V_2 IL_000f: ldloc.0 - IL_0010: ldloc.2 - IL_0011: stloc.1 - IL_0012: ldloca.s V_1 + IL_0010: ldloc.1 + IL_0011: stloc.3 + IL_0012: ldloca.s V_3 IL_0014: callvirt ""int IA.P[ref int].get"" IL_0019: ldc.i4.1 IL_001a: add @@ -461,23 +462,24 @@ .locals init (IA V_0, // Code size 33 (0x21) .maxstack 4 .locals init (int V_0, - int V_1, - int V_2) + int V_1, + int V_2, + int V_3) IL_0000: ldarg.0 IL_0001: call ""int[] B.F()"" IL_0006: ldc.i4.0 IL_0007: ldelem.i4 - IL_0008: stloc.1 + IL_0008: stloc.0 IL_0009: dup - IL_000a: ldloc.1 - IL_000b: stloc.0 - IL_000c: ldloca.s V_0 + IL_000a: ldloc.0 + IL_000b: stloc.2 + IL_000c: ldloca.s V_2 IL_000e: callvirt ""int IA.P[ref int].get"" - IL_0013: stloc.2 - IL_0014: ldloc.1 - IL_0015: stloc.0 - IL_0016: ldloca.s V_0 - IL_0018: ldloc.2 + IL_0013: stloc.1 + IL_0014: ldloc.0 + IL_0015: stloc.3 + IL_0016: ldloca.s V_3 + IL_0018: ldloc.1 IL_0019: ldc.i4.1 IL_001a: add IL_001b: callvirt ""void IA.P[ref int].set"" @@ -730,70 +732,74 @@ static void ReportAndReset() 0, 3"); compilation2.VerifyIL("C.MissingArg(IA)", @"{ - // Code size 39 (0x27) + // Code size 41 (0x29) .maxstack 5 .locals init (int V_0, - int V_1, - int V_2, - int V_3) + int V_1, + int V_2, + int V_3, + int V_4, + int V_5) IL_0000: ldarg.0 IL_0001: ldsfld ""int C.x"" - IL_0006: stloc.2 + IL_0006: stloc.0 IL_0007: dup - IL_0008: ldloc.2 - IL_0009: stloc.0 - IL_000a: ldloca.s V_0 + IL_0008: ldloc.0 + IL_0009: stloc.2 + IL_000a: ldloca.s V_2 IL_000c: ldc.i4.0 - IL_000d: stloc.1 - IL_000e: ldloca.s V_1 + IL_000d: stloc.3 + IL_000e: ldloca.s V_3 IL_0010: callvirt ""int IA.P[ref int, ref int].get"" - IL_0015: stloc.3 - IL_0016: ldloc.2 - IL_0017: stloc.0 - IL_0018: ldloca.s V_0 - IL_001a: ldc.i4.0 - IL_001b: stloc.1 - IL_001c: ldloca.s V_1 - IL_001e: ldloc.3 - IL_001f: ldc.i4.1 - IL_0020: add - IL_0021: callvirt ""void IA.P[ref int, ref int].set"" - IL_0026: ret + IL_0015: stloc.1 + IL_0016: ldloc.0 + IL_0017: stloc.s V_4 + IL_0019: ldloca.s V_4 + IL_001b: ldc.i4.0 + IL_001c: stloc.s V_5 + IL_001e: ldloca.s V_5 + IL_0020: ldloc.1 + IL_0021: ldc.i4.1 + IL_0022: add + IL_0023: callvirt ""void IA.P[ref int, ref int].set"" + IL_0028: ret }"); compilation2.VerifyIL("C.ValueArgs(IA)", @"{ - // Code size 47 (0x2f) + // Code size 48 (0x30) .maxstack 5 .locals init (int V_0, - int V_1, - int V_2, - int V_3, - int V_4) + int V_1, + int V_2, + int V_3, + int V_4, + int V_5, + int V_6) IL_0000: ldarg.0 IL_0001: ldsfld ""int C.x"" - IL_0006: stloc.2 + IL_0006: stloc.0 IL_0007: ldsfld ""int C.y"" - IL_000c: stloc.3 + IL_000c: stloc.1 IL_000d: dup - IL_000e: ldloc.2 - IL_000f: stloc.0 - IL_0010: ldloca.s V_0 - IL_0012: ldloc.3 - IL_0013: stloc.1 - IL_0014: ldloca.s V_1 - IL_0016: callvirt ""int IA.P[ref int, ref int].get"" - IL_001b: stloc.s V_4 - IL_001d: ldloc.2 - IL_001e: stloc.0 - IL_001f: ldloca.s V_0 - IL_0021: ldloc.3 - IL_0022: stloc.1 - IL_0023: ldloca.s V_1 - IL_0025: ldloc.s V_4 - IL_0027: ldc.i4.1 - IL_0028: add - IL_0029: callvirt ""void IA.P[ref int, ref int].set"" - IL_002e: ret + IL_000e: ldloc.0 + IL_000f: stloc.3 + IL_0010: ldloca.s V_3 + IL_0012: ldloc.1 + IL_0013: stloc.s V_4 + IL_0015: ldloca.s V_4 + IL_0017: callvirt ""int IA.P[ref int, ref int].get"" + IL_001c: stloc.2 + IL_001d: ldloc.0 + IL_001e: stloc.s V_5 + IL_0020: ldloca.s V_5 + IL_0022: ldloc.1 + IL_0023: stloc.s V_6 + IL_0025: ldloca.s V_6 + IL_0027: ldloc.2 + IL_0028: ldc.i4.1 + IL_0029: add + IL_002a: callvirt ""void IA.P[ref int, ref int].set"" + IL_002f: ret }"); compilation2.VerifyIL("C.RefArgs(IA)", @"{ diff --git a/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs b/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs index 7f02bdc0ce639..da63a68b3edfb 100644 --- a/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs +++ b/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs @@ -64,6 +64,23 @@ public override bool Equals(object? obj) // maps local identities to locals. private Dictionary? _localMap; + // The lowered tree might define the same local symbol + // in multiple sequences that are part of one expression, for example: + // + // call: + // arguments: + // sequence: + // locals: LoweringTemp.1 + // ... + // sequence: + // locals: LoweringTemp.1 + // ... + // + // At the same time, these expression locals might be lifted to the containing block + // (to avoid reusing them if they might be captured by a ref struct). + // Then we use this map to keep track of the redeclared locals. + private Dictionary? _redeclaredLocals; + // pool of free slots partitioned by their signature. private KeyedStack? _freeSlots; @@ -102,6 +119,14 @@ private Dictionary LocalMap } } + private Dictionary RedeclaredLocals + { + get + { + return _redeclaredLocals ??= new Dictionary(ReferenceEqualityComparer.Instance); + } + } + private KeyedStack FreeSlots { get @@ -136,7 +161,16 @@ internal LocalDefinition DeclareLocal( local = this.DeclareLocalImpl(type, symbol, name, kind, id, pdbAttributes, constraints, dynamicTransformFlags, tupleElementNames); } - LocalMap.Add(symbol, local); + if (LocalMap.TryGetValue(symbol, out var previous)) + { + RedeclaredLocals.Add(local, previous); + LocalMap[symbol] = local; + } + else + { + LocalMap.Add(symbol, local); + } + return local; } @@ -155,8 +189,17 @@ internal LocalDefinition GetLocal(ILocalSymbolInternal symbol) internal void FreeLocal(ILocalSymbolInternal symbol) { var slot = GetLocal(symbol); - LocalMap.Remove(symbol); - FreeSlot(slot); + + if (RedeclaredLocals.TryGetValue(slot, out var previous)) + { + LocalMap[symbol] = previous; + RedeclaredLocals.Remove(slot); + } + else + { + LocalMap.Remove(symbol); + FreeSlot(slot); + } } /// From 4672a11a2e877d634fbb387237f53393b090b583 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Tue, 26 Nov 2024 11:38:50 +0100 Subject: [PATCH 02/22] Revert some changes --- .../CSharp/Portable/CodeGen/CodeGenerator.cs | 43 ++---- .../CSharp/Portable/CodeGen/EmitStatement.cs | 2 - .../LocalRewriter/LocalRewriter_Call.cs | 11 +- ...ocalRewriter_CompoundAssignmentOperator.cs | 2 +- .../Symbol/Symbols/IndexedPropertyTests.cs | 142 +++++++++--------- .../Core/Portable/CodeGen/LocalSlotManager.cs | 49 +----- 6 files changed, 93 insertions(+), 156 deletions(-) diff --git a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator.cs b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator.cs index bb3e6489064c4..ce7045499e411 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator.cs @@ -38,10 +38,7 @@ internal sealed partial class CodeGenerator // There are scenarios where rvalues need to be passed to ref/in parameters // in such cases the values must be spilled into temps and retained for the entirety of // the most encompassing expression. - // If a ref to the temp could escape, it is retained for the most encompassing block. private ArrayBuilder _expressionTemps; - private ArrayBuilder _blockTemps; - private bool _tempRefsMightEscape; // not 0 when in a protected region with a handler. private int _tryNestingLevel; @@ -507,47 +504,35 @@ private TextSpan EmitSequencePoint(SyntaxTree syntaxTree, TextSpan span) private void AddExpressionTemp(LocalDefinition temp) { - if (_tempRefsMightEscape) - { - AddBlockTemp(temp); - } - else - { - AddTemp(ref _expressionTemps, temp); - } - } - - private void ReleaseExpressionTemps() => ReleaseTemps(_expressionTemps); - - private void AddBlockTemp(LocalDefinition temp) => AddTemp(ref _blockTemps, temp); - - private void ReleaseBlockTemps() => ReleaseTemps(_blockTemps); - - private static void AddTemp(ref ArrayBuilder temps, LocalDefinition temp) - { + // in some cases like stack locals, there is no slot allocated. if (temp == null) { return; } - temps ??= ArrayBuilder.GetInstance(); + ArrayBuilder exprTemps = _expressionTemps; + if (exprTemps == null) + { + exprTemps = ArrayBuilder.GetInstance(); + _expressionTemps = exprTemps; + } - Debug.Assert(!temps.Contains(temp)); - temps.Add(temp); + Debug.Assert(!exprTemps.Contains(temp)); + exprTemps.Add(temp); } - private void ReleaseTemps(ArrayBuilder temps) + private void ReleaseExpressionTemps() { - if (temps?.Count > 0) + if (_expressionTemps?.Count > 0) { // release in reverse order to keep same temps on top of the temp stack if possible - for (int i = temps.Count - 1; i >= 0; i--) + for (int i = _expressionTemps.Count - 1; i >= 0; i--) { - var temp = temps[i]; + var temp = _expressionTemps[i]; FreeTemp(temp); } - temps.Clear(); + _expressionTemps.Clear(); } } } diff --git a/src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs b/src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs index 9a5bfd0428440..107a8f02f2481 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/EmitStatement.cs @@ -673,8 +673,6 @@ private void EmitBlock(BoundBlock block) { EmitUninstrumentedBlock(block); } - - ReleaseBlockTemps(); } private void EmitInstrumentedBlock(BoundBlockInstrumentation instrumentation, BoundBlock block) diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs index 2620087347fa7..021a5509ae2d7 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs @@ -1112,7 +1112,7 @@ private ImmutableArray MakeArguments( if (isComReceiver) { - RewriteArgumentsForComCall(parameters, actualArguments, refKinds); + RewriteArgumentsForComCall(parameters, actualArguments, refKinds, temps); } // * The refkind map is now filled out to match the arguments. @@ -1599,7 +1599,8 @@ static BoundExpression mergeArgumentAndSideEffect(BoundExpression argument, Arra private void RewriteArgumentsForComCall( ImmutableArray parameters, BoundExpression[] actualArguments, //already re-ordered to match parameters - ArrayBuilder argsRefKindsBuilder) + ArrayBuilder argsRefKindsBuilder, + ArrayBuilder temporariesBuilder) { Debug.Assert(actualArguments != null); Debug.Assert(actualArguments.Length == parameters.Length); @@ -1639,11 +1640,13 @@ private void RewriteArgumentsForComCall( actualArguments[argIndex] = new BoundSequence( argument.Syntax, - locals: [boundTemp.LocalSymbol], - sideEffects: [boundAssignmentToTemp], + locals: ImmutableArray.Empty, + sideEffects: ImmutableArray.Create(boundAssignmentToTemp), value: boundTemp, type: boundTemp.Type); argsRefKindsBuilder[argIndex] = RefKind.Ref; + + temporariesBuilder.Add(boundTemp.LocalSymbol); } } diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_CompoundAssignmentOperator.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_CompoundAssignmentOperator.cs index 4f72e87fbc03a..6b87177c37ffb 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_CompoundAssignmentOperator.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_CompoundAssignmentOperator.cs @@ -370,7 +370,7 @@ private BoundIndexerAccess TransformIndexerAccessContinued( if (indexer.ContainingType.IsComImport) { - RewriteArgumentsForComCall(parameters, actualArguments, refKinds); + RewriteArgumentsForComCall(parameters, actualArguments, refKinds, temps); } Debug.Assert(actualArguments.All(static arg => arg is not null)); diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/IndexedPropertyTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/IndexedPropertyTests.cs index 1b47af24e69c8..1556bf4fddc92 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/IndexedPropertyTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/IndexedPropertyTests.cs @@ -412,23 +412,22 @@ .locals init (int V_0) //value // Code size 33 (0x21) .maxstack 4 .locals init (IA V_0, - int V_1, - int V_2, - int V_3) + int V_1, + int V_2) IL_0000: ldarg.0 IL_0001: stloc.0 IL_0002: call ""int[] B.F()"" IL_0007: ldc.i4.0 IL_0008: ldelem.i4 - IL_0009: stloc.1 + IL_0009: stloc.2 IL_000a: ldloc.0 - IL_000b: ldloc.1 - IL_000c: stloc.2 - IL_000d: ldloca.s V_2 + IL_000b: ldloc.2 + IL_000c: stloc.1 + IL_000d: ldloca.s V_1 IL_000f: ldloc.0 - IL_0010: ldloc.1 - IL_0011: stloc.3 - IL_0012: ldloca.s V_3 + IL_0010: ldloc.2 + IL_0011: stloc.1 + IL_0012: ldloca.s V_1 IL_0014: callvirt ""int IA.P[ref int].get"" IL_0019: ldc.i4.1 IL_001a: add @@ -462,24 +461,23 @@ .locals init (IA V_0, // Code size 33 (0x21) .maxstack 4 .locals init (int V_0, - int V_1, - int V_2, - int V_3) + int V_1, + int V_2) IL_0000: ldarg.0 IL_0001: call ""int[] B.F()"" IL_0006: ldc.i4.0 IL_0007: ldelem.i4 - IL_0008: stloc.0 + IL_0008: stloc.1 IL_0009: dup - IL_000a: ldloc.0 - IL_000b: stloc.2 - IL_000c: ldloca.s V_2 + IL_000a: ldloc.1 + IL_000b: stloc.0 + IL_000c: ldloca.s V_0 IL_000e: callvirt ""int IA.P[ref int].get"" - IL_0013: stloc.1 - IL_0014: ldloc.0 - IL_0015: stloc.3 - IL_0016: ldloca.s V_3 - IL_0018: ldloc.1 + IL_0013: stloc.2 + IL_0014: ldloc.1 + IL_0015: stloc.0 + IL_0016: ldloca.s V_0 + IL_0018: ldloc.2 IL_0019: ldc.i4.1 IL_001a: add IL_001b: callvirt ""void IA.P[ref int].set"" @@ -732,74 +730,70 @@ static void ReportAndReset() 0, 3"); compilation2.VerifyIL("C.MissingArg(IA)", @"{ - // Code size 41 (0x29) + // Code size 39 (0x27) .maxstack 5 .locals init (int V_0, - int V_1, - int V_2, - int V_3, - int V_4, - int V_5) + int V_1, + int V_2, + int V_3) IL_0000: ldarg.0 IL_0001: ldsfld ""int C.x"" - IL_0006: stloc.0 + IL_0006: stloc.2 IL_0007: dup - IL_0008: ldloc.0 - IL_0009: stloc.2 - IL_000a: ldloca.s V_2 + IL_0008: ldloc.2 + IL_0009: stloc.0 + IL_000a: ldloca.s V_0 IL_000c: ldc.i4.0 - IL_000d: stloc.3 - IL_000e: ldloca.s V_3 + IL_000d: stloc.1 + IL_000e: ldloca.s V_1 IL_0010: callvirt ""int IA.P[ref int, ref int].get"" - IL_0015: stloc.1 - IL_0016: ldloc.0 - IL_0017: stloc.s V_4 - IL_0019: ldloca.s V_4 - IL_001b: ldc.i4.0 - IL_001c: stloc.s V_5 - IL_001e: ldloca.s V_5 - IL_0020: ldloc.1 - IL_0021: ldc.i4.1 - IL_0022: add - IL_0023: callvirt ""void IA.P[ref int, ref int].set"" - IL_0028: ret + IL_0015: stloc.3 + IL_0016: ldloc.2 + IL_0017: stloc.0 + IL_0018: ldloca.s V_0 + IL_001a: ldc.i4.0 + IL_001b: stloc.1 + IL_001c: ldloca.s V_1 + IL_001e: ldloc.3 + IL_001f: ldc.i4.1 + IL_0020: add + IL_0021: callvirt ""void IA.P[ref int, ref int].set"" + IL_0026: ret }"); compilation2.VerifyIL("C.ValueArgs(IA)", @"{ - // Code size 48 (0x30) + // Code size 47 (0x2f) .maxstack 5 .locals init (int V_0, - int V_1, - int V_2, - int V_3, - int V_4, - int V_5, - int V_6) + int V_1, + int V_2, + int V_3, + int V_4) IL_0000: ldarg.0 IL_0001: ldsfld ""int C.x"" - IL_0006: stloc.0 + IL_0006: stloc.2 IL_0007: ldsfld ""int C.y"" - IL_000c: stloc.1 + IL_000c: stloc.3 IL_000d: dup - IL_000e: ldloc.0 - IL_000f: stloc.3 - IL_0010: ldloca.s V_3 - IL_0012: ldloc.1 - IL_0013: stloc.s V_4 - IL_0015: ldloca.s V_4 - IL_0017: callvirt ""int IA.P[ref int, ref int].get"" - IL_001c: stloc.2 - IL_001d: ldloc.0 - IL_001e: stloc.s V_5 - IL_0020: ldloca.s V_5 - IL_0022: ldloc.1 - IL_0023: stloc.s V_6 - IL_0025: ldloca.s V_6 - IL_0027: ldloc.2 - IL_0028: ldc.i4.1 - IL_0029: add - IL_002a: callvirt ""void IA.P[ref int, ref int].set"" - IL_002f: ret + IL_000e: ldloc.2 + IL_000f: stloc.0 + IL_0010: ldloca.s V_0 + IL_0012: ldloc.3 + IL_0013: stloc.1 + IL_0014: ldloca.s V_1 + IL_0016: callvirt ""int IA.P[ref int, ref int].get"" + IL_001b: stloc.s V_4 + IL_001d: ldloc.2 + IL_001e: stloc.0 + IL_001f: ldloca.s V_0 + IL_0021: ldloc.3 + IL_0022: stloc.1 + IL_0023: ldloca.s V_1 + IL_0025: ldloc.s V_4 + IL_0027: ldc.i4.1 + IL_0028: add + IL_0029: callvirt ""void IA.P[ref int, ref int].set"" + IL_002e: ret }"); compilation2.VerifyIL("C.RefArgs(IA)", @"{ diff --git a/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs b/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs index da63a68b3edfb..7f02bdc0ce639 100644 --- a/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs +++ b/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs @@ -64,23 +64,6 @@ public override bool Equals(object? obj) // maps local identities to locals. private Dictionary? _localMap; - // The lowered tree might define the same local symbol - // in multiple sequences that are part of one expression, for example: - // - // call: - // arguments: - // sequence: - // locals: LoweringTemp.1 - // ... - // sequence: - // locals: LoweringTemp.1 - // ... - // - // At the same time, these expression locals might be lifted to the containing block - // (to avoid reusing them if they might be captured by a ref struct). - // Then we use this map to keep track of the redeclared locals. - private Dictionary? _redeclaredLocals; - // pool of free slots partitioned by their signature. private KeyedStack? _freeSlots; @@ -119,14 +102,6 @@ private Dictionary LocalMap } } - private Dictionary RedeclaredLocals - { - get - { - return _redeclaredLocals ??= new Dictionary(ReferenceEqualityComparer.Instance); - } - } - private KeyedStack FreeSlots { get @@ -161,16 +136,7 @@ internal LocalDefinition DeclareLocal( local = this.DeclareLocalImpl(type, symbol, name, kind, id, pdbAttributes, constraints, dynamicTransformFlags, tupleElementNames); } - if (LocalMap.TryGetValue(symbol, out var previous)) - { - RedeclaredLocals.Add(local, previous); - LocalMap[symbol] = local; - } - else - { - LocalMap.Add(symbol, local); - } - + LocalMap.Add(symbol, local); return local; } @@ -189,17 +155,8 @@ internal LocalDefinition GetLocal(ILocalSymbolInternal symbol) internal void FreeLocal(ILocalSymbolInternal symbol) { var slot = GetLocal(symbol); - - if (RedeclaredLocals.TryGetValue(slot, out var previous)) - { - LocalMap[symbol] = previous; - RedeclaredLocals.Remove(slot); - } - else - { - LocalMap.Remove(symbol); - FreeSlot(slot); - } + LocalMap.Remove(symbol); + FreeSlot(slot); } /// From 4dc8f82c695eeef83c531750287bf0579a53cdec Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Tue, 26 Nov 2024 16:17:12 +0100 Subject: [PATCH 03/22] Simplify the heuristic --- .../CodeGen/CodeGenerator_RefSafety.cs | 89 ++++++------------- 1 file changed, 29 insertions(+), 60 deletions(-) diff --git a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs index 62bbb62e8d47a..fffa98613ab7b 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs @@ -20,10 +20,7 @@ private static bool MightEscapeTemporaryRefs(BoundCall node, bool used, AddressK receiverScope: node.Method.TryGetThisParameter(out var thisParameter) ? thisParameter?.EffectiveScope : null, receiverAddressKind: receiverAddressKind, isReceiverReadOnly: node.Method.IsEffectivelyReadOnly, - parameters: node.Method.Parameters, - arguments: node.Arguments, - argsToParamsOpt: node.ArgsToParamsOpt, - expanded: node.Expanded); + parameters: node.Method.Parameters); } private static bool MightEscapeTemporaryRefs(BoundObjectCreationExpression node, bool used) @@ -36,10 +33,7 @@ private static bool MightEscapeTemporaryRefs(BoundObjectCreationExpression node, receiverScope: null, receiverAddressKind: null, isReceiverReadOnly: false, - parameters: node.Constructor.Parameters, - arguments: node.Arguments, - argsToParamsOpt: node.ArgsToParamsOpt, - expanded: node.Expanded); + parameters: node.Constructor.Parameters); } private static bool MightEscapeTemporaryRefs(BoundFunctionPointerInvocation node, bool used) @@ -53,10 +47,7 @@ private static bool MightEscapeTemporaryRefs(BoundFunctionPointerInvocation node receiverScope: null, receiverAddressKind: null, isReceiverReadOnly: false, - parameters: method.Parameters, - arguments: node.Arguments, - argsToParamsOpt: default, - expanded: false); + parameters: method.Parameters); } private static bool MightEscapeTemporaryRefs( @@ -67,82 +58,59 @@ private static bool MightEscapeTemporaryRefs( ScopedKind? receiverScope, AddressKind? receiverAddressKind, bool isReceiverReadOnly, - ImmutableArray parameters, - ImmutableArray arguments, - ImmutableArray argsToParamsOpt, - bool expanded) + ImmutableArray parameters) { Debug.Assert(receiverAddressKind is null || receiverType is not null); - int writableRefs = 0; - int readonlyRefs = 0; + // number of outputs that can capture references + int refTargets = 0; + // number of inputs that can contain references + int refSources = 0; if (used && (returnRefKind != RefKind.None || returnType.IsRefLikeOrAllowsRefLikeType())) { - writableRefs++; + // If returning by reference or returning a ref struct, the result might capture references. + refTargets++; } if (receiverType is not null) { receiverScope ??= ScopedKind.None; - if (receiverAddressKind is { } a && !IsAnyReadOnly(a) && receiverScope == ScopedKind.None) + if (receiverType.IsRefLikeOrAllowsRefLikeType() && receiverScope != ScopedKind.ScopedValue) { - writableRefs++; - } - else if (receiverType.IsRefLikeOrAllowsRefLikeType() && receiverScope != ScopedKind.ScopedValue) - { - if (isReceiverReadOnly || receiverType.IsReadOnly) - { - readonlyRefs++; - } - else + refSources++; + if (!isReceiverReadOnly && !receiverType.IsReadOnly) { - writableRefs++; + refTargets++; } } else if (receiverAddressKind != null && receiverScope == ScopedKind.None) { - readonlyRefs++; + refSources++; } } - if (shouldReturnTrue(writableRefs, readonlyRefs)) + if (shouldReturnTrue(refTargets, refSources)) { return true; } - for (var arg = 0; arg < arguments.Length; arg++) + foreach (var parameter in parameters) { - var parameter = Binder.GetCorrespondingParameter( - arg, - parameters, - argsToParamsOpt, - expanded); - - if (parameter is not null) + if (parameter.Type.IsRefLikeOrAllowsRefLikeType() && parameter.EffectiveScope != ScopedKind.ScopedValue) { - if (parameter.RefKind.IsWritableReference() && parameter.EffectiveScope == ScopedKind.None) + refSources++; + if (!parameter.Type.IsReadOnly && parameter.RefKind.IsWritableReference()) { - writableRefs++; - } - else if (parameter.Type.IsRefLikeOrAllowsRefLikeType() && parameter.EffectiveScope != ScopedKind.ScopedValue) - { - if (parameter.Type.IsReadOnly || !parameter.RefKind.IsWritableReference()) - { - readonlyRefs++; - } - else - { - writableRefs++; - } - } - else if (parameter.RefKind != RefKind.None && parameter.EffectiveScope == ScopedKind.None) - { - readonlyRefs++; + refTargets++; } } + else if (parameter.RefKind != RefKind.None && parameter.EffectiveScope == ScopedKind.None) + { + refSources++; + } - if (shouldReturnTrue(writableRefs, readonlyRefs)) + if (shouldReturnTrue(refTargets, refSources)) { return true; } @@ -150,9 +118,10 @@ private static bool MightEscapeTemporaryRefs( return false; - static bool shouldReturnTrue(int writableRefs, int readonlyRefs) + static bool shouldReturnTrue(int writableRefs, int readableRefs) { - return writableRefs > 0 && (writableRefs + readonlyRefs) > 1; + // If there is at least one output and at least one input, a reference can be captured. + return writableRefs > 0 && readableRefs > 0; } } } From 60dec1eb71d4a9a9098470c987950fbf4d866227 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Tue, 26 Nov 2024 12:26:31 +0100 Subject: [PATCH 04/22] Avoid reusing any local whose address has been taken --- .../CSharp/Portable/CodeGen/EmitExpression.cs | 106 +++++++++--------- .../Semantic/Semantics/RefEscapingTests.cs | 49 ++++---- .../Core/Portable/CodeGen/ILBuilderEmit.cs | 2 + .../Core/Portable/CodeGen/LocalSlotManager.cs | 60 +++++++++- 4 files changed, 141 insertions(+), 76 deletions(-) diff --git a/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs b/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs index f58773a366c2f..b8c42239e352c 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs @@ -943,7 +943,7 @@ private void EmitSideEffects(BoundSequence sequence) } } - private void EmitArguments(ImmutableArray arguments, ImmutableArray parameters, ImmutableArray argRefKindsOpt, bool mightEscapeTemporaryRefs) + private void EmitArguments(ImmutableArray arguments, ImmutableArray parameters, ImmutableArray argRefKindsOpt) { // We might have an extra argument for the __arglist() of a varargs method. Debug.Assert(arguments.Length == parameters.Length || @@ -953,19 +953,11 @@ private void EmitArguments(ImmutableArray arguments, ImmutableA Debug.Assert(argRefKindsOpt.IsDefault || argRefKindsOpt.Length == arguments.Length || (argRefKindsOpt.Length == arguments.Length - 1 && arguments is [.., BoundArgListOperator]), "if we have argRefKinds, we should have one for each argument"); - var oldTempRefsMightEscape = _tempRefsMightEscape; - if (mightEscapeTemporaryRefs) - { - _tempRefsMightEscape = true; - } - for (int i = 0; i < arguments.Length; i++) { RefKind argRefKind = GetArgumentRefKind(arguments, parameters, argRefKindsOpt, i); EmitArgument(arguments[i], argRefKind); } - - _tempRefsMightEscape = oldTempRefsMightEscape; } /// @@ -1668,11 +1660,13 @@ private void EmitStaticCallExpression(BoundCall call, UseKind useKind) Debug.Assert(method.IsStatic); - EmitArguments( - arguments, - method.Parameters, - call.ArgumentRefKindsOpt, - mightEscapeTemporaryRefs: MightEscapeTemporaryRefs(call, used: useKind != UseKind.Unused, receiverAddressKind: null)); + var countBefore = _builder.LocalSlotManager.StartScopeOfTrackingAddressedLocals(); + + EmitArguments(arguments, method.Parameters, call.ArgumentRefKindsOpt); + + _builder.LocalSlotManager.EndScopeOfTrackingAddressedLocals(countBefore, + MightEscapeTemporaryRefs(call, used: useKind != UseKind.Unused, receiverAddressKind: null)); + int stackBehavior = GetCallStackBehavior(method, arguments); if (method.IsAbstract || method.IsVirtual) @@ -1701,7 +1695,8 @@ private void EmitInstanceCallExpression(BoundCall call, UseKind useKind) AddressKind? addressKind; bool box; LocalDefinition tempOpt; - bool mightEscapeTemporaryRefs; + + var countBefore = _builder.LocalSlotManager.StartScopeOfTrackingAddressedLocals(); if (receiverIsInstanceCall(call, out BoundCall nested)) { @@ -1717,7 +1712,7 @@ private void EmitInstanceCallExpression(BoundCall call, UseKind useKind) } callKind = determineEmitReceiverStrategy(call, out addressKind, out box); - emitReceiver(call, callKind, addressKind, box, out tempOpt, out mightEscapeTemporaryRefs); + emitReceiver(call, callKind, addressKind, box, out tempOpt); while (calls.Count != 0) { @@ -1767,11 +1762,15 @@ private void EmitInstanceCallExpression(BoundCall call, UseKind useKind) } } - emitArgumentsAndCallEpilogue(call, callKind, receiverUseKind, - mightEscapeTemporaryRefs: MightEscapeTemporaryRefs( - call, - used: useKind != UseKind.Unused, - receiverAddressKind: receiverUseKind != UseKind.UsedAsAddress ? null : addressKind)); + emitArgumentsAndCallEpilogue(call, callKind, receiverUseKind); + + _builder.LocalSlotManager.EndScopeOfTrackingAddressedLocals(countBefore, MightEscapeTemporaryRefs( + call, + used: useKind != UseKind.Unused, + receiverAddressKind: receiverUseKind != UseKind.UsedAsAddress ? null : addressKind)); + + countBefore = _builder.LocalSlotManager.StartScopeOfTrackingAddressedLocals(); + FreeOptTemp(tempOpt); tempOpt = null; @@ -1828,10 +1827,16 @@ private void EmitInstanceCallExpression(BoundCall call, UseKind useKind) else { callKind = determineEmitReceiverStrategy(call, out addressKind, out box); - emitReceiver(call, callKind, addressKind, box, out tempOpt, out mightEscapeTemporaryRefs); + emitReceiver(call, callKind, addressKind, box, out tempOpt); } - emitArgumentsAndCallEpilogue(call, callKind, useKind, mightEscapeTemporaryRefs); + emitArgumentsAndCallEpilogue(call, callKind, useKind); + + _builder.LocalSlotManager.EndScopeOfTrackingAddressedLocals(countBefore, MightEscapeTemporaryRefs( + call, + used: useKind != UseKind.Unused, + receiverAddressKind: useKind != UseKind.UsedAsAddress ? null : addressKind)); + FreeOptTemp(tempOpt); return; @@ -1928,12 +1933,11 @@ CallKind determineEmitReceiverStrategy(BoundCall call, out AddressKind? addressK } [MethodImpl(MethodImplOptions.NoInlining)] - void emitReceiver(BoundCall call, CallKind callKind, AddressKind? addressKind, bool box, out LocalDefinition tempOpt, out bool mightEscapeTemporaryRefs) + void emitReceiver(BoundCall call, CallKind callKind, AddressKind? addressKind, bool box, out LocalDefinition tempOpt) { var receiver = call.ReceiverOpt; var receiverType = receiver.Type; tempOpt = null; - mightEscapeTemporaryRefs = MightEscapeTemporaryRefs(call, used: useKind != UseKind.Unused, receiverAddressKind: addressKind); if (addressKind is null) { @@ -1950,18 +1954,12 @@ void emitReceiver(BoundCall call, CallKind callKind, AddressKind? addressKind, b Debug.Assert(!receiverType.IsVerifierReference()); tempOpt = EmitReceiverRef(receiver, addressKind.GetValueOrDefault()); - if (mightEscapeTemporaryRefs) - { - AddBlockTemp(tempOpt); - tempOpt = null; - } - emitGenericReceiverCloneIfNecessary(call, callKind, ref tempOpt); } } [MethodImpl(MethodImplOptions.NoInlining)] - void emitArgumentsAndCallEpilogue(BoundCall call, CallKind callKind, UseKind useKind, bool mightEscapeTemporaryRefs) + void emitArgumentsAndCallEpilogue(BoundCall call, CallKind callKind, UseKind useKind) { var method = call.Method; var receiver = call.ReceiverOpt; @@ -2015,11 +2013,7 @@ void emitArgumentsAndCallEpilogue(BoundCall call, CallKind callKind, UseKind use } var arguments = call.Arguments; - EmitArguments( - arguments, - method.Parameters, - call.ArgumentRefKindsOpt, - mightEscapeTemporaryRefs: mightEscapeTemporaryRefs); + EmitArguments(arguments, method.Parameters, call.ArgumentRefKindsOpt); int stackBehavior = GetCallStackBehavior(method, arguments); switch (callKind) { @@ -2474,11 +2468,13 @@ private void EmitObjectCreationExpression(BoundObjectCreationExpression expressi } // none of the above cases, so just create an instance - EmitArguments( - expression.Arguments, - constructor.Parameters, - expression.ArgumentRefKindsOpt, - mightEscapeTemporaryRefs: MightEscapeTemporaryRefs(expression, used)); + + var countBefore = _builder.LocalSlotManager.StartScopeOfTrackingAddressedLocals(); + + EmitArguments(expression.Arguments, constructor.Parameters, expression.ArgumentRefKindsOpt); + + _builder.LocalSlotManager.EndScopeOfTrackingAddressedLocals(countBefore, + MightEscapeTemporaryRefs(expression, used)); var stackAdjustment = GetObjCreationStackBehavior(expression); _builder.EmitOpCode(ILOpCode.Newobj, stackAdjustment); @@ -2736,11 +2732,14 @@ private bool TryInPlaceCtorCall(BoundExpression target, BoundObjectCreationExpre Debug.Assert(temp == null, "in-place ctor target should not create temps"); var constructor = objCreation.Constructor; - EmitArguments( - objCreation.Arguments, - constructor.Parameters, - objCreation.ArgumentRefKindsOpt, - mightEscapeTemporaryRefs: MightEscapeTemporaryRefs(objCreation, used)); + + var countBefore = _builder.LocalSlotManager.StartScopeOfTrackingAddressedLocals(); + + EmitArguments(objCreation.Arguments, constructor.Parameters, objCreation.ArgumentRefKindsOpt); + + _builder.LocalSlotManager.EndScopeOfTrackingAddressedLocals(countBefore, + MightEscapeTemporaryRefs(objCreation, used)); + // -2 to adjust for consumed target address and not produced value. var stackAdjustment = GetObjCreationStackBehavior(objCreation) - 2; _builder.EmitOpCode(ILOpCode.Call, stackAdjustment); @@ -4063,11 +4062,14 @@ private void EmitCalli(BoundFunctionPointerInvocation ptrInvocation, UseKind use } FunctionPointerMethodSymbol method = ptrInvocation.FunctionPointer.Signature; - EmitArguments( - ptrInvocation.Arguments, - method.Parameters, - ptrInvocation.ArgumentRefKindsOpt, - mightEscapeTemporaryRefs: MightEscapeTemporaryRefs(ptrInvocation, used: useKind != UseKind.Unused)); + + var countBefore = _builder.LocalSlotManager.StartScopeOfTrackingAddressedLocals(); + + EmitArguments(ptrInvocation.Arguments, method.Parameters, ptrInvocation.ArgumentRefKindsOpt); + + _builder.LocalSlotManager.EndScopeOfTrackingAddressedLocals(countBefore, + MightEscapeTemporaryRefs(ptrInvocation, used: useKind != UseKind.Unused)); + var stackBehavior = GetCallStackBehavior(ptrInvocation.FunctionPointer.Signature, ptrInvocation.Arguments); if (temp is object) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs index 7ce2e394ec670..dc7de6cae6fca 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs @@ -10401,15 +10401,17 @@ ref struct R(in int x) targetFramework: TargetFramework.Net70, verify: Verification.FailsPEVerify) .VerifyDiagnostics() - // Two int and two R temps are enough. + // Two int and two R temps would be enough, but the emit layer currently does not take blocks into account. .VerifyIL("", """ { - // Code size 88 (0x58) + // Code size 90 (0x5a) .maxstack 2 .locals init (R V_0, //r2 int V_1, int V_2, - R V_3) //r2 + R V_3, //r2 + int V_4, + int V_5) IL_0000: ldc.i4.s 111 IL_0002: stloc.1 IL_0003: ldloca.s V_1 @@ -10426,21 +10428,21 @@ .locals init (R V_0, //r2 IL_0024: ldind.i4 IL_0025: call "void Program.<
$>g__Report|0_0(int, int)" IL_002a: ldc.i4 0x14d - IL_002f: stloc.1 - IL_0030: ldloca.s V_1 - IL_0032: newobj "R..ctor(in int)" - IL_0037: ldc.i4 0x1bc - IL_003c: stloc.2 - IL_003d: ldloca.s V_2 - IL_003f: newobj "R..ctor(in int)" - IL_0044: stloc.3 - IL_0045: ldfld "ref readonly int R.F" - IL_004a: ldind.i4 - IL_004b: ldloc.3 - IL_004c: ldfld "ref readonly int R.F" - IL_0051: ldind.i4 - IL_0052: call "void Program.<
$>g__Report|0_0(int, int)" - IL_0057: ret + IL_002f: stloc.s V_4 + IL_0031: ldloca.s V_4 + IL_0033: newobj "R..ctor(in int)" + IL_0038: ldc.i4 0x1bc + IL_003d: stloc.s V_5 + IL_003f: ldloca.s V_5 + IL_0041: newobj "R..ctor(in int)" + IL_0046: stloc.3 + IL_0047: ldfld "ref readonly int R.F" + IL_004c: ldind.i4 + IL_004d: ldloc.3 + IL_004e: ldfld "ref readonly int R.F" + IL_0053: ldind.i4 + IL_0054: call "void Program.<
$>g__Report|0_0(int, int)" + IL_0059: ret } """); } @@ -10837,23 +10839,24 @@ public void RefTemp_CannotEscape_ViaOutDiscard() """; CompileAndVerify(source) .VerifyDiagnostics() - // One int temp would be enough. But currently the emit layer does not see the argument is a discard. + // One int and one R temp would be enough. But currently the emit layer does not see the argument is a discard. .VerifyIL("", """ { // Code size 28 (0x1c) .maxstack 2 .locals init (R V_0, int V_1, - int V_2) + R V_2, + int V_3) IL_0000: ldc.i4.s 111 IL_0002: stloc.1 IL_0003: ldloca.s V_1 IL_0005: ldloca.s V_0 IL_0007: call "void Program.<
$>g__M|0_0(in int, out R)" IL_000c: ldc.i4 0xde - IL_0011: stloc.2 - IL_0012: ldloca.s V_2 - IL_0014: ldloca.s V_0 + IL_0011: stloc.3 + IL_0012: ldloca.s V_3 + IL_0014: ldloca.s V_2 IL_0016: call "void Program.<
$>g__M|0_0(in int, out R)" IL_001b: ret } diff --git a/src/Compilers/Core/Portable/CodeGen/ILBuilderEmit.cs b/src/Compilers/Core/Portable/CodeGen/ILBuilderEmit.cs index 03b1f616d8dda..41e100dac9787 100644 --- a/src/Compilers/Core/Portable/CodeGen/ILBuilderEmit.cs +++ b/src/Compilers/Core/Portable/CodeGen/ILBuilderEmit.cs @@ -489,6 +489,8 @@ internal void EmitLocalStore(LocalDefinition local) internal void EmitLocalAddress(LocalDefinition local) { + LocalSlotManager.AddAddressedLocal(local); + if (local.IsReference) { EmitLocalLoad(local); diff --git a/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs b/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs index 7f02bdc0ce639..b591ce23ecd2f 100644 --- a/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs +++ b/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs @@ -67,6 +67,12 @@ public override bool Equals(object? obj) // pool of free slots partitioned by their signature. private KeyedStack? _freeSlots; + // these locals cannot be added to "FreeSlots" + private HashSet? _nonReusableLocals; + + // locals whose address has been taken + private ArrayBuilder? _addressedLocals; + // all locals in order private ArrayBuilder? _lazyAllLocals; @@ -246,7 +252,59 @@ private LocalDefinition DeclareLocalImpl( internal void FreeSlot(LocalDefinition slot) { Debug.Assert(slot.Name == null); - FreeSlots.Push(new LocalSignature(slot.Type, slot.Constraints), slot); + + if (_nonReusableLocals?.Contains(slot) != true) + { + FreeSlots.Push(new LocalSignature(slot.Type, slot.Constraints), slot); + } + } + + internal void MarkLocalAsNotReusable(LocalDefinition slot) + { + _nonReusableLocals ??= new HashSet(ReferenceEqualityComparer.Instance); + _nonReusableLocals.Add(slot); + } + + internal int? StartScopeOfTrackingAddressedLocals() + { + if (_addressedLocals is null) + { + _addressedLocals = ArrayBuilder.GetInstance(); + return null; + } + + return _addressedLocals.Count; + } + + internal void AddAddressedLocal(LocalDefinition localDef) + { + if (localDef != null) + { + _addressedLocals?.Add(localDef); + } + } + + internal void EndScopeOfTrackingAddressedLocals(int? countBefore, bool markAsNotReusable) + { + Debug.Assert(_addressedLocals != null); + + if (markAsNotReusable) + { + for (var i = countBefore ?? 0; i < _addressedLocals.Count; i++) + { + MarkLocalAsNotReusable(_addressedLocals[i]); + } + } + + if (countBefore is { } c) + { + _addressedLocals.Count = c; + } + else + { + _addressedLocals.Free(); + _addressedLocals = null; + } } public ImmutableArray LocalsInOrder() From 1d6b786402e723443b4fc386aad2f02eacf452b0 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Wed, 27 Nov 2024 14:24:58 +0100 Subject: [PATCH 05/22] Update tests --- .../LocalStateTracingTests.cs | 7 +- .../Semantics/CollectionExpressionTests.cs | 148 +++++++++--------- .../Test/Emit3/Semantics/InlineArrayTests.cs | 7 +- .../Semantic/Semantics/InterpolationTests.cs | 125 +++++++-------- .../RawInterpolationTests_Handler.cs | 68 ++++---- 5 files changed, 183 insertions(+), 172 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit2/Emit/LocalStateTracing/LocalStateTracingTests.cs b/src/Compilers/CSharp/Test/Emit2/Emit/LocalStateTracing/LocalStateTracingTests.cs index 899fc155bff31..2e38d531a45ff 100644 --- a/src/Compilers/CSharp/Test/Emit2/Emit/LocalStateTracing/LocalStateTracingTests.cs +++ b/src/Compilers/CSharp/Test/Emit2/Emit/LocalStateTracing/LocalStateTracingTests.cs @@ -1628,7 +1628,8 @@ public static void F(S p) .maxstack 4 .locals init (Microsoft.CodeAnalysis.Runtime.LocalStoreTracker V_0, S V_1, //x - S V_2) + S V_2, + S V_3) // sequence point: IL_0000: ldtoken ""void S.F(S)"" IL_0005: call ""Microsoft.CodeAnalysis.Runtime.LocalStoreTracker Microsoft.CodeAnalysis.Runtime.LocalStoreTracker.LogMethodEntry(int)"" @@ -1659,8 +1660,8 @@ .locals init (Microsoft.CodeAnalysis.Runtime.LocalStoreTracker V_0, IL_0043: ldarg.0 IL_0044: dup IL_0045: stloc.1 - IL_0046: stloc.2 - IL_0047: ldloca.s V_2 + IL_0046: stloc.3 + IL_0047: ldloca.s V_3 IL_0049: constrained. ""S"" IL_004f: callvirt ""string object.ToString()"" IL_0054: ldc.i4.1 diff --git a/src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests.cs b/src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests.cs index 1e75957eea944..194794a8dd457 100644 --- a/src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests.cs +++ b/src/Compilers/CSharp/Test/Emit3/Semantics/CollectionExpressionTests.cs @@ -11713,7 +11713,8 @@ .locals init (int[] V_0, int V_10, int V_11, int V_12, - int V_13) + int V_13, + System.Span V_14) IL_0000: ldstr "A" IL_0005: ldc.i4.2 IL_0006: newarr "int" @@ -11830,8 +11831,8 @@ .locals init (int[] V_0, IL_0105: ldloca.s V_6 IL_0107: ldloc.s V_4 IL_0109: newobj "System.Span..ctor(int[])" - IL_010e: stloc.s V_7 - IL_0110: ldloca.s V_7 + IL_010e: stloc.s V_14 + IL_0110: ldloca.s V_14 IL_0112: ldloc.3 IL_0113: ldloca.s V_6 IL_0115: call "int System.Span.Length.get" @@ -33877,7 +33878,8 @@ .locals init (System.ReadOnlySpan V_0, //li1 C[] V_6, System.ReadOnlySpan.Enumerator V_7, D V_8, - System.ReadOnlySpan V_9) + System.ReadOnlySpan.Enumerator V_9, + System.ReadOnlySpan V_10) IL_0000: newobj "D..ctor()" IL_0005: stloc.1 IL_0006: ldloca.s V_1 @@ -33920,9 +33922,9 @@ .locals init (System.ReadOnlySpan V_0, //li1 IL_0061: brtrue.s IL_0043 IL_0063: ldloca.s V_4 IL_0065: call "System.ReadOnlySpan.Enumerator System.ReadOnlySpan.GetEnumerator()" - IL_006a: stloc.s V_7 + IL_006a: stloc.s V_9 IL_006c: br.s IL_0085 - IL_006e: ldloca.s V_7 + IL_006e: ldloca.s V_9 IL_0070: call "ref readonly D System.ReadOnlySpan.Enumerator.Current.get" IL_0075: ldind.ref IL_0076: stloc.s V_8 @@ -33934,13 +33936,13 @@ .locals init (System.ReadOnlySpan V_0, //li1 IL_0081: ldc.i4.1 IL_0082: add IL_0083: stloc.s V_5 - IL_0085: ldloca.s V_7 + IL_0085: ldloca.s V_9 IL_0087: call "bool System.ReadOnlySpan.Enumerator.MoveNext()" IL_008c: brtrue.s IL_006e IL_008e: ldloc.s V_6 IL_0090: call "System.ReadOnlySpan System.ReadOnlySpan.op_Implicit(C[])" - IL_0095: stloc.s V_9 - IL_0097: ldloca.s V_9 + IL_0095: stloc.s V_10 + IL_0097: ldloca.s V_10 IL_0099: call "void CollectionExtensions.Report(in System.ReadOnlySpan)" IL_009e: ret } @@ -35650,13 +35652,14 @@ static void Main() verifier.VerifyDiagnostics(); verifier.VerifyIL("C.Main", """ { - // Code size 146 (0x92) + // Code size 147 (0x93) .maxstack 4 .locals init (int V_0, System.Span V_1, int V_2, System.Collections.Generic.List V_3, - System.Span V_4) + System.Span V_4, + System.Span V_5) IL_0000: ldc.i4.3 IL_0001: stloc.0 IL_0002: ldloc.0 @@ -35706,27 +35709,27 @@ .locals init (int V_0, IL_0055: call "void System.Runtime.InteropServices.CollectionsMarshal.SetCount(System.Collections.Generic.List, int)" IL_005a: ldloc.3 IL_005b: call "System.Span System.Runtime.InteropServices.CollectionsMarshal.AsSpan(System.Collections.Generic.List)" - IL_0060: stloc.1 - IL_0061: ldc.i4.0 - IL_0062: stloc.0 - IL_0063: call "System.Span System.Runtime.InteropServices.CollectionsMarshal.AsSpan(System.Collections.Generic.List)" - IL_0068: stloc.s V_4 - IL_006a: ldloca.s V_4 - IL_006c: ldloca.s V_1 - IL_006e: ldloc.0 - IL_006f: ldloca.s V_4 - IL_0071: call "int System.Span.Length.get" - IL_0076: call "System.Span System.Span.Slice(int, int)" - IL_007b: call "void System.Span.CopyTo(System.Span)" - IL_0080: ldloc.0 - IL_0081: ldloca.s V_4 - IL_0083: call "int System.Span.Length.get" - IL_0088: add - IL_0089: stloc.0 - IL_008a: ldloc.3 - IL_008b: ldc.i4.0 - IL_008c: call "void CollectionExtensions.Report(object, bool)" - IL_0091: ret + IL_0060: stloc.s V_4 + IL_0062: ldc.i4.0 + IL_0063: stloc.0 + IL_0064: call "System.Span System.Runtime.InteropServices.CollectionsMarshal.AsSpan(System.Collections.Generic.List)" + IL_0069: stloc.s V_5 + IL_006b: ldloca.s V_5 + IL_006d: ldloca.s V_4 + IL_006f: ldloc.0 + IL_0070: ldloca.s V_5 + IL_0072: call "int System.Span.Length.get" + IL_0077: call "System.Span System.Span.Slice(int, int)" + IL_007c: call "void System.Span.CopyTo(System.Span)" + IL_0081: ldloc.0 + IL_0082: ldloca.s V_5 + IL_0084: call "int System.Span.Length.get" + IL_0089: add + IL_008a: stloc.0 + IL_008b: ldloc.3 + IL_008c: ldc.i4.0 + IL_008d: call "void CollectionExtensions.Report(object, bool)" + IL_0092: ret } """); } @@ -37417,15 +37420,16 @@ static void Main() // Ideally we'd like to be able to use *both* something like AddRange, *and* AsSpan/CopyTo/etc. while building the same target collection verifier.VerifyIL("C.Main", """ { - // Code size 181 (0xb5) + // Code size 182 (0xb6) .maxstack 3 .locals init (int V_0, System.Span V_1, int V_2, System.Collections.Generic.ICollection V_3, System.Collections.Generic.List V_4, - System.Collections.Generic.IEnumerator V_5, - int V_6) + System.Span V_5, + System.Collections.Generic.IEnumerator V_6, + int V_7) IL_0000: ldc.i4.3 IL_0001: stloc.0 IL_0002: ldloc.0 @@ -37478,49 +37482,49 @@ .locals init (int V_0, IL_005a: call "void System.Runtime.InteropServices.CollectionsMarshal.SetCount(System.Collections.Generic.List, int)" IL_005f: ldloc.s V_4 IL_0061: call "System.Span System.Runtime.InteropServices.CollectionsMarshal.AsSpan(System.Collections.Generic.List)" - IL_0066: stloc.1 - IL_0067: ldc.i4.0 - IL_0068: stloc.0 - IL_0069: ldloc.3 - IL_006a: callvirt "System.Collections.Generic.IEnumerator System.Collections.Generic.IEnumerable.GetEnumerator()" - IL_006f: stloc.s V_5 + IL_0066: stloc.s V_5 + IL_0068: ldc.i4.0 + IL_0069: stloc.0 + IL_006a: ldloc.3 + IL_006b: callvirt "System.Collections.Generic.IEnumerator System.Collections.Generic.IEnumerable.GetEnumerator()" + IL_0070: stloc.s V_6 .try { - IL_0071: br.s IL_008b - IL_0073: ldloc.s V_5 - IL_0075: callvirt "int System.Collections.Generic.IEnumerator.Current.get" - IL_007a: stloc.s V_6 - IL_007c: ldloca.s V_1 - IL_007e: ldloc.0 - IL_007f: call "ref int System.Span.this[int].get" - IL_0084: ldloc.s V_6 - IL_0086: stind.i4 - IL_0087: ldloc.0 - IL_0088: ldc.i4.1 - IL_0089: add - IL_008a: stloc.0 - IL_008b: ldloc.s V_5 - IL_008d: callvirt "bool System.Collections.IEnumerator.MoveNext()" - IL_0092: brtrue.s IL_0073 - IL_0094: leave.s IL_00a2 + IL_0072: br.s IL_008c + IL_0074: ldloc.s V_6 + IL_0076: callvirt "int System.Collections.Generic.IEnumerator.Current.get" + IL_007b: stloc.s V_7 + IL_007d: ldloca.s V_5 + IL_007f: ldloc.0 + IL_0080: call "ref int System.Span.this[int].get" + IL_0085: ldloc.s V_7 + IL_0087: stind.i4 + IL_0088: ldloc.0 + IL_0089: ldc.i4.1 + IL_008a: add + IL_008b: stloc.0 + IL_008c: ldloc.s V_6 + IL_008e: callvirt "bool System.Collections.IEnumerator.MoveNext()" + IL_0093: brtrue.s IL_0074 + IL_0095: leave.s IL_00a3 } finally { - IL_0096: ldloc.s V_5 - IL_0098: brfalse.s IL_00a1 - IL_009a: ldloc.s V_5 - IL_009c: callvirt "void System.IDisposable.Dispose()" - IL_00a1: endfinally + IL_0097: ldloc.s V_6 + IL_0099: brfalse.s IL_00a2 + IL_009b: ldloc.s V_6 + IL_009d: callvirt "void System.IDisposable.Dispose()" + IL_00a2: endfinally } - IL_00a2: ldloca.s V_1 - IL_00a4: ldloc.0 - IL_00a5: call "ref int System.Span.this[int].get" - IL_00aa: ldc.i4.4 - IL_00ab: stind.i4 - IL_00ac: ldloc.s V_4 - IL_00ae: ldc.i4.0 - IL_00af: call "void CollectionExtensions.Report(object, bool)" - IL_00b4: ret + IL_00a3: ldloca.s V_5 + IL_00a5: ldloc.0 + IL_00a6: call "ref int System.Span.this[int].get" + IL_00ab: ldc.i4.4 + IL_00ac: stind.i4 + IL_00ad: ldloc.s V_4 + IL_00af: ldc.i4.0 + IL_00b0: call "void CollectionExtensions.Report(object, bool)" + IL_00b5: ret } """); } diff --git a/src/Compilers/CSharp/Test/Emit3/Semantics/InlineArrayTests.cs b/src/Compilers/CSharp/Test/Emit3/Semantics/InlineArrayTests.cs index ac248191c94d6..eaf7d51046c3f 100644 --- a/src/Compilers/CSharp/Test/Emit3/Semantics/InlineArrayTests.cs +++ b/src/Compilers/CSharp/Test/Emit3/Semantics/InlineArrayTests.cs @@ -8818,7 +8818,8 @@ static int M4(in int x, Buffer10 y) // Code size 32 (0x20) .maxstack 2 .locals init (Buffer10 V_0, - int V_1) + int V_1, + Buffer10 V_2) IL_0000: call ""Buffer10 Program.M3()"" IL_0005: stloc.0 IL_0006: ldloca.s V_0 @@ -8826,9 +8827,9 @@ .locals init (Buffer10 V_0, IL_000d: ldind.i4 IL_000e: stloc.1 IL_000f: ldloca.s V_1 - IL_0011: ldloca.s V_0 + IL_0011: ldloca.s V_2 IL_0013: initobj ""Buffer10"" - IL_0019: ldloc.0 + IL_0019: ldloc.2 IL_001a: call ""int Program.M4(in int, Buffer10)"" IL_001f: ret } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs index 6d829cd9dcdf2..209b3d719896f 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs @@ -3822,7 +3822,8 @@ public void NestedInterpolatedStrings_02(string expression) .maxstack 4 .locals init (int V_0, //i1 int V_1, //i2 - System.Runtime.CompilerServices.DefaultInterpolatedStringHandler V_2) + System.Runtime.CompilerServices.DefaultInterpolatedStringHandler V_2, + System.Runtime.CompilerServices.DefaultInterpolatedStringHandler V_3) IL_0000: ldc.i4.1 IL_0001: stloc.0 IL_0002: ldc.i4.2 @@ -3836,14 +3837,14 @@ .locals init (int V_0, //i1 IL_0010: call ""void System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.AppendFormatted(int)"" IL_0015: ldloca.s V_2 IL_0017: call ""string System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.ToStringAndClear()"" - IL_001c: ldloca.s V_2 + IL_001c: ldloca.s V_3 IL_001e: ldc.i4.0 IL_001f: ldc.i4.1 IL_0020: call ""System.Runtime.CompilerServices.DefaultInterpolatedStringHandler..ctor(int, int)"" - IL_0025: ldloca.s V_2 + IL_0025: ldloca.s V_3 IL_0027: ldloc.1 IL_0028: call ""void System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.AppendFormatted(int)"" - IL_002d: ldloca.s V_2 + IL_002d: ldloca.s V_3 IL_002f: call ""string System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.ToStringAndClear()"" IL_0034: call ""string string.Concat(string, string)"" IL_0039: call ""void System.Console.WriteLine(string)"" @@ -10139,14 +10140,15 @@ Creating DummyHandler from StructLogger#2 verifier.VerifyIL("", @" { - // Code size 175 (0xaf) + // Code size 177 (0xb1) .maxstack 4 .locals init (int V_0, //i StructLogger V_1, //s StructLogger V_2, DummyHandler V_3, bool V_4, - StructLogger& V_5) + StructLogger& V_5, + DummyHandler V_6) IL_0000: ldc.i4.0 IL_0001: stloc.0 IL_0002: ldloca.s V_2 @@ -10195,32 +10197,32 @@ .locals init (int V_0, //i IL_0064: ldobj ""StructLogger"" IL_0069: ldloca.s V_4 IL_006b: newobj ""DummyHandler..ctor(int, int, StructLogger, out bool)"" - IL_0070: stloc.3 - IL_0071: ldloc.s V_4 - IL_0073: brfalse.s IL_008f - IL_0075: ldloca.s V_3 - IL_0077: ldstr ""log:"" - IL_007c: call ""void DummyHandler.AppendLiteral(string)"" - IL_0081: nop - IL_0082: ldloca.s V_3 - IL_0084: ldloc.0 - IL_0085: dup - IL_0086: ldc.i4.1 - IL_0087: add - IL_0088: stloc.0 - IL_0089: call ""void DummyHandler.AppendFormatted(int)"" - IL_008e: nop - IL_008f: ldloc.s V_5 - IL_0091: ldloc.3 - IL_0092: call ""void StructLogger.Log(DummyHandler)"" - IL_0097: nop - IL_0098: ldstr ""(2) i={0}"" - IL_009d: ldloc.0 - IL_009e: box ""int"" - IL_00a3: call ""string string.Format(string, object)"" - IL_00a8: call ""void System.Console.WriteLine(string)"" - IL_00ad: nop - IL_00ae: ret + IL_0070: stloc.s V_6 + IL_0072: ldloc.s V_4 + IL_0074: brfalse.s IL_0090 + IL_0076: ldloca.s V_6 + IL_0078: ldstr ""log:"" + IL_007d: call ""void DummyHandler.AppendLiteral(string)"" + IL_0082: nop + IL_0083: ldloca.s V_6 + IL_0085: ldloc.0 + IL_0086: dup + IL_0087: ldc.i4.1 + IL_0088: add + IL_0089: stloc.0 + IL_008a: call ""void DummyHandler.AppendFormatted(int)"" + IL_008f: nop + IL_0090: ldloc.s V_5 + IL_0092: ldloc.s V_6 + IL_0094: call ""void StructLogger.Log(DummyHandler)"" + IL_0099: nop + IL_009a: ldstr ""(2) i={0}"" + IL_009f: ldloc.0 + IL_00a0: box ""int"" + IL_00a5: call ""string string.Format(string, object)"" + IL_00aa: call ""void System.Console.WriteLine(string)"" + IL_00af: nop + IL_00b0: ret } "); } @@ -16788,13 +16790,14 @@ public void ParenthesizedAdditiveExpression_04() verifier.VerifyIL("Program.<
$>d__0.System.Runtime.CompilerServices.IAsyncStateMachine.MoveNext()", @" { - // Code size 244 (0xf4) + // Code size 245 (0xf5) .maxstack 4 .locals init (int V_0, int V_1, System.Runtime.CompilerServices.TaskAwaiter V_2, System.Runtime.CompilerServices.DefaultInterpolatedStringHandler V_3, - System.Exception V_4) + System.Runtime.CompilerServices.DefaultInterpolatedStringHandler V_4, + System.Exception V_5) IL_0000: ldarg.0 IL_0001: ldfld ""int Program.<
$>d__0.<>1__state"" IL_0006: stloc.0 @@ -16827,7 +16830,7 @@ .locals init (int V_0, IL_0042: ldloca.s V_2 IL_0044: ldarg.0 IL_0045: call ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.AwaitUnsafeOnCompleted, Program.<
$>d__0>(ref System.Runtime.CompilerServices.TaskAwaiter, ref Program.<
$>d__0)"" - IL_004a: leave IL_00f3 + IL_004a: leave IL_00f4 IL_004f: ldarg.0 IL_0050: ldfld ""System.Runtime.CompilerServices.TaskAwaiter Program.<
$>d__0.<>u__1"" IL_0055: stloc.2 @@ -16859,36 +16862,36 @@ .locals init (int V_0, IL_009f: ldc.i4.0 IL_00a0: ldc.i4.1 IL_00a1: newobj ""System.Runtime.CompilerServices.DefaultInterpolatedStringHandler..ctor(int, int)"" - IL_00a6: stloc.3 - IL_00a7: ldloca.s V_3 - IL_00a9: ldarg.0 - IL_00aa: ldfld ""int Program.<
$>d__0.5__3"" - IL_00af: call ""void System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.AppendFormatted(int)"" - IL_00b4: ldloca.s V_3 - IL_00b6: call ""string System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.ToStringAndClear()"" - IL_00bb: call ""string string.Concat(string, string, string)"" - IL_00c0: call ""void System.Console.WriteLine(string)"" - IL_00c5: leave.s IL_00e0 + IL_00a6: stloc.s V_4 + IL_00a8: ldloca.s V_4 + IL_00aa: ldarg.0 + IL_00ab: ldfld ""int Program.<
$>d__0.5__3"" + IL_00b0: call ""void System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.AppendFormatted(int)"" + IL_00b5: ldloca.s V_4 + IL_00b7: call ""string System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.ToStringAndClear()"" + IL_00bc: call ""string string.Concat(string, string, string)"" + IL_00c1: call ""void System.Console.WriteLine(string)"" + IL_00c6: leave.s IL_00e1 } catch System.Exception { - IL_00c7: stloc.s V_4 - IL_00c9: ldarg.0 - IL_00ca: ldc.i4.s -2 - IL_00cc: stfld ""int Program.<
$>d__0.<>1__state"" - IL_00d1: ldarg.0 - IL_00d2: ldflda ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder Program.<
$>d__0.<>t__builder"" - IL_00d7: ldloc.s V_4 - IL_00d9: call ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetException(System.Exception)"" - IL_00de: leave.s IL_00f3 + IL_00c8: stloc.s V_5 + IL_00ca: ldarg.0 + IL_00cb: ldc.i4.s -2 + IL_00cd: stfld ""int Program.<
$>d__0.<>1__state"" + IL_00d2: ldarg.0 + IL_00d3: ldflda ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder Program.<
$>d__0.<>t__builder"" + IL_00d8: ldloc.s V_5 + IL_00da: call ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetException(System.Exception)"" + IL_00df: leave.s IL_00f4 } - IL_00e0: ldarg.0 - IL_00e1: ldc.i4.s -2 - IL_00e3: stfld ""int Program.<
$>d__0.<>1__state"" - IL_00e8: ldarg.0 - IL_00e9: ldflda ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder Program.<
$>d__0.<>t__builder"" - IL_00ee: call ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetResult()"" - IL_00f3: ret + IL_00e1: ldarg.0 + IL_00e2: ldc.i4.s -2 + IL_00e4: stfld ""int Program.<
$>d__0.<>1__state"" + IL_00e9: ldarg.0 + IL_00ea: ldflda ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder Program.<
$>d__0.<>t__builder"" + IL_00ef: call ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetResult()"" + IL_00f4: ret }"); } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RawInterpolationTests_Handler.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RawInterpolationTests_Handler.cs index bde5a43a8840b..a03d5a3b05814 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RawInterpolationTests_Handler.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RawInterpolationTests_Handler.cs @@ -2223,7 +2223,8 @@ public void NestedInterpolatedStrings_02(string expression) .maxstack 4 .locals init (int V_0, //i1 int V_1, //i2 - System.Runtime.CompilerServices.DefaultInterpolatedStringHandler V_2) + System.Runtime.CompilerServices.DefaultInterpolatedStringHandler V_2, + System.Runtime.CompilerServices.DefaultInterpolatedStringHandler V_3) IL_0000: ldc.i4.1 IL_0001: stloc.0 IL_0002: ldc.i4.2 @@ -2237,14 +2238,14 @@ .locals init (int V_0, //i1 IL_0010: call ""void System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.AppendFormatted(int)"" IL_0015: ldloca.s V_2 IL_0017: call ""string System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.ToStringAndClear()"" - IL_001c: ldloca.s V_2 + IL_001c: ldloca.s V_3 IL_001e: ldc.i4.0 IL_001f: ldc.i4.1 IL_0020: call ""System.Runtime.CompilerServices.DefaultInterpolatedStringHandler..ctor(int, int)"" - IL_0025: ldloca.s V_2 + IL_0025: ldloca.s V_3 IL_0027: ldloc.1 IL_0028: call ""void System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.AppendFormatted(int)"" - IL_002d: ldloca.s V_2 + IL_002d: ldloca.s V_3 IL_002f: call ""string System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.ToStringAndClear()"" IL_0034: call ""string string.Concat(string, string)"" IL_0039: call ""void System.Console.WriteLine(string)"" @@ -12220,13 +12221,14 @@ public void ParenthesizedAdditiveExpression_04() verifier.VerifyIL("Program.<
$>d__0.System.Runtime.CompilerServices.IAsyncStateMachine.MoveNext()", @" { - // Code size 244 (0xf4) + // Code size 245 (0xf5) .maxstack 4 .locals init (int V_0, int V_1, System.Runtime.CompilerServices.TaskAwaiter V_2, System.Runtime.CompilerServices.DefaultInterpolatedStringHandler V_3, - System.Exception V_4) + System.Runtime.CompilerServices.DefaultInterpolatedStringHandler V_4, + System.Exception V_5) IL_0000: ldarg.0 IL_0001: ldfld ""int Program.<
$>d__0.<>1__state"" IL_0006: stloc.0 @@ -12259,7 +12261,7 @@ .locals init (int V_0, IL_0042: ldloca.s V_2 IL_0044: ldarg.0 IL_0045: call ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.AwaitUnsafeOnCompleted, Program.<
$>d__0>(ref System.Runtime.CompilerServices.TaskAwaiter, ref Program.<
$>d__0)"" - IL_004a: leave IL_00f3 + IL_004a: leave IL_00f4 IL_004f: ldarg.0 IL_0050: ldfld ""System.Runtime.CompilerServices.TaskAwaiter Program.<
$>d__0.<>u__1"" IL_0055: stloc.2 @@ -12291,36 +12293,36 @@ .locals init (int V_0, IL_009f: ldc.i4.0 IL_00a0: ldc.i4.1 IL_00a1: newobj ""System.Runtime.CompilerServices.DefaultInterpolatedStringHandler..ctor(int, int)"" - IL_00a6: stloc.3 - IL_00a7: ldloca.s V_3 - IL_00a9: ldarg.0 - IL_00aa: ldfld ""int Program.<
$>d__0.5__3"" - IL_00af: call ""void System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.AppendFormatted(int)"" - IL_00b4: ldloca.s V_3 - IL_00b6: call ""string System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.ToStringAndClear()"" - IL_00bb: call ""string string.Concat(string, string, string)"" - IL_00c0: call ""void System.Console.WriteLine(string)"" - IL_00c5: leave.s IL_00e0 + IL_00a6: stloc.s V_4 + IL_00a8: ldloca.s V_4 + IL_00aa: ldarg.0 + IL_00ab: ldfld ""int Program.<
$>d__0.5__3"" + IL_00b0: call ""void System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.AppendFormatted(int)"" + IL_00b5: ldloca.s V_4 + IL_00b7: call ""string System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.ToStringAndClear()"" + IL_00bc: call ""string string.Concat(string, string, string)"" + IL_00c1: call ""void System.Console.WriteLine(string)"" + IL_00c6: leave.s IL_00e1 } catch System.Exception { - IL_00c7: stloc.s V_4 - IL_00c9: ldarg.0 - IL_00ca: ldc.i4.s -2 - IL_00cc: stfld ""int Program.<
$>d__0.<>1__state"" - IL_00d1: ldarg.0 - IL_00d2: ldflda ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder Program.<
$>d__0.<>t__builder"" - IL_00d7: ldloc.s V_4 - IL_00d9: call ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetException(System.Exception)"" - IL_00de: leave.s IL_00f3 + IL_00c8: stloc.s V_5 + IL_00ca: ldarg.0 + IL_00cb: ldc.i4.s -2 + IL_00cd: stfld ""int Program.<
$>d__0.<>1__state"" + IL_00d2: ldarg.0 + IL_00d3: ldflda ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder Program.<
$>d__0.<>t__builder"" + IL_00d8: ldloc.s V_5 + IL_00da: call ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetException(System.Exception)"" + IL_00df: leave.s IL_00f4 } - IL_00e0: ldarg.0 - IL_00e1: ldc.i4.s -2 - IL_00e3: stfld ""int Program.<
$>d__0.<>1__state"" - IL_00e8: ldarg.0 - IL_00e9: ldflda ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder Program.<
$>d__0.<>t__builder"" - IL_00ee: call ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetResult()"" - IL_00f3: ret + IL_00e1: ldarg.0 + IL_00e2: ldc.i4.s -2 + IL_00e4: stfld ""int Program.<
$>d__0.<>1__state"" + IL_00e9: ldarg.0 + IL_00ea: ldflda ""System.Runtime.CompilerServices.AsyncTaskMethodBuilder Program.<
$>d__0.<>t__builder"" + IL_00ef: call ""void System.Runtime.CompilerServices.AsyncTaskMethodBuilder.SetResult()"" + IL_00f4: ret }"); } From 6ccc2ad56cb302db9201a463d8150a4996835a70 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Wed, 27 Nov 2024 15:41:03 +0100 Subject: [PATCH 06/22] Inline a function --- .../Core/Portable/CodeGen/LocalSlotManager.cs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs b/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs index b591ce23ecd2f..c8d8436219b06 100644 --- a/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs +++ b/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs @@ -259,12 +259,6 @@ internal void FreeSlot(LocalDefinition slot) } } - internal void MarkLocalAsNotReusable(LocalDefinition slot) - { - _nonReusableLocals ??= new HashSet(ReferenceEqualityComparer.Instance); - _nonReusableLocals.Add(slot); - } - internal int? StartScopeOfTrackingAddressedLocals() { if (_addressedLocals is null) @@ -288,11 +282,12 @@ internal void EndScopeOfTrackingAddressedLocals(int? countBefore, bool markAsNot { Debug.Assert(_addressedLocals != null); - if (markAsNotReusable) + if (markAsNotReusable && (countBefore ?? 0) < _addressedLocals.Count) { + _nonReusableLocals ??= new HashSet(ReferenceEqualityComparer.Instance); for (var i = countBefore ?? 0; i < _addressedLocals.Count; i++) { - MarkLocalAsNotReusable(_addressedLocals[i]); + _nonReusableLocals.Add(_addressedLocals[i]); } } From 402376f18081e7b46b41974a168ca6a592307996 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Thu, 5 Dec 2024 10:10:50 +0100 Subject: [PATCH 07/22] Revert unrelated change --- .../CSharp/Portable/Lowering/SyntheticBoundNodeFactory.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Lowering/SyntheticBoundNodeFactory.cs b/src/Compilers/CSharp/Portable/Lowering/SyntheticBoundNodeFactory.cs index 79c3f09cd7b3f..637a3a8da0e85 100644 --- a/src/Compilers/CSharp/Portable/Lowering/SyntheticBoundNodeFactory.cs +++ b/src/Compilers/CSharp/Portable/Lowering/SyntheticBoundNodeFactory.cs @@ -887,7 +887,7 @@ public BoundCall Call(BoundExpression? receiver, MethodSymbol method, ImmutableA return new BoundCall( Syntax, receiver, initialBindingReceiverIsSubjectToCloning: ThreeState.Unknown, method, args, argumentNamesOpt: default(ImmutableArray), argumentRefKindsOpt: refKinds, isDelegateCall: false, expanded: false, invokedAsExtensionMethod: false, - argsToParamsOpt: default, defaultArguments: default(BitVector), resultKind: LookupResultKind.Viable, type: method.ReturnType) + argsToParamsOpt: ImmutableArray.Empty, defaultArguments: default(BitVector), resultKind: LookupResultKind.Viable, type: method.ReturnType) { WasCompilerGenerated = true }; } From 689cc3ebf232e8462083b7a1164d7095c0956473 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Thu, 5 Dec 2024 10:25:42 +0100 Subject: [PATCH 08/22] Remove non-reusable locals after checking for them --- src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs b/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs index c8d8436219b06..80527af8f4f68 100644 --- a/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs +++ b/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs @@ -161,7 +161,8 @@ internal LocalDefinition GetLocal(ILocalSymbolInternal symbol) internal void FreeLocal(ILocalSymbolInternal symbol) { var slot = GetLocal(symbol); - LocalMap.Remove(symbol); + var removed = LocalMap.Remove(symbol); + Debug.Assert(removed, $"Attempted to free '{symbol}' more than once."); FreeSlot(slot); } @@ -253,7 +254,7 @@ internal void FreeSlot(LocalDefinition slot) { Debug.Assert(slot.Name == null); - if (_nonReusableLocals?.Contains(slot) != true) + if (_nonReusableLocals?.Remove(slot) != true) { FreeSlots.Push(new LocalSignature(slot.Type, slot.Constraints), slot); } From 0428e095c1ce00bb5d3324980c6acebb379e261b Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Thu, 5 Dec 2024 10:32:36 +0100 Subject: [PATCH 09/22] Keep ref count for addressed locals list --- .../Core/Portable/CodeGen/LocalSlotManager.cs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs b/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs index 80527af8f4f68..5da44a5bb8a9f 100644 --- a/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs +++ b/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs @@ -72,6 +72,7 @@ public override bool Equals(object? obj) // locals whose address has been taken private ArrayBuilder? _addressedLocals; + private int _addressedLocalScopes; // all locals in order private ArrayBuilder? _lazyAllLocals; @@ -260,14 +261,11 @@ internal void FreeSlot(LocalDefinition slot) } } - internal int? StartScopeOfTrackingAddressedLocals() + internal int StartScopeOfTrackingAddressedLocals() { - if (_addressedLocals is null) - { - _addressedLocals = ArrayBuilder.GetInstance(); - return null; - } - + Debug.Assert((_addressedLocals == null) == (_addressedLocalScopes == 0)); + _addressedLocals ??= ArrayBuilder.GetInstance(); + _addressedLocalScopes++; return _addressedLocals.Count; } @@ -279,25 +277,27 @@ internal void AddAddressedLocal(LocalDefinition localDef) } } - internal void EndScopeOfTrackingAddressedLocals(int? countBefore, bool markAsNotReusable) + internal void EndScopeOfTrackingAddressedLocals(int countBefore, bool markAsNotReusable) { Debug.Assert(_addressedLocals != null); - if (markAsNotReusable && (countBefore ?? 0) < _addressedLocals.Count) + if (markAsNotReusable && countBefore < _addressedLocals.Count) { _nonReusableLocals ??= new HashSet(ReferenceEqualityComparer.Instance); - for (var i = countBefore ?? 0; i < _addressedLocals.Count; i++) + for (var i = countBefore; i < _addressedLocals.Count; i++) { _nonReusableLocals.Add(_addressedLocals[i]); } } - if (countBefore is { } c) + _addressedLocalScopes--; + if (_addressedLocalScopes > 0) { - _addressedLocals.Count = c; + _addressedLocals.Count = countBefore; } else { + Debug.Assert(_addressedLocalScopes == 0); _addressedLocals.Free(); _addressedLocals = null; } From 1558dbcb35d518eeb9031a1b7257fe9327578d92 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Fri, 6 Dec 2024 13:19:03 +0100 Subject: [PATCH 10/22] Extend an assert --- src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs b/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs index 5da44a5bb8a9f..7d4f2d6b2c44a 100644 --- a/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs +++ b/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs @@ -297,7 +297,7 @@ internal void EndScopeOfTrackingAddressedLocals(int countBefore, bool markAsNotR } else { - Debug.Assert(_addressedLocalScopes == 0); + Debug.Assert(_addressedLocalScopes == 0 && countBefore == 0); _addressedLocals.Free(); _addressedLocals = null; } From 910d71129a59b0b37c4bc28d8f69bdc3f85b2876 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Wed, 11 Dec 2024 17:17:53 +0100 Subject: [PATCH 11/22] Add high-level comment to MightEscapeTemporaryRefs --- .../Portable/CodeGen/CodeGenerator_RefSafety.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs index fffa98613ab7b..a4aeeabacbc3b 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs @@ -62,14 +62,18 @@ private static bool MightEscapeTemporaryRefs( { Debug.Assert(receiverAddressKind is null || receiverType is not null); - // number of outputs that can capture references + // We check the signature of the method, counting potential `ref` sources and destinations + // to determine whether a `ref` can be captured by the method. + // The emit layer then uses this information to avoid reusing temporaries that are passed by ref to such methods. + + // number of outputs that can capture `ref`s int refTargets = 0; - // number of inputs that can contain references + // number of inputs that can contain `ref`s int refSources = 0; if (used && (returnRefKind != RefKind.None || returnType.IsRefLikeOrAllowsRefLikeType())) { - // If returning by reference or returning a ref struct, the result might capture references. + // If returning by ref or returning a ref struct, the result might capture `ref`s. refTargets++; } @@ -120,7 +124,7 @@ private static bool MightEscapeTemporaryRefs( static bool shouldReturnTrue(int writableRefs, int readableRefs) { - // If there is at least one output and at least one input, a reference can be captured. + // If there is at least one output and at least one input, a `ref` can be captured. return writableRefs > 0 && readableRefs > 0; } } From 583015d3fdc5ed23d2e8db6d1946455a605735ad Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Thu, 12 Dec 2024 18:06:30 +0100 Subject: [PATCH 12/22] Filter non-reusable locals --- src/Compilers/Core/Portable/CodeGen/ILBuilderEmit.cs | 2 +- src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Compilers/Core/Portable/CodeGen/ILBuilderEmit.cs b/src/Compilers/Core/Portable/CodeGen/ILBuilderEmit.cs index 41e100dac9787..cdb479b37214f 100644 --- a/src/Compilers/Core/Portable/CodeGen/ILBuilderEmit.cs +++ b/src/Compilers/Core/Portable/CodeGen/ILBuilderEmit.cs @@ -489,7 +489,7 @@ internal void EmitLocalStore(LocalDefinition local) internal void EmitLocalAddress(LocalDefinition local) { - LocalSlotManager.AddAddressedLocal(local); + LocalSlotManager.AddAddressedLocal(local, _optimizations); if (local.IsReference) { diff --git a/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs b/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs index 7d4f2d6b2c44a..0c6e4ad945e40 100644 --- a/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs +++ b/src/Compilers/Core/Portable/CodeGen/LocalSlotManager.cs @@ -70,7 +70,7 @@ public override bool Equals(object? obj) // these locals cannot be added to "FreeSlots" private HashSet? _nonReusableLocals; - // locals whose address has been taken + // locals whose address has been taken; excludes non-reusable local kinds private ArrayBuilder? _addressedLocals; private int _addressedLocalScopes; @@ -269,9 +269,11 @@ internal int StartScopeOfTrackingAddressedLocals() return _addressedLocals.Count; } - internal void AddAddressedLocal(LocalDefinition localDef) + internal void AddAddressedLocal(LocalDefinition localDef, OptimizationLevel optimizations) { - if (localDef != null) + // No need to add non-reusable local kinds to `_addressedLocals` because that list + // only contains locals with reusable kinds to mark them as actually non-reusable. + if (localDef != null && localDef.SymbolOpt?.SynthesizedKind.IsSlotReusable(optimizations) != false) { _addressedLocals?.Add(localDef); } From 45dcef5d9c072db87f93c4bca368f94e2a3a2766 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Tue, 4 Feb 2025 20:51:11 +0100 Subject: [PATCH 13/22] Simplify `int`s to `bool`s --- .../CodeGen/CodeGenerator_RefSafety.cs | 33 ++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs index a4aeeabacbc3b..e150cc4fb3459 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs @@ -66,15 +66,16 @@ private static bool MightEscapeTemporaryRefs( // to determine whether a `ref` can be captured by the method. // The emit layer then uses this information to avoid reusing temporaries that are passed by ref to such methods. - // number of outputs that can capture `ref`s - int refTargets = 0; - // number of inputs that can contain `ref`s - int refSources = 0; + // whether we have any outputs that can capture `ref`s + bool anyRefTargets = false; + // whether we have any inputs that can contain `ref`s + bool anyRefSources = false; + // NOTE: If there is at least one output and at least one input, a `ref` can be captured. if (used && (returnRefKind != RefKind.None || returnType.IsRefLikeOrAllowsRefLikeType())) { // If returning by ref or returning a ref struct, the result might capture `ref`s. - refTargets++; + anyRefTargets = true; } if (receiverType is not null) @@ -82,19 +83,19 @@ private static bool MightEscapeTemporaryRefs( receiverScope ??= ScopedKind.None; if (receiverType.IsRefLikeOrAllowsRefLikeType() && receiverScope != ScopedKind.ScopedValue) { - refSources++; + anyRefSources = true; if (!isReceiverReadOnly && !receiverType.IsReadOnly) { - refTargets++; + anyRefTargets = true; } } else if (receiverAddressKind != null && receiverScope == ScopedKind.None) { - refSources++; + anyRefSources = true; } } - if (shouldReturnTrue(refTargets, refSources)) + if (anyRefTargets && anyRefSources) { return true; } @@ -103,29 +104,23 @@ private static bool MightEscapeTemporaryRefs( { if (parameter.Type.IsRefLikeOrAllowsRefLikeType() && parameter.EffectiveScope != ScopedKind.ScopedValue) { - refSources++; + anyRefSources = true; if (!parameter.Type.IsReadOnly && parameter.RefKind.IsWritableReference()) { - refTargets++; + anyRefTargets = true; } } else if (parameter.RefKind != RefKind.None && parameter.EffectiveScope == ScopedKind.None) { - refSources++; + anyRefSources = true; } - if (shouldReturnTrue(refTargets, refSources)) + if (anyRefTargets && anyRefSources) { return true; } } return false; - - static bool shouldReturnTrue(int writableRefs, int readableRefs) - { - // If there is at least one output and at least one input, a `ref` can be captured. - return writableRefs > 0 && readableRefs > 0; - } } } From 0fca5366bb6e8679a601ea6afaa2533179a971ce Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Tue, 4 Feb 2025 20:55:21 +0100 Subject: [PATCH 14/22] Replace coalesce with an assert --- .../CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs index e150cc4fb3459..6bdeff07a928b 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs @@ -80,7 +80,7 @@ private static bool MightEscapeTemporaryRefs( if (receiverType is not null) { - receiverScope ??= ScopedKind.None; + Debug.Assert(receiverScope != null); if (receiverType.IsRefLikeOrAllowsRefLikeType() && receiverScope != ScopedKind.ScopedValue) { anyRefSources = true; From 0f304607082d3264795c4f660479cf0ac33f30fa Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Wed, 5 Feb 2025 09:58:58 +0100 Subject: [PATCH 15/22] Mark nested calls as always used --- src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs b/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs index b8c42239e352c..e57a0b6a6ce06 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs @@ -1766,7 +1766,7 @@ private void EmitInstanceCallExpression(BoundCall call, UseKind useKind) _builder.LocalSlotManager.EndScopeOfTrackingAddressedLocals(countBefore, MightEscapeTemporaryRefs( call, - used: useKind != UseKind.Unused, + used: true, receiverAddressKind: receiverUseKind != UseKind.UsedAsAddress ? null : addressKind)); countBefore = _builder.LocalSlotManager.StartScopeOfTrackingAddressedLocals(); From cd7f4bcf7613569bd80032a80174752e3a9dacb7 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Wed, 5 Feb 2025 11:33:58 +0100 Subject: [PATCH 16/22] Fix this parameter of nint methods --- .../Portable/Symbols/NativeIntegerTypeSymbol.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/Compilers/CSharp/Portable/Symbols/NativeIntegerTypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/NativeIntegerTypeSymbol.cs index 4bf3c91ad7608..ec77ce06eebc3 100644 --- a/src/Compilers/CSharp/Portable/Symbols/NativeIntegerTypeSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/NativeIntegerTypeSymbol.cs @@ -363,6 +363,20 @@ public override ImmutableArray Parameters } } + internal override bool TryGetThisParameter(out ParameterSymbol? thisParameter) + { + if (UnderlyingMethod.TryGetThisParameter(out ParameterSymbol? underlyingThisParameter)) + { + thisParameter = underlyingThisParameter != null + ? new ThisParameterSymbol(this, _container) + : null; + return true; + } + + thisParameter = null; + return false; + } + public override ImmutableArray ExplicitInterfaceImplementations => ImmutableArray.Empty; public override ImmutableArray RefCustomModifiers => UnderlyingMethod.RefCustomModifiers; From 50accc56e10684a1b5d63395cc8b51bf8ba4f643 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Wed, 5 Feb 2025 16:56:18 +0100 Subject: [PATCH 17/22] Test chained call --- .../Semantic/Semantics/RefEscapingTests.cs | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs index dc7de6cae6fca..de5e7c3769bf4 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs @@ -10891,6 +10891,46 @@ ref struct R .VerifyDiagnostics(); } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_Escapes_InstanceMethod_Chained() + { + var source = """ + using System.Diagnostics.CodeAnalysis; + + scoped var r1 = new R(); + scoped var r2 = new R(); + new C().Set(111).To(ref r1); + new C().Set(222).To(ref r2); + Report(r1.F, r2.F); + + static void Report(int x, int y) => System.Console.WriteLine($"{x} {y}"); + + class C + { + public R Set([UnscopedRef] in int x) + { + var r = new R(); + r.F = ref x; + return r; + } + } + + ref struct R + { + public ref readonly int F; + public void To(ref R r) + { + r.F = ref F; + } + } + """; + CompileAndVerify(source, + expectedOutput: RefFieldTests.IncludeExpectedOutput("111 222"), + targetFramework: TargetFramework.Net70, + verify: Verification.Fails) + .VerifyDiagnostics(); + } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] public void RefTemp_Escapes_FunctionPointer() { From 6718681b4e256e60bb7e36dd1619102e5346bd70 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Thu, 6 Feb 2025 11:46:11 +0100 Subject: [PATCH 18/22] Simplify by using this parameter symbol --- .../CodeGen/CodeGenerator_RefSafety.cs | 55 ++++++------------- 1 file changed, 18 insertions(+), 37 deletions(-) diff --git a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs index 6bdeff07a928b..f346366a3ee0e 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs @@ -16,10 +16,8 @@ private static bool MightEscapeTemporaryRefs(BoundCall node, bool used, AddressK used: used, returnType: node.Type, returnRefKind: node.Method.RefKind, - receiverType: !node.Method.RequiresInstanceReceiver ? null : node.ReceiverOpt?.Type, - receiverScope: node.Method.TryGetThisParameter(out var thisParameter) ? thisParameter?.EffectiveScope : null, + thisParameterSymbol: node.Method.TryGetThisParameter(out var thisParameter) ? thisParameter : null, receiverAddressKind: receiverAddressKind, - isReceiverReadOnly: node.Method.IsEffectivelyReadOnly, parameters: node.Method.Parameters); } @@ -29,10 +27,8 @@ private static bool MightEscapeTemporaryRefs(BoundObjectCreationExpression node, used: used, returnType: node.Type, returnRefKind: RefKind.None, - receiverType: null, - receiverScope: null, + thisParameterSymbol: null, receiverAddressKind: null, - isReceiverReadOnly: false, parameters: node.Constructor.Parameters); } @@ -43,10 +39,8 @@ private static bool MightEscapeTemporaryRefs(BoundFunctionPointerInvocation node used: used, returnType: node.Type, returnRefKind: method.RefKind, - receiverType: null, - receiverScope: null, + thisParameterSymbol: null, receiverAddressKind: null, - isReceiverReadOnly: false, parameters: method.Parameters); } @@ -54,13 +48,11 @@ private static bool MightEscapeTemporaryRefs( bool used, TypeSymbol returnType, RefKind returnRefKind, - TypeSymbol? receiverType, - ScopedKind? receiverScope, + ParameterSymbol? thisParameterSymbol, AddressKind? receiverAddressKind, - bool isReceiverReadOnly, ImmutableArray parameters) { - Debug.Assert(receiverAddressKind is null || receiverType is not null); + Debug.Assert(receiverAddressKind is null || thisParameterSymbol is not null); // We check the signature of the method, counting potential `ref` sources and destinations // to determine whether a `ref` can be captured by the method. @@ -70,7 +62,6 @@ private static bool MightEscapeTemporaryRefs( bool anyRefTargets = false; // whether we have any inputs that can contain `ref`s bool anyRefSources = false; - // NOTE: If there is at least one output and at least one input, a `ref` can be captured. if (used && (returnRefKind != RefKind.None || returnType.IsRefLikeOrAllowsRefLikeType())) { @@ -78,29 +69,23 @@ private static bool MightEscapeTemporaryRefs( anyRefTargets = true; } - if (receiverType is not null) + if (thisParameterSymbol is not null && processParameter(thisParameterSymbol, anyRefSources: ref anyRefSources, anyRefTargets: ref anyRefTargets)) { - Debug.Assert(receiverScope != null); - if (receiverType.IsRefLikeOrAllowsRefLikeType() && receiverScope != ScopedKind.ScopedValue) - { - anyRefSources = true; - if (!isReceiverReadOnly && !receiverType.IsReadOnly) - { - anyRefTargets = true; - } - } - else if (receiverAddressKind != null && receiverScope == ScopedKind.None) - { - anyRefSources = true; - } + return true; } - if (anyRefTargets && anyRefSources) + foreach (var parameter in parameters) { - return true; + if (processParameter(parameter, anyRefSources: ref anyRefSources, anyRefTargets: ref anyRefTargets)) + { + return true; + } } - foreach (var parameter in parameters) + return false; + + // Returns true if we can return 'true' early. + static bool processParameter(ParameterSymbol parameter, ref bool anyRefSources, ref bool anyRefTargets) { if (parameter.Type.IsRefLikeOrAllowsRefLikeType() && parameter.EffectiveScope != ScopedKind.ScopedValue) { @@ -115,12 +100,8 @@ private static bool MightEscapeTemporaryRefs( anyRefSources = true; } - if (anyRefTargets && anyRefSources) - { - return true; - } + // If there is at least one output and at least one input, a `ref` can be captured. + return anyRefTargets && anyRefSources; } - - return false; } } From d01890fa9a83494422d2eec10bf028df68c1d342 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Thu, 6 Feb 2025 16:59:24 +0100 Subject: [PATCH 19/22] Fix readonly targets --- .../CodeGen/CodeGenerator_RefSafety.cs | 24 ++- .../Semantic/Semantics/RefEscapingTests.cs | 189 +++++++++++++++++- 2 files changed, 204 insertions(+), 9 deletions(-) diff --git a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs index f346366a3ee0e..6aff057f98e4b 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs @@ -10,6 +10,7 @@ namespace Microsoft.CodeAnalysis.CSharp.CodeGen; internal partial class CodeGenerator { + /// private static bool MightEscapeTemporaryRefs(BoundCall node, bool used, AddressKind? receiverAddressKind) { return MightEscapeTemporaryRefs( @@ -21,6 +22,7 @@ private static bool MightEscapeTemporaryRefs(BoundCall node, bool used, AddressK parameters: node.Method.Parameters); } + /// private static bool MightEscapeTemporaryRefs(BoundObjectCreationExpression node, bool used) { return MightEscapeTemporaryRefs( @@ -32,6 +34,7 @@ private static bool MightEscapeTemporaryRefs(BoundObjectCreationExpression node, parameters: node.Constructor.Parameters); } + /// private static bool MightEscapeTemporaryRefs(BoundFunctionPointerInvocation node, bool used) { FunctionPointerMethodSymbol method = node.FunctionPointer.Signature; @@ -44,6 +47,21 @@ private static bool MightEscapeTemporaryRefs(BoundFunctionPointerInvocation node parameters: method.Parameters); } + /// + /// Determines whether a 'ref' can be captured by the call (considering only its signature). + /// + /// + /// + /// The emit layer consults this to avoid reusing temporaries that are passed by ref to such methods. + /// + /// + /// This is a heuristic which might have false positives, i.e., + /// it might not recognize that a call cannot capture a 'ref' + /// even if the binding-time ref safety analysis would recognize that. + /// That is fine, the IL will be correct, albeit less optimal (it might use more temps). + /// But we still want some heuristic to avoid regressing IL of common safe cases. + /// + /// private static bool MightEscapeTemporaryRefs( bool used, TypeSymbol returnType, @@ -54,10 +72,6 @@ private static bool MightEscapeTemporaryRefs( { Debug.Assert(receiverAddressKind is null || thisParameterSymbol is not null); - // We check the signature of the method, counting potential `ref` sources and destinations - // to determine whether a `ref` can be captured by the method. - // The emit layer then uses this information to avoid reusing temporaries that are passed by ref to such methods. - // whether we have any outputs that can capture `ref`s bool anyRefTargets = false; // whether we have any inputs that can contain `ref`s @@ -90,7 +104,7 @@ static bool processParameter(ParameterSymbol parameter, ref bool anyRefSources, if (parameter.Type.IsRefLikeOrAllowsRefLikeType() && parameter.EffectiveScope != ScopedKind.ScopedValue) { anyRefSources = true; - if (!parameter.Type.IsReadOnly && parameter.RefKind.IsWritableReference()) + if (parameter.RefKind.IsWritableReference()) { anyRefTargets = true; } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs index de5e7c3769bf4..e394c61d4c28d 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RefEscapingTests.cs @@ -10801,22 +10801,126 @@ ref struct R .VerifyDiagnostics(); } + [Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_Escapes_ViaOutParameter( + [CombinatorialValues("", "scoped")] string scoped, + [CombinatorialValues("", "readonly")] string ro) + { + var source = $$""" + scoped R r1, r2; + M(111, out r1); + M(222, out r2); + Report(r1.F, r2.F); + + static void Report(int x, int y) => System.Console.WriteLine($"{x} {y}"); + + static void M(in int x, {{scoped}} out R r) => r = new R(x); + + {{ro}} ref struct R(in int x) + { + public {{ro}} ref readonly int F = ref x; + } + """; + CompileAndVerify(source, + expectedOutput: RefFieldTests.IncludeExpectedOutput("111 222"), + targetFramework: TargetFramework.Net70, + verify: Verification.FailsPEVerify) + .VerifyDiagnostics(); + } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] - public void RefTemp_Escapes_ViaOutParameter() + public void RefTemp_CannotEscape_ViaOutParameter() { var source = """ scoped R r1, r2; M(111, out r1); M(222, out r2); + + static void M(scoped in int x, scoped out R r) { } + + ref struct R; + """; + CompileAndVerify(source) + .VerifyDiagnostics() + // One int temp would be enough, but currently the scope difference between input/output is not recognized by the heuristic. + .VerifyIL("", """ + { + // Code size 28 (0x1c) + .maxstack 2 + .locals init (R V_0, //r1 + R V_1, //r2 + int V_2, + int V_3) + IL_0000: ldc.i4.s 111 + IL_0002: stloc.2 + IL_0003: ldloca.s V_2 + IL_0005: ldloca.s V_0 + IL_0007: call "void Program.<
$>g__M|0_0(scoped in int, out R)" + IL_000c: ldc.i4 0xde + IL_0011: stloc.3 + IL_0012: ldloca.s V_3 + IL_0014: ldloca.s V_1 + IL_0016: call "void Program.<
$>g__M|0_0(scoped in int, out R)" + IL_001b: ret + } + """); + } + + [Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_Escapes_ViaRefParameter( + [CombinatorialValues("", "scoped")] string scoped) + { + var source = $$""" + using System.Diagnostics.CodeAnalysis; + + scoped var r1 = new R(); + scoped var r2 = new R(); + M(111, ref r1); + M(222, ref r2); Report(r1.F, r2.F); static void Report(int x, int y) => System.Console.WriteLine($"{x} {y}"); - static void M(in int x, out R r) => r = new R(x); + static void M([UnscopedRef] in int x, {{scoped}} ref R r) + { + r.F = ref x; + } - ref struct R(in int x) + ref struct R { - public ref readonly int F = ref x; + public ref readonly int F; + } + """; + CompileAndVerify(source, + expectedOutput: RefFieldTests.IncludeExpectedOutput("111 222"), + targetFramework: TargetFramework.Net70, + verify: Verification.FailsPEVerify) + .VerifyDiagnostics(); + } + + [Theory, CombinatorialData, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_Escapes_ViaRefParameter_Readonly( + [CombinatorialValues("", "scoped")] string scoped) + { + var source = $$""" + using System.Diagnostics.CodeAnalysis; + + scoped var r1 = new R(); + scoped var r2 = new R(); + M(111, ref r1); + M(222, ref r2); + Report(r1.F, r2.F); + + static void Report(int x, int y) => System.Console.WriteLine($"{x} {y}"); + + static void M([UnscopedRef] in int x, {{scoped}} ref R r) + { + r = new R(in x); + } + + readonly ref struct R(ref readonly int x) + { + public readonly ref readonly int F = ref x; } """; CompileAndVerify(source, @@ -10826,6 +10930,48 @@ ref struct R(in int x) .VerifyDiagnostics(); } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_CannotEscape_ViaRefParameter() + { + var source = """ + scoped R r1 = new(), r2 = new(); + M(111, ref r1); + M(222, ref r2); + + static void M(in int x, ref R r) { } + + ref struct R; + """; + CompileAndVerify(source) + .VerifyDiagnostics() + // One int temp would be enough, but currently the scope difference between input/output is not recognized by the heuristic. + .VerifyIL("", """ + { + // Code size 44 (0x2c) + .maxstack 2 + .locals init (R V_0, //r1 + R V_1, //r2 + int V_2, + int V_3) + IL_0000: ldloca.s V_0 + IL_0002: initobj "R" + IL_0008: ldloca.s V_1 + IL_000a: initobj "R" + IL_0010: ldc.i4.s 111 + IL_0012: stloc.2 + IL_0013: ldloca.s V_2 + IL_0015: ldloca.s V_0 + IL_0017: call "void Program.<
$>g__M|0_0(in int, ref R)" + IL_001c: ldc.i4 0xde + IL_0021: stloc.3 + IL_0022: ldloca.s V_3 + IL_0024: ldloca.s V_1 + IL_0026: call "void Program.<
$>g__M|0_0(in int, ref R)" + IL_002b: ret + } + """); + } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] public void RefTemp_CannotEscape_ViaOutDiscard() { @@ -10863,6 +11009,41 @@ .locals init (R V_0, """); } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] + public void RefTemp_CannotEscape_Scoped() + { + var source = """ + var r1 = new R(111); + var r2 = new R(222); + + ref struct R + { + public R(scoped in int x) { } + } + """; + CompileAndVerify(source) + .VerifyDiagnostics() + // One int temp is enough. + .VerifyIL("", """ + { + // Code size 26 (0x1a) + .maxstack 1 + .locals init (int V_0) + IL_0000: ldc.i4.s 111 + IL_0002: stloc.0 + IL_0003: ldloca.s V_0 + IL_0005: newobj "R..ctor(scoped in int)" + IL_000a: pop + IL_000b: ldc.i4 0xde + IL_0010: stloc.0 + IL_0011: ldloca.s V_0 + IL_0013: newobj "R..ctor(scoped in int)" + IL_0018: pop + IL_0019: ret + } + """); + } + [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/67435")] public void RefTemp_Escapes_InstanceMethod() { From 42ffdc8c398ac09fa8d7fec3bb44c3fd9106fa8b Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Thu, 6 Feb 2025 17:53:01 +0100 Subject: [PATCH 20/22] Fixup a test --- .../Emit2/Emit/LocalStateTracing/LocalStateTracingTests.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit2/Emit/LocalStateTracing/LocalStateTracingTests.cs b/src/Compilers/CSharp/Test/Emit2/Emit/LocalStateTracing/LocalStateTracingTests.cs index 2e38d531a45ff..899fc155bff31 100644 --- a/src/Compilers/CSharp/Test/Emit2/Emit/LocalStateTracing/LocalStateTracingTests.cs +++ b/src/Compilers/CSharp/Test/Emit2/Emit/LocalStateTracing/LocalStateTracingTests.cs @@ -1628,8 +1628,7 @@ public static void F(S p) .maxstack 4 .locals init (Microsoft.CodeAnalysis.Runtime.LocalStoreTracker V_0, S V_1, //x - S V_2, - S V_3) + S V_2) // sequence point: IL_0000: ldtoken ""void S.F(S)"" IL_0005: call ""Microsoft.CodeAnalysis.Runtime.LocalStoreTracker Microsoft.CodeAnalysis.Runtime.LocalStoreTracker.LogMethodEntry(int)"" @@ -1660,8 +1659,8 @@ .locals init (Microsoft.CodeAnalysis.Runtime.LocalStoreTracker V_0, IL_0043: ldarg.0 IL_0044: dup IL_0045: stloc.1 - IL_0046: stloc.3 - IL_0047: ldloca.s V_3 + IL_0046: stloc.2 + IL_0047: ldloca.s V_2 IL_0049: constrained. ""S"" IL_004f: callvirt ""string object.ToString()"" IL_0054: ldc.i4.1 From 0669bce7354af717a7507ddfe5a5b3690aa0df59 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Tue, 18 Mar 2025 12:45:43 +0100 Subject: [PATCH 21/22] Remove an unused parameter --- .../Portable/CodeGen/CodeGenerator_RefSafety.cs | 15 ++++----------- .../CSharp/Portable/CodeGen/EmitExpression.cs | 12 +++--------- 2 files changed, 7 insertions(+), 20 deletions(-) diff --git a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs index 6aff057f98e4b..07ea26d9bc8f0 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs @@ -3,26 +3,24 @@ // See the LICENSE file in the project root for more information. using System.Collections.Immutable; -using System.Diagnostics; using Microsoft.CodeAnalysis.CSharp.Symbols; namespace Microsoft.CodeAnalysis.CSharp.CodeGen; internal partial class CodeGenerator { - /// - private static bool MightEscapeTemporaryRefs(BoundCall node, bool used, AddressKind? receiverAddressKind) + /// + private static bool MightEscapeTemporaryRefs(BoundCall node, bool used) { return MightEscapeTemporaryRefs( used: used, returnType: node.Type, returnRefKind: node.Method.RefKind, thisParameterSymbol: node.Method.TryGetThisParameter(out var thisParameter) ? thisParameter : null, - receiverAddressKind: receiverAddressKind, parameters: node.Method.Parameters); } - /// + /// private static bool MightEscapeTemporaryRefs(BoundObjectCreationExpression node, bool used) { return MightEscapeTemporaryRefs( @@ -30,11 +28,10 @@ private static bool MightEscapeTemporaryRefs(BoundObjectCreationExpression node, returnType: node.Type, returnRefKind: RefKind.None, thisParameterSymbol: null, - receiverAddressKind: null, parameters: node.Constructor.Parameters); } - /// + /// private static bool MightEscapeTemporaryRefs(BoundFunctionPointerInvocation node, bool used) { FunctionPointerMethodSymbol method = node.FunctionPointer.Signature; @@ -43,7 +40,6 @@ private static bool MightEscapeTemporaryRefs(BoundFunctionPointerInvocation node returnType: node.Type, returnRefKind: method.RefKind, thisParameterSymbol: null, - receiverAddressKind: null, parameters: method.Parameters); } @@ -67,11 +63,8 @@ private static bool MightEscapeTemporaryRefs( TypeSymbol returnType, RefKind returnRefKind, ParameterSymbol? thisParameterSymbol, - AddressKind? receiverAddressKind, ImmutableArray parameters) { - Debug.Assert(receiverAddressKind is null || thisParameterSymbol is not null); - // whether we have any outputs that can capture `ref`s bool anyRefTargets = false; // whether we have any inputs that can contain `ref`s diff --git a/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs b/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs index e57a0b6a6ce06..b56a01fd28a36 100644 --- a/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs +++ b/src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs @@ -1665,7 +1665,7 @@ private void EmitStaticCallExpression(BoundCall call, UseKind useKind) EmitArguments(arguments, method.Parameters, call.ArgumentRefKindsOpt); _builder.LocalSlotManager.EndScopeOfTrackingAddressedLocals(countBefore, - MightEscapeTemporaryRefs(call, used: useKind != UseKind.Unused, receiverAddressKind: null)); + MightEscapeTemporaryRefs(call, used: useKind != UseKind.Unused)); int stackBehavior = GetCallStackBehavior(method, arguments); @@ -1764,10 +1764,7 @@ private void EmitInstanceCallExpression(BoundCall call, UseKind useKind) emitArgumentsAndCallEpilogue(call, callKind, receiverUseKind); - _builder.LocalSlotManager.EndScopeOfTrackingAddressedLocals(countBefore, MightEscapeTemporaryRefs( - call, - used: true, - receiverAddressKind: receiverUseKind != UseKind.UsedAsAddress ? null : addressKind)); + _builder.LocalSlotManager.EndScopeOfTrackingAddressedLocals(countBefore, MightEscapeTemporaryRefs(call, used: true)); countBefore = _builder.LocalSlotManager.StartScopeOfTrackingAddressedLocals(); @@ -1832,10 +1829,7 @@ private void EmitInstanceCallExpression(BoundCall call, UseKind useKind) emitArgumentsAndCallEpilogue(call, callKind, useKind); - _builder.LocalSlotManager.EndScopeOfTrackingAddressedLocals(countBefore, MightEscapeTemporaryRefs( - call, - used: useKind != UseKind.Unused, - receiverAddressKind: useKind != UseKind.UsedAsAddress ? null : addressKind)); + _builder.LocalSlotManager.EndScopeOfTrackingAddressedLocals(countBefore, MightEscapeTemporaryRefs(call, used: useKind != UseKind.Unused)); FreeOptTemp(tempOpt); From bf95dba9087ec158e0e3f6cb1626d630681c8c38 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Thu, 17 Jul 2025 11:01:53 +0200 Subject: [PATCH 22/22] Fix nullability after merge --- src/Compilers/Core/Portable/CodeGen/ILBuilderEmit.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Compilers/Core/Portable/CodeGen/ILBuilderEmit.cs b/src/Compilers/Core/Portable/CodeGen/ILBuilderEmit.cs index 1dbf2f50b56cd..c883dca5b9869 100644 --- a/src/Compilers/Core/Portable/CodeGen/ILBuilderEmit.cs +++ b/src/Compilers/Core/Portable/CodeGen/ILBuilderEmit.cs @@ -486,6 +486,7 @@ internal void EmitLocalStore(LocalDefinition local) internal void EmitLocalAddress(LocalDefinition local) { + Debug.Assert(LocalSlotManager != null); LocalSlotManager.AddAddressedLocal(local, _optimizations); if (local.IsReference)