Skip to content

Commit 65637cc

Browse files
dotnet-policy-service[bot]dotnet-policy-service[bot]
authored
[automated] Merge branch 'main' => 'main-vs-deps' (#79435)
I detected changes in the main branch which have not been merged yet to main-vs-deps. I'm a robot and am configured to help you automatically keep main-vs-deps up to date, so I've opened this PR. This PR merges commits made on main by the following committers: * CyrusNajmabadi ## Instructions for merging from UI This PR will not be auto-merged. When pull request checks pass, complete this PR by creating a merge commit, *not* a squash or rebase commit. <img alt="merge button instructions" src="https://i.imgur.com/GepcNJV.png" width="300" /> If this repo does not allow creating merge commits from the GitHub UI, use command line instructions. ## Instructions for merging via command line Run these commands to merge this pull request from the command line. ``` sh git fetch git checkout main git pull --ff-only git checkout main-vs-deps git pull --ff-only git merge --no-ff main # If there are merge conflicts, resolve them and then run git merge --continue to complete the merge # Pushing the changes to the PR branch will re-trigger PR validation. git push https://github.com/dotnet/roslyn HEAD:merge/main-to-main-vs-deps ``` <details> <summary>or if you are using SSH</summary> ``` git push [email protected]:dotnet/roslyn HEAD:merge/main-to-main-vs-deps ``` </details> After PR checks are complete push the branch ``` git push ``` ## Instructions for resolving conflicts :warning: If there are merge conflicts, you will need to resolve them manually before merging. You can do this [using GitHub][resolve-github] or using the [command line][resolve-cli]. [resolve-github]: https://help.github.com/articles/resolving-a-merge-conflict-on-github/ [resolve-cli]: https://help.github.com/articles/resolving-a-merge-conflict-using-the-command-line/ ## Instructions for updating this pull request Contributors to this repo have permission update this pull request by pushing to the branch 'merge/main-to-main-vs-deps'. This can be done to resolve conflicts or make other changes to this pull request before it is merged. The provided examples assume that the remote is named 'origin'. If you have a different remote name, please replace 'origin' with the name of your remote. ``` git fetch git checkout -b merge/main-to-main-vs-deps origin/main-vs-deps git pull https://github.com/dotnet/roslyn merge/main-to-main-vs-deps (make changes) git commit -m "Updated PR with my changes" git push https://github.com/dotnet/roslyn HEAD:merge/main-to-main-vs-deps ``` <details> <summary>or if you are using SSH</summary> ``` git fetch git checkout -b merge/main-to-main-vs-deps origin/main-vs-deps git pull [email protected]:dotnet/roslyn merge/main-to-main-vs-deps (make changes) git commit -m "Updated PR with my changes" git push [email protected]:dotnet/roslyn HEAD:merge/main-to-main-vs-deps ``` </details> Contact .NET Core Engineering (dotnet/dnceng) if you have questions or issues. Also, if this PR was generated incorrectly, help us fix it. See https://github.com/dotnet/arcade/blob/main/.github/workflows/scripts/inter-branch-merge.ps1.
2 parents 4ff0763 + bf18293 commit 65637cc

File tree

32 files changed

+2180
-455
lines changed

32 files changed

+2180
-455
lines changed

‎src/Analyzers/CSharp/Tests/UseAutoProperty/UseAutoPropertyTests.cs‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
using System.Threading.Tasks;
66
using Microsoft.CodeAnalysis.CodeFixes;
7+
using Microsoft.CodeAnalysis.CodeStyle;
78
using Microsoft.CodeAnalysis.CSharp;
89
using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
910
using Microsoft.CodeAnalysis.CSharp.UseAutoProperty;
@@ -2999,4 +3000,62 @@ class Class
29993000
readonly ref int P => ref i;
30003001
}
30013002
""");
3003+
3004+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/77011")]
3005+
public Task TestRemoveThisIfPreferredCodeStyle()
3006+
=> TestInRegularAndScriptAsync(
3007+
"""
3008+
class C
3009+
{
3010+
[|private readonly string a;|]
3011+
3012+
public C(string a)
3013+
{
3014+
this.a = a;
3015+
}
3016+
3017+
public string A => a;
3018+
}
3019+
""",
3020+
"""
3021+
class C
3022+
{
3023+
public C(string a)
3024+
{
3025+
A = a;
3026+
}
3027+
3028+
public string A { get; }
3029+
}
3030+
""",
3031+
options: Option(CodeStyleOptions2.QualifyPropertyAccess, false, NotificationOption2.Error));
3032+
3033+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/77011")]
3034+
public Task TestKeepThisIfPreferredCodeStyle()
3035+
=> TestInRegularAndScriptAsync(
3036+
"""
3037+
class C
3038+
{
3039+
[|private readonly string a;|]
3040+
3041+
public C(string a)
3042+
{
3043+
this.a = a;
3044+
}
3045+
3046+
public string A => a;
3047+
}
3048+
""",
3049+
"""
3050+
class C
3051+
{
3052+
public C(string a)
3053+
{
3054+
this.A = a;
3055+
}
3056+
3057+
public string A { get; }
3058+
}
3059+
""",
3060+
options: Option(CodeStyleOptions2.QualifyPropertyAccess, true, NotificationOption2.Error));
30023061
}

‎src/Compilers/CSharp/Portable/CodeGen/CodeGenerator_RefSafety.cs‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System.Collections.Immutable;
6+
using Microsoft.CodeAnalysis.CSharp.Symbols;
7+
8+
namespace Microsoft.CodeAnalysis.CSharp.CodeGen;
9+
10+
internal partial class CodeGenerator
11+
{
12+
/// <inheritdoc cref="MightEscapeTemporaryRefs(bool, TypeSymbol, RefKind, ParameterSymbol?, ImmutableArray{ParameterSymbol})"/>
13+
private static bool MightEscapeTemporaryRefs(BoundCall node, bool used)
14+
{
15+
return MightEscapeTemporaryRefs(
16+
used: used,
17+
returnType: node.Type,
18+
returnRefKind: node.Method.RefKind,
19+
thisParameterSymbol: node.Method.TryGetThisParameter(out var thisParameter) ? thisParameter : null,
20+
parameters: node.Method.Parameters);
21+
}
22+
23+
/// <inheritdoc cref="MightEscapeTemporaryRefs(bool, TypeSymbol, RefKind, ParameterSymbol?, ImmutableArray{ParameterSymbol})"/>
24+
private static bool MightEscapeTemporaryRefs(BoundObjectCreationExpression node, bool used)
25+
{
26+
return MightEscapeTemporaryRefs(
27+
used: used,
28+
returnType: node.Type,
29+
returnRefKind: RefKind.None,
30+
thisParameterSymbol: null,
31+
parameters: node.Constructor.Parameters);
32+
}
33+
34+
/// <inheritdoc cref="MightEscapeTemporaryRefs(bool, TypeSymbol, RefKind, ParameterSymbol?, ImmutableArray{ParameterSymbol})"/>
35+
private static bool MightEscapeTemporaryRefs(BoundFunctionPointerInvocation node, bool used)
36+
{
37+
FunctionPointerMethodSymbol method = node.FunctionPointer.Signature;
38+
return MightEscapeTemporaryRefs(
39+
used: used,
40+
returnType: node.Type,
41+
returnRefKind: method.RefKind,
42+
thisParameterSymbol: null,
43+
parameters: method.Parameters);
44+
}
45+
46+
/// <summary>
47+
/// Determines whether a 'ref' can be captured by the call (considering only its signature).
48+
/// </summary>
49+
/// <remarks>
50+
/// <para>
51+
/// The emit layer consults this to avoid reusing temporaries that are passed by ref to such methods.
52+
/// </para>
53+
/// <para>
54+
/// This is a heuristic which might have false positives, i.e.,
55+
/// it might not recognize that a call cannot capture a 'ref'
56+
/// even if the binding-time ref safety analysis would recognize that.
57+
/// That is fine, the IL will be correct, albeit less optimal (it might use more temps).
58+
/// But we still want some heuristic to avoid regressing IL of common safe cases.
59+
/// </para>
60+
/// </remarks>
61+
private static bool MightEscapeTemporaryRefs(
62+
bool used,
63+
TypeSymbol returnType,
64+
RefKind returnRefKind,
65+
ParameterSymbol? thisParameterSymbol,
66+
ImmutableArray<ParameterSymbol> parameters)
67+
{
68+
// whether we have any outputs that can capture `ref`s
69+
bool anyRefTargets = false;
70+
// whether we have any inputs that can contain `ref`s
71+
bool anyRefSources = false;
72+
73+
if (used && (returnRefKind != RefKind.None || returnType.IsRefLikeOrAllowsRefLikeType()))
74+
{
75+
// If returning by ref or returning a ref struct, the result might capture `ref`s.
76+
anyRefTargets = true;
77+
}
78+
79+
if (thisParameterSymbol is not null && processParameter(thisParameterSymbol, anyRefSources: ref anyRefSources, anyRefTargets: ref anyRefTargets))
80+
{
81+
return true;
82+
}
83+
84+
foreach (var parameter in parameters)
85+
{
86+
if (processParameter(parameter, anyRefSources: ref anyRefSources, anyRefTargets: ref anyRefTargets))
87+
{
88+
return true;
89+
}
90+
}
91+
92+
return false;
93+
94+
// Returns true if we can return 'true' early.
95+
static bool processParameter(ParameterSymbol parameter, ref bool anyRefSources, ref bool anyRefTargets)
96+
{
97+
if (parameter.Type.IsRefLikeOrAllowsRefLikeType() && parameter.EffectiveScope != ScopedKind.ScopedValue)
98+
{
99+
anyRefSources = true;
100+
if (parameter.RefKind.IsWritableReference())
101+
{
102+
anyRefTargets = true;
103+
}
104+
}
105+
else if (parameter.RefKind != RefKind.None && parameter.EffectiveScope == ScopedKind.None)
106+
{
107+
anyRefSources = true;
108+
}
109+
110+
// If there is at least one output and at least one input, a `ref` can be captured.
111+
return anyRefTargets && anyRefSources;
112+
}
113+
}
114+
}

‎src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1660,7 +1660,13 @@ private void EmitStaticCallExpression(BoundCall call, UseKind useKind)
16601660

16611661
Debug.Assert(method.IsStatic);
16621662

1663+
var countBefore = _builder.LocalSlotManager.StartScopeOfTrackingAddressedLocals();
1664+
16631665
EmitArguments(arguments, method.Parameters, call.ArgumentRefKindsOpt);
1666+
1667+
_builder.LocalSlotManager.EndScopeOfTrackingAddressedLocals(countBefore,
1668+
MightEscapeTemporaryRefs(call, used: useKind != UseKind.Unused));
1669+
16641670
int stackBehavior = GetCallStackBehavior(method, arguments);
16651671

16661672
if (method.IsAbstract || method.IsVirtual)
@@ -1690,6 +1696,8 @@ private void EmitInstanceCallExpression(BoundCall call, UseKind useKind)
16901696
bool box;
16911697
LocalDefinition tempOpt;
16921698

1699+
var countBefore = _builder.LocalSlotManager.StartScopeOfTrackingAddressedLocals();
1700+
16931701
if (receiverIsInstanceCall(call, out BoundCall nested))
16941702
{
16951703
var calls = ArrayBuilder<BoundCall>.GetInstance();
@@ -1755,6 +1763,11 @@ private void EmitInstanceCallExpression(BoundCall call, UseKind useKind)
17551763
}
17561764

17571765
emitArgumentsAndCallEpilogue(call, callKind, receiverUseKind);
1766+
1767+
_builder.LocalSlotManager.EndScopeOfTrackingAddressedLocals(countBefore, MightEscapeTemporaryRefs(call, used: true));
1768+
1769+
countBefore = _builder.LocalSlotManager.StartScopeOfTrackingAddressedLocals();
1770+
17581771
FreeOptTemp(tempOpt);
17591772
tempOpt = null;
17601773

@@ -1815,6 +1828,9 @@ private void EmitInstanceCallExpression(BoundCall call, UseKind useKind)
18151828
}
18161829

18171830
emitArgumentsAndCallEpilogue(call, callKind, useKind);
1831+
1832+
_builder.LocalSlotManager.EndScopeOfTrackingAddressedLocals(countBefore, MightEscapeTemporaryRefs(call, used: useKind != UseKind.Unused));
1833+
18181834
FreeOptTemp(tempOpt);
18191835

18201836
return;
@@ -2446,8 +2462,14 @@ private void EmitObjectCreationExpression(BoundObjectCreationExpression expressi
24462462
}
24472463

24482464
// none of the above cases, so just create an instance
2465+
2466+
var countBefore = _builder.LocalSlotManager.StartScopeOfTrackingAddressedLocals();
2467+
24492468
EmitArguments(expression.Arguments, constructor.Parameters, expression.ArgumentRefKindsOpt);
24502469

2470+
_builder.LocalSlotManager.EndScopeOfTrackingAddressedLocals(countBefore,
2471+
MightEscapeTemporaryRefs(expression, used));
2472+
24512473
var stackAdjustment = GetObjCreationStackBehavior(expression);
24522474
_builder.EmitOpCode(ILOpCode.Newobj, stackAdjustment);
24532475

@@ -2572,8 +2594,7 @@ private void EmitAssignmentExpression(BoundAssignmentOperator assignmentOperator
25722594
private bool TryEmitAssignmentInPlace(BoundAssignmentOperator assignmentOperator, bool used)
25732595
{
25742596
// If the left hand is itself a ref, then we can't use in-place assignment
2575-
// because we need to spill the creation. This code can't be written in C#, but
2576-
// can be produced by lowering.
2597+
// because we need to spill the creation.
25772598
if (assignmentOperator.IsRef)
25782599
{
25792600
return false;
@@ -2704,7 +2725,14 @@ private bool TryInPlaceCtorCall(BoundExpression target, BoundObjectCreationExpre
27042725
Debug.Assert(temp == null, "in-place ctor target should not create temps");
27052726

27062727
var constructor = objCreation.Constructor;
2728+
2729+
var countBefore = _builder.LocalSlotManager.StartScopeOfTrackingAddressedLocals();
2730+
27072731
EmitArguments(objCreation.Arguments, constructor.Parameters, objCreation.ArgumentRefKindsOpt);
2732+
2733+
_builder.LocalSlotManager.EndScopeOfTrackingAddressedLocals(countBefore,
2734+
MightEscapeTemporaryRefs(objCreation, used));
2735+
27082736
// -2 to adjust for consumed target address and not produced value.
27092737
var stackAdjustment = GetObjCreationStackBehavior(objCreation) - 2;
27102738
_builder.EmitOpCode(ILOpCode.Call, stackAdjustment);
@@ -4026,7 +4054,14 @@ private void EmitCalli(BoundFunctionPointerInvocation ptrInvocation, UseKind use
40264054
}
40274055

40284056
FunctionPointerMethodSymbol method = ptrInvocation.FunctionPointer.Signature;
4057+
4058+
var countBefore = _builder.LocalSlotManager.StartScopeOfTrackingAddressedLocals();
4059+
40294060
EmitArguments(ptrInvocation.Arguments, method.Parameters, ptrInvocation.ArgumentRefKindsOpt);
4061+
4062+
_builder.LocalSlotManager.EndScopeOfTrackingAddressedLocals(countBefore,
4063+
MightEscapeTemporaryRefs(ptrInvocation, used: useKind != UseKind.Unused));
4064+
40304065
var stackBehavior = GetCallStackBehavior(ptrInvocation.FunctionPointer.Signature, ptrInvocation.Arguments);
40314066

40324067
if (temp is object)

‎src/Compilers/CSharp/Portable/CodeGen/Optimizer.cs‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -991,13 +991,23 @@ public override BoundNode VisitAssignmentOperator(BoundAssignmentOperator node)
991991
// Special Case: If the RHS is a pointer conversion, then the assignment functions as
992992
// a conversion (because the RHS will actually be typed as a native u/int in IL), so
993993
// we should not optimize away the local (i.e. schedule it on the stack).
994-
if (CanScheduleToStack(localSymbol) &&
994+
else if (CanScheduleToStack(localSymbol) &&
995995
assignmentLocal.Type.IsPointerOrFunctionPointer() && right.Kind == BoundKind.Conversion &&
996996
((BoundConversion)right).ConversionKind.IsPointerConversion())
997997
{
998998
ShouldNotSchedule(localSymbol);
999999
}
10001000

1001+
// If this is a pointer-to-ref assignment, keep the local so GC knows to re-track it.
1002+
// We don't need to do this for implicitly synthesized locals because working with pointers in an unsafe context does not guarantee any GC tracking,
1003+
// but when a pointer is converted to a user-defined ref local, it becomes a use of a "safe" feature where we should guarantee the ref is tracked by GC.
1004+
else if (localSymbol.RefKind != RefKind.None &&
1005+
localSymbol.SynthesizedKind == SynthesizedLocalKind.UserDefined &&
1006+
right.Kind == BoundKind.PointerIndirectionOperator)
1007+
{
1008+
ShouldNotSchedule(localSymbol);
1009+
}
1010+
10011011
RecordVarWrite(localSymbol);
10021012
assignmentLocal = null;
10031013
}

‎src/Compilers/CSharp/Portable/Symbols/NativeIntegerTypeSymbol.cs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,20 @@ public override ImmutableArray<ParameterSymbol> Parameters
368368
}
369369
}
370370

371+
internal override bool TryGetThisParameter(out ParameterSymbol? thisParameter)
372+
{
373+
if (UnderlyingMethod.TryGetThisParameter(out ParameterSymbol? underlyingThisParameter))
374+
{
375+
thisParameter = underlyingThisParameter != null
376+
? new ThisParameterSymbol(this, _container)
377+
: null;
378+
return true;
379+
}
380+
381+
thisParameter = null;
382+
return false;
383+
}
384+
371385
public override ImmutableArray<MethodSymbol> ExplicitInterfaceImplementations => ImmutableArray<MethodSymbol>.Empty;
372386

373387
public override ImmutableArray<CustomModifier> RefCustomModifiers => UnderlyingMethod.RefCustomModifiers;

‎src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenInParametersTests.cs‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -861,7 +861,8 @@ static ref readonly int M(in int x)
861861
// Code size 72 (0x48)
862862
.maxstack 3
863863
.locals init (int V_0,
864-
int V_1)
864+
int V_1,
865+
int V_2)
865866
IL_0000: ldc.i4.s 42
866867
IL_0002: stloc.0
867868
IL_0003: ldloca.s V_0
@@ -870,22 +871,22 @@ .locals init (int V_0,
870871
IL_000b: call ""void System.Console.WriteLine(int)""
871872
IL_0010: newobj ""Program..ctor()""
872873
IL_0015: ldc.i4.5
873-
IL_0016: stloc.0
874-
IL_0017: ldloca.s V_0
874+
IL_0016: stloc.1
875+
IL_0017: ldloca.s V_1
875876
IL_0019: ldc.i4.6
876-
IL_001a: stloc.1
877-
IL_001b: ldloca.s V_1
877+
IL_001a: stloc.2
878+
IL_001b: ldloca.s V_2
878879
IL_001d: call ""int Program.this[in int, in int].get""
879880
IL_0022: call ""void System.Console.WriteLine(int)""
880881
IL_0027: ldc.i4.s 42
881-
IL_0029: stloc.0
882-
IL_002a: ldloca.s V_0
882+
IL_0029: stloc.1
883+
IL_002a: ldloca.s V_1
883884
IL_002c: call ""ref readonly int Program.M(in int)""
884885
IL_0031: ldind.i4
885886
IL_0032: call ""void System.Console.WriteLine(int)""
886887
IL_0037: ldc.i4.s 42
887-
IL_0039: stloc.0
888-
IL_003a: ldloca.s V_0
888+
IL_0039: stloc.2
889+
IL_003a: ldloca.s V_2
889890
IL_003c: call ""ref readonly int Program.M(in int)""
890891
IL_0041: ldind.i4
891892
IL_0042: call ""void System.Console.WriteLine(int)""

0 commit comments

Comments
 (0)