Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3026,13 +3026,13 @@ private bool CheckInvocationArgMixingWithUpdatedRules(
}
}

inferDeclarationExpressionValEscape();
inferDeclarationExpressionValEscape(argsOpt, localScopeDepth, escapeValues);

mixableArguments.Free();
escapeValues.Free();
return valid;

void inferDeclarationExpressionValEscape()
void inferDeclarationExpressionValEscape(ImmutableArray<BoundExpression> argsOpt, SafeContext localScopeDepth, ArrayBuilder<EscapeValue> escapeValues)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 I was debugging in this area as part of this PR and noticed we're doing some captures

{
// find the widest scope that arguments could safely escape to.
// use this scope as the inferred STE of declaration expressions.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ protected BoundExpression LowerEvaluation(BoundDagEvaluation evaluation)
PropertySymbol property = p.Property;
var outputTemp = new BoundDagTemp(p.Syntax, property.Type, p);
BoundExpression output = _tempAllocator.GetTemp(outputTemp);
input = _factory.ConvertReceiverForExtensionMemberIfNeeded(property, input);
// Tracked by https://github.com/dotnet/roslyn/issues/76130 : Consider preserving the BoundConversion from initial binding instead of using markAsChecked here
input = _localRewriter.ConvertReceiverForExtensionMemberIfNeeded(property, input, markAsChecked: true);
return _factory.AssignmentExpression(output, _localRewriter.MakePropertyAccess(_factory.Syntax, input, property, LookupResultKind.Viable, property.Type, isLeftOfAssignment: false));
}

Expand Down Expand Up @@ -191,7 +192,8 @@ void addArg(RefKind refKind, BoundExpression expression)
addArg(RefKind.Out, _tempAllocator.GetTemp(outputTemp));
}

receiver = _factory.ConvertReceiverForExtensionMemberIfNeeded(method, receiver);
// Tracked by https://github.com/dotnet/roslyn/issues/76130 : Consider preserving the BoundConversion from initial binding instead of using markAsChecked here
receiver = _localRewriter.ConvertReceiverForExtensionMemberIfNeeded(method, receiver, markAsChecked: true);
return _factory.Call(receiver, method, refKindBuilder.ToImmutableAndFree(), argBuilder.ToImmutableAndFree());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1136,6 +1136,25 @@ private CompoundUseSiteInfo<AssemblySymbol> GetNewCompoundUseSiteInfo()
return new CompoundUseSiteInfo<AssemblySymbol>(_diagnostics, _compilation.Assembly);
}

private BoundExpression ConvertReceiverForExtensionMemberIfNeeded(Symbol member, BoundExpression receiver, bool markAsChecked)
{
if (member.GetIsNewExtensionMember())
{
Debug.Assert(!member.IsStatic);
ParameterSymbol? extensionParameter = member.ContainingType.ExtensionParameter;
Debug.Assert(extensionParameter is not null);
#if DEBUG
var discardedUseSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;
Debug.Assert(Conversions.IsValidExtensionMethodThisArgConversion(this._compilation.Conversions.ClassifyConversionFromType(receiver.Type, extensionParameter.Type, isChecked: false, ref discardedUseSiteInfo)));
#endif

// We don't need to worry about checked context because only implicit conversions are allowed on the receiver of an extension member
return MakeConversionNode(receiver, extensionParameter.Type, @checked: false, acceptFailingConversion: false, markAsChecked: markAsChecked);
}

return receiver;
}

#if DEBUG
/// <summary>
/// Note: do not use a static/singleton instance of this type, as it holds state.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -781,6 +781,10 @@ bool IsInRange(SpecialType type, SpecialType low, SpecialType high) =>
/// (e.g. if the default value was specified by an attribute and was, therefore, not checked by the compiler).
/// Set acceptFailingConversion if you want to see default(rewrittenType) in such cases.
/// The error will be suppressed only for conversions from <see cref="decimal"/> or <see cref="DateTime"/>.
///
/// Generally, conversions should be checked and BoundConversion nodes created during initial binding and re-used in lowering. But in some cases,
/// we do not preserve the conversion node. In such case, <paramref name="markAsChecked"/> is set to true
/// to indicate that the conversion has been previously checked during initial binding.
/// </remarks>
private BoundExpression MakeConversionNode(BoundExpression rewrittenOperand, TypeSymbol rewrittenType, bool @checked, bool acceptFailingConversion = false, bool markAsChecked = false)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -354,8 +354,9 @@ private BoundStatement InitializeFixedStatementGetPinnable(
callReceiver = initializerExpr;
}

// Tracked by https://github.com/dotnet/roslyn/issues/76130 : Consider preserving the BoundConversion from initial binding instead of using markAsChecked here
// .GetPinnable()
callReceiver = factory.ConvertReceiverForExtensionMemberIfNeeded(getPinnableMethod, callReceiver);
callReceiver = this.ConvertReceiverForExtensionMemberIfNeeded(getPinnableMethod, callReceiver, markAsChecked: true);
var getPinnableCall = getPinnableMethod.IsStatic ?
factory.Call(null, getPinnableMethod, callReceiver) :
factory.Call(callReceiver, getPinnableMethod);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,8 @@ private BoundExpression MakeObjectInitializerMemberAccess(
_compilation.Conversions.HasImplicitConversionToOrImplementsVarianceCompatibleInterface(rewrittenReceiver.Type, memberSymbol.ContainingType, ref discardedUseSiteInfo, out _));
// It is possible there are use site diagnostics from the above, but none that we need report as we aren't generating code for the conversion
#endif
rewrittenReceiver = _factory.ConvertReceiverForExtensionMemberIfNeeded(memberSymbol, rewrittenReceiver);
// Tracked by https://github.com/dotnet/roslyn/issues/76130 : Consider preserving the BoundConversion from initial binding instead of using markAsChecked here
rewrittenReceiver = this.ConvertReceiverForExtensionMemberIfNeeded(memberSymbol, rewrittenReceiver, markAsChecked: true);

switch (memberSymbol.Kind)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -431,24 +431,6 @@ public BoundExpression AssignmentExpression(BoundExpression left, BoundExpressio
return AssignmentExpression(Syntax, left, right, isRef: isRef, wasCompilerGenerated: true);
}

public BoundExpression ConvertReceiverForExtensionMemberIfNeeded(Symbol member, BoundExpression receiver)
{
if (member.GetIsNewExtensionMember())
{
Debug.Assert(!member.IsStatic);
ParameterSymbol? extensionParameter = member.ContainingType.ExtensionParameter;
Debug.Assert(extensionParameter is not null);
#if DEBUG
var discardedUseSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;
Debug.Assert(Conversions.IsValidExtensionMethodThisArgConversion(this.Compilation.Conversions.ClassifyConversionFromType(receiver.Type, extensionParameter.Type, isChecked: false, ref discardedUseSiteInfo)));
#endif

return this.Convert(extensionParameter.Type, receiver);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 the problem was that this produces a BoundConversion. That's okay (can be handled by the emit layer) for simple conversions, but for more complex conversions we need to produce lowered nodes (see MakeConversionNode).

}

return receiver;
}

/// <summary>
/// Creates a general assignment that might be instrumented.
/// </summary>
Expand Down
Loading
Loading