Skip to content

Commit 1dd4753

Browse files
authored
Revert "Do not copy nullable value type receiver of a constrained call." (#66407)
Reverts #66311 due to dotnet/runtime#80429 (comment)
2 parents a13836f + 6c097b6 commit 1dd4753

File tree

22 files changed

+2984
-3419
lines changed

22 files changed

+2984
-3419
lines changed

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

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4580,21 +4580,6 @@ internal static bool HasHome(
45804580
stackLocalsOpt));
45814581
goto case BoundKind.ConditionalReceiver;
45824582

4583-
case BoundKind.ComplexReceiver:
4584-
Debug.Assert(HasHome(
4585-
((BoundComplexReceiver)expression).ValueTypeReceiver,
4586-
addressKind,
4587-
containingSymbol,
4588-
peVerifyCompatEnabled,
4589-
stackLocalsOpt));
4590-
Debug.Assert(HasHome(
4591-
((BoundComplexReceiver)expression).ReferenceTypeReceiver,
4592-
addressKind,
4593-
containingSymbol,
4594-
peVerifyCompatEnabled,
4595-
stackLocalsOpt));
4596-
goto case BoundKind.ConditionalReceiver;
4597-
45984583
case BoundKind.ConditionalReceiver:
45994584
//ConditionalReceiver is a noop from Emit point of view. - it represents something that has already been pushed.
46004585
//We should never need a temp for it.

src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1714,8 +1714,8 @@
17141714
<Field Name="Id" Type="int"/>
17151715
</Node>
17161716

1717-
<!-- This node represents a complex receiver for a conditional access.
1718-
At runtime, when its type is a non-nullable value type, ValueTypeReceiver should be used as a receiver.
1717+
<!-- This node represents a complex receiver for a call, or a conditional access.
1718+
At runtime, when its type is a value type, ValueTypeReceiver should be used as a receiver.
17191719
Otherwise, ReferenceTypeReceiver should be used.
17201720
This kind of receiver is created only by SpillSequenceSpiller rewriter.
17211721
-->
@@ -1725,16 +1725,6 @@
17251725
<Field Name="ReferenceTypeReceiver" Type="BoundExpression" Null="disallow"/>
17261726
</Node>
17271727

1728-
<!-- This node represents a complex receiver for a call.
1729-
At runtime, when its type is a value type (including nullable value type), ValueTypeReceiver should be used as a receiver.
1730-
Otherwise, ReferenceTypeReceiver should be used.
1731-
-->
1732-
<Node Name="BoundComplexReceiver" Base="BoundExpression">
1733-
<Field Name="Type" Type="TypeSymbol" Override="true" Null="disallow"/>
1734-
<Field Name="ValueTypeReceiver" Type="BoundExpression" Null="disallow"/>
1735-
<Field Name="ReferenceTypeReceiver" Type="BoundExpression" Null="disallow"/>
1736-
</Node>
1737-
17381728
<Node Name="BoundMethodGroup" Base="BoundMethodOrPropertyGroup">
17391729
<!-- SPEC: A method group is a set of overloaded methods resulting from a member lookup.
17401730
SPEC: A method group may have an associated instance expression and

src/Compilers/CSharp/Portable/CodeGen/CodeGenerator.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,14 +224,14 @@ public void Generate(
224224
hasStackAlloc = _sawStackalloc;
225225
Debug.Assert(_asyncCatchHandlerOffset >= 0);
226226

227-
asyncCatchHandlerOffset = _diagnostics.HasAnyErrors() ? -1 : _builder.GetILOffsetFromMarker(_asyncCatchHandlerOffset);
227+
asyncCatchHandlerOffset = _builder.GetILOffsetFromMarker(_asyncCatchHandlerOffset);
228228

229229
ArrayBuilder<int> yieldPoints = _asyncYieldPoints;
230230
ArrayBuilder<int> resumePoints = _asyncResumePoints;
231231

232232
Debug.Assert((yieldPoints == null) == (resumePoints == null));
233233

234-
if (yieldPoints == null || _diagnostics.HasAnyErrors())
234+
if (yieldPoints == null)
235235
{
236236
asyncYieldPoints = ImmutableArray<int>.Empty;
237237
asyncResumePoints = ImmutableArray<int>.Empty;

src/Compilers/CSharp/Portable/CodeGen/EmitAddress.cs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,6 @@ private LocalDefinition EmitAddress(BoundExpression expression, AddressKind addr
4848
EmitComplexConditionalReceiverAddress((BoundComplexConditionalReceiver)expression);
4949
break;
5050

51-
case BoundKind.ComplexReceiver:
52-
EmitComplexReceiverAddress((BoundComplexReceiver)expression);
53-
break;
54-
5551
case BoundKind.Parameter:
5652
return EmitParameterAddress((BoundParameter)expression, addressKind);
5753

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

Lines changed: 14 additions & 150 deletions
Original file line numberDiff line numberDiff line change
@@ -316,10 +316,6 @@ private void EmitExpressionCore(BoundExpression expression, bool used)
316316
EmitComplexConditionalReceiver((BoundComplexConditionalReceiver)expression, used);
317317
break;
318318

319-
case BoundKind.ComplexReceiver:
320-
EmitComplexReceiver((BoundComplexReceiver)expression, used);
321-
break;
322-
323319
case BoundKind.PseudoVariable:
324320
EmitPseudoVariableValue((BoundPseudoVariable)expression, used);
325321
break;
@@ -369,11 +365,7 @@ private void EmitComplexConditionalReceiver(BoundComplexConditionalReceiver expr
369365

370366
EmitExpression(expression.ReferenceTypeReceiver, used);
371367
_builder.EmitBranch(ILOpCode.Br, doneLabel);
372-
373-
if (used)
374-
{
375-
_builder.AdjustStack(-1);
376-
}
368+
_builder.AdjustStack(-1);
377369

378370
_builder.MarkLabel(whenValueTypeLabel);
379371
EmitExpression(expression.ValueTypeReceiver, used);
@@ -420,8 +412,7 @@ private void EmitLoweredConditionalAccessExpression(BoundLoweredConditionalAcces
420412
((TypeParameterSymbol)receiverType).EffectiveInterfacesNoUseSiteDiagnostics.IsEmpty) || // This could be a nullable value type, which must be copied in order to not mutate the original value
421413
LocalRewriter.CanChangeValueBetweenReads(receiver, localsMayBeAssignedOrCaptured: false) ||
422414
(receiverType.IsReferenceType && receiverType.TypeKind == TypeKind.TypeParameter) ||
423-
(receiver.Kind == BoundKind.Local && IsStackLocal(((BoundLocal)receiver).LocalSymbol)) ||
424-
(notConstrained && IsConditionalConstrainedCallThatMustUseTempForReferenceTypeReceiverWalker.Analyze(expression));
415+
(receiver.Kind == BoundKind.Local && IsStackLocal(((BoundLocal)receiver).LocalSymbol));
425416

426417
// ===== RECEIVER
427418
if (nullCheckOnCopy)
@@ -573,60 +564,6 @@ private void EmitLoweredConditionalAccessExpression(BoundLoweredConditionalAcces
573564
}
574565
}
575566

576-
private sealed class IsConditionalConstrainedCallThatMustUseTempForReferenceTypeReceiverWalker : BoundTreeWalkerWithStackGuardWithoutRecursionOnTheLeftOfBinaryOperator
577-
{
578-
private readonly BoundLoweredConditionalAccess _conditionalAccess;
579-
private bool? _result;
580-
581-
private IsConditionalConstrainedCallThatMustUseTempForReferenceTypeReceiverWalker(BoundLoweredConditionalAccess conditionalAccess)
582-
: base()
583-
{
584-
_conditionalAccess = conditionalAccess;
585-
}
586-
587-
public static bool Analyze(BoundLoweredConditionalAccess conditionalAccess)
588-
{
589-
var walker = new IsConditionalConstrainedCallThatMustUseTempForReferenceTypeReceiverWalker(conditionalAccess);
590-
walker.Visit(conditionalAccess.WhenNotNull);
591-
Debug.Assert(walker._result.HasValue);
592-
return walker._result.GetValueOrDefault();
593-
}
594-
595-
public override BoundNode Visit(BoundNode node)
596-
{
597-
if (_result.HasValue)
598-
{
599-
return null;
600-
}
601-
602-
return base.Visit(node);
603-
}
604-
605-
public override BoundNode VisitCall(BoundCall node)
606-
{
607-
if (node.ReceiverOpt is BoundConditionalReceiver { Id: var id } && id == _conditionalAccess.Id)
608-
{
609-
Debug.Assert(!_result.HasValue);
610-
_result = !IsSafeToDereferenceReceiverRefAfterEvaluatingArguments(node.Arguments);
611-
return null;
612-
}
613-
614-
return base.VisitCall(node);
615-
}
616-
617-
public override BoundNode VisitConditionalReceiver(BoundConditionalReceiver node)
618-
{
619-
if (node.Id == _conditionalAccess.Id)
620-
{
621-
Debug.Assert(!_result.HasValue);
622-
_result = false;
623-
return null;
624-
}
625-
626-
return base.VisitConditionalReceiver(node);
627-
}
628-
}
629-
630567
private void EmitConditionalReceiver(BoundConditionalReceiver expression, bool used)
631568
{
632569
Debug.Assert(!expression.Type.IsValueType);
@@ -1815,21 +1752,25 @@ LocalDefinition emitGenericReceiver(BoundCall call, out CallKind callKind)
18151752
// reference to the stack. So, for a class we need to emit a reference to a temporary
18161753
// location, rather than to the original location
18171754

1818-
// We will emit a runtime check for a class, but the check is really a JIT-time
1755+
// Struct values are never nulls.
1756+
// We will emit a check for such case, but the check is really a JIT-time
18191757
// constant since JIT will know if T is a struct or not.
18201758

1821-
// if (!typeof(T).IsValueType)
1759+
// if ((object)default(T) == null)
18221760
// {
18231761
// temp = receiverRef
18241762
// receiverRef = ref temp
18251763
// }
18261764

1827-
object whenValueTypeLabel = null;
1765+
object whenNotNullLabel = null;
18281766

18291767
if (!receiverType.IsReferenceType)
18301768
{
1831-
// if (!typeof(T).IsValueType)
1832-
whenValueTypeLabel = TryEmitIsValueTypeBranch(receiverType, receiver.Syntax);
1769+
// if ((object)default(T) == null)
1770+
EmitDefaultValue(receiverType, true, receiver.Syntax);
1771+
EmitBox(receiverType, receiver.Syntax);
1772+
whenNotNullLabel = new object();
1773+
_builder.EmitBranch(ILOpCode.Brtrue, whenNotNullLabel);
18331774
}
18341775

18351776
// temp = receiverRef
@@ -1839,91 +1780,16 @@ LocalDefinition emitGenericReceiver(BoundCall call, out CallKind callKind)
18391780
_builder.EmitLocalStore(tempOpt);
18401781
_builder.EmitLocalAddress(tempOpt);
18411782

1842-
if (whenValueTypeLabel is not null)
1783+
if (whenNotNullLabel is not null)
18431784
{
1844-
_builder.MarkLabel(whenValueTypeLabel);
1785+
_builder.MarkLabel(whenNotNullLabel);
18451786
}
18461787
}
18471788

18481789
return tempOpt;
18491790
}
18501791
}
18511792

1852-
private object TryEmitIsValueTypeBranch(TypeSymbol type, SyntaxNode syntax)
1853-
{
1854-
if (Binder.GetWellKnownTypeMember(_module.Compilation, WellKnownMember.System_Type__GetTypeFromHandle, _diagnostics, syntax: syntax, isOptional: false) is MethodSymbol getTypeFromHandle &&
1855-
Binder.GetWellKnownTypeMember(_module.Compilation, WellKnownMember.System_Type__get_IsValueType, _diagnostics, syntax: syntax, isOptional: false) is MethodSymbol getIsValueType)
1856-
{
1857-
_builder.EmitOpCode(ILOpCode.Ldtoken);
1858-
EmitSymbolToken(type, syntax);
1859-
_builder.EmitOpCode(ILOpCode.Call, stackAdjustment: 0); // argument off, return value on
1860-
EmitSymbolToken(getTypeFromHandle, syntax, null);
1861-
_builder.EmitOpCode(ILOpCode.Call, stackAdjustment: 0); // instance off, return value on
1862-
EmitSymbolToken(getIsValueType, syntax, null);
1863-
var whenValueTypeLabel = new object();
1864-
_builder.EmitBranch(ILOpCode.Brtrue, whenValueTypeLabel);
1865-
1866-
return whenValueTypeLabel;
1867-
}
1868-
1869-
return null;
1870-
}
1871-
1872-
private void EmitComplexReceiver(BoundComplexReceiver expression, bool used)
1873-
{
1874-
Debug.Assert(!used);
1875-
Debug.Assert(!expression.Type.IsReferenceType);
1876-
Debug.Assert(!expression.Type.IsValueType);
1877-
1878-
var receiverType = expression.Type;
1879-
1880-
// if (!typeof(T).IsValueType)
1881-
object whenValueTypeLabel = TryEmitIsValueTypeBranch(receiverType, expression.Syntax);
1882-
1883-
EmitExpression(expression.ReferenceTypeReceiver, used);
1884-
var doneLabel = new object();
1885-
_builder.EmitBranch(ILOpCode.Br, doneLabel);
1886-
1887-
if (whenValueTypeLabel is not null)
1888-
{
1889-
if (used)
1890-
{
1891-
_builder.AdjustStack(-1);
1892-
}
1893-
1894-
_builder.MarkLabel(whenValueTypeLabel);
1895-
EmitExpression(expression.ValueTypeReceiver, used);
1896-
}
1897-
1898-
_builder.MarkLabel(doneLabel);
1899-
}
1900-
1901-
private void EmitComplexReceiverAddress(BoundComplexReceiver expression)
1902-
{
1903-
Debug.Assert(!expression.Type.IsReferenceType);
1904-
Debug.Assert(!expression.Type.IsValueType);
1905-
1906-
var receiverType = expression.Type;
1907-
1908-
// if (!typeof(T).IsValueType)
1909-
object whenValueTypeLabel = TryEmitIsValueTypeBranch(receiverType, expression.Syntax);
1910-
1911-
var receiverTemp = EmitAddress(expression.ReferenceTypeReceiver, AddressKind.ReadOnly);
1912-
Debug.Assert(receiverTemp == null);
1913-
var doneLabel = new Object();
1914-
_builder.EmitBranch(ILOpCode.Br, doneLabel);
1915-
1916-
if (whenValueTypeLabel is not null)
1917-
{
1918-
_builder.AdjustStack(-1);
1919-
_builder.MarkLabel(whenValueTypeLabel);
1920-
// we will not write through this receiver, but it could be a target of mutating calls
1921-
EmitReceiverRef(expression.ValueTypeReceiver, AddressKind.Constrained);
1922-
}
1923-
1924-
_builder.MarkLabel(doneLabel);
1925-
}
1926-
19271793
internal static bool IsPossibleReferenceTypeReceiverOfConstrainedCall(BoundExpression receiver)
19281794
{
19291795
var receiverType = receiver.Type;
@@ -1945,9 +1811,7 @@ internal static bool ReceiverIsKnownToReferToTempIfReferenceType(BoundExpression
19451811

19461812
if (receiver is
19471813
BoundLocal { LocalSymbol.IsKnownToReferToTempIfReferenceType: true } or
1948-
BoundComplexConditionalReceiver or
1949-
BoundComplexReceiver or
1950-
BoundConditionalReceiver { Type: { IsReferenceType: false, IsValueType: false } })
1814+
BoundComplexConditionalReceiver)
19511815
{
19521816
return true;
19531817
}

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

Lines changed: 1 addition & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
using System.Collections.Immutable;
1010
using System.Diagnostics;
1111
using System.Linq;
12-
using System.Runtime.CompilerServices;
1312
using System.Text;
1413
using Microsoft.CodeAnalysis.Collections;
1514
using Microsoft.CodeAnalysis.CSharp.Symbols;
@@ -1564,43 +1563,6 @@ public override BoundNode VisitComplexConditionalReceiver(BoundComplexConditiona
15641563
return node.Update(valueTypeReceiver, referenceTypeReceiver, node.Type);
15651564
}
15661565

1567-
public override BoundNode VisitComplexReceiver(BoundComplexReceiver node)
1568-
{
1569-
EnsureOnlyEvalStack();
1570-
1571-
var origStack = StackDepth();
1572-
1573-
PushEvalStack(null, ExprContext.None);
1574-
1575-
var cookie = GetStackStateCookie(); // implicit goto here
1576-
1577-
SetStackDepth(origStack); // consequence is evaluated with original stack
1578-
1579-
var valueTypeReceiver = (BoundExpression)this.Visit(node.ValueTypeReceiver);
1580-
1581-
EnsureStackState(cookie); // implicit label here
1582-
1583-
SetStackDepth(origStack); // alternative is evaluated with original stack
1584-
1585-
var unwrappedSequence = node.ReferenceTypeReceiver;
1586-
1587-
while (unwrappedSequence is BoundSequence sequence)
1588-
{
1589-
unwrappedSequence = sequence.Value;
1590-
}
1591-
1592-
if (unwrappedSequence is BoundLocal { LocalSymbol: { } localSymbol })
1593-
{
1594-
ShouldNotSchedule(localSymbol);
1595-
}
1596-
1597-
var referenceTypeReceiver = (BoundExpression)this.Visit(node.ReferenceTypeReceiver);
1598-
1599-
EnsureStackState(cookie); // implicit label here
1600-
1601-
return node.Update(valueTypeReceiver, referenceTypeReceiver, node.Type);
1602-
}
1603-
16041566
public override BoundNode VisitUnaryOperator(BoundUnaryOperator node)
16051567
{
16061568
// checked(-x) is emitted as "0 - x"
@@ -2252,14 +2214,7 @@ internal override SyntaxNode ScopeDesignatorOpt
22522214
get { return null; }
22532215
}
22542216

2255-
internal override LocalSymbol WithSynthesizedLocalKindAndSyntax(
2256-
SynthesizedLocalKind kind, SyntaxNode syntax
2257-
#if DEBUG
2258-
,
2259-
[CallerLineNumber] int createdAtLineNumber = 0,
2260-
[CallerFilePath] string createdAtFilePath = null
2261-
#endif
2262-
)
2217+
internal override LocalSymbol WithSynthesizedLocalKindAndSyntax(SynthesizedLocalKind kind, SyntaxNode syntax)
22632218
{
22642219
throw new NotImplementedException();
22652220
}

src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2940,20 +2940,6 @@ public override BoundNode VisitComplexConditionalReceiver(BoundComplexConditiona
29402940
return null;
29412941
}
29422942

2943-
public override BoundNode VisitComplexReceiver(BoundComplexReceiver node)
2944-
{
2945-
var savedState = this.State.Clone();
2946-
2947-
VisitRvalue(node.ValueTypeReceiver);
2948-
Join(ref this.State, ref savedState);
2949-
2950-
savedState = this.State.Clone();
2951-
VisitRvalue(node.ReferenceTypeReceiver);
2952-
Join(ref this.State, ref savedState);
2953-
2954-
return null;
2955-
}
2956-
29572943
public override BoundNode VisitSequence(BoundSequence node)
29582944
{
29592945
var sideEffects = node.SideEffects;

0 commit comments

Comments
 (0)