Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Save and restore '_factory.Syntax' in LocalRewriter methods #59236

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Save and restore '_factory.Syntax' in LocalRewriter methods #59236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ea559d7
Save and restore '_factory.Syntax' in LocalRewriter methods
cston Jan 31, 2022
79723d9
Misc.
cston Feb 7, 2022
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
9 changes: 9 additions & 0 deletions src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.DecisionDagRewriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,7 @@ testNode.WhenTrue is BoundEvaluationDecisionDagNode evaluationNode &&
private void GenerateTest(BoundExpression test, BoundDecisionDagNode whenTrue, BoundDecisionDagNode whenFalse, BoundDecisionDagNode nextNode)
{
// Because we have already "optimized" away tests for a constant switch expression, the test should be nontrivial.
var oldSyntax = _factory.Syntax;
_factory.Syntax = test.Syntax;
Debug.Assert(test != null);

Expand All @@ -490,6 +491,8 @@ private void GenerateTest(BoundExpression test, BoundDecisionDagNode whenTrue, B
_loweredDecisionDag.Add(_factory.ConditionalGoto(test, GetDagNodeLabel(whenTrue), jumpIfTrue: true));
_loweredDecisionDag.Add(_factory.Goto(GetDagNodeLabel(whenFalse)));
}

_factory.Syntax = oldSyntax;
}

/// <summary>
Expand Down Expand Up @@ -1026,6 +1029,7 @@ void lowerWhenExpressionIfShared(BoundExpression whenExpression, LabelSymbol lab
// }
void addConditionalGoto(BoundExpression whenExpression, SyntaxNode whenClauseSyntax, LabelSymbol whenTrueLabel, ArrayBuilder<BoundStatement> sectionBuilder)
{
var oldSyntax = _factory.Syntax;
_factory.Syntax = whenClauseSyntax;
BoundStatement conditionalGoto = _factory.ConditionalGoto(_localRewriter.VisitExpression(whenExpression), whenTrueLabel, jumpIfTrue: true);

Expand All @@ -1036,6 +1040,7 @@ void addConditionalGoto(BoundExpression whenExpression, SyntaxNode whenClauseSyn
}

sectionBuilder.Add(conditionalGoto);
_factory.Syntax = oldSyntax;
}

bool isSharedWhenExpression(BoundExpression? whenExpression)
Expand Down Expand Up @@ -1101,7 +1106,9 @@ void lowerBindings(ImmutableArray<BoundPatternBinding> bindings, ArrayBuilder<Bo
/// </summary>
private void LowerDecisionDagNode(BoundDecisionDagNode node, BoundDecisionDagNode nextNode)
{
var oldSyntax = _factory.Syntax;
_factory.Syntax = node.Syntax;

switch (node)
{
case BoundEvaluationDecisionDagNode evaluationNode:
Expand Down Expand Up @@ -1136,6 +1143,8 @@ private void LowerDecisionDagNode(BoundDecisionDagNode node, BoundDecisionDagNod
default:
throw ExceptionUtilities.UnexpectedValue(node.Kind);
}

_factory.Syntax = oldSyntax;
}
}
}
Expand Down
18 changes: 16 additions & 2 deletions src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,14 @@ private PEModuleBuilder? EmitModule
return VisitExpressionImpl(expr);
}

return node.Accept(this);
#if DEBUG
var oldSyntax = _factory.Syntax;
#endif
var result = node.Accept(this);
#if DEBUG
Debug.Assert(oldSyntax == _factory.Syntax);
#endif
return result;
}

[return: NotNullIfNotNull("node")]
Expand All @@ -192,7 +199,14 @@ private PEModuleBuilder? EmitModule
}
Debug.Assert(!node.HasErrors, "nodes with errors should not be lowered");

return (BoundStatement?)node.Accept(this);
#if DEBUG
var oldSyntax = _factory.Syntax;
#endif
var result = (BoundStatement?)node.Accept(this);
#if DEBUG
Debug.Assert(oldSyntax == _factory.Syntax);
#endif
return result;
}

private BoundExpression? VisitExpressionImpl(BoundExpression node)
Expand Down
12 changes: 8 additions & 4 deletions src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_IsPatternOperator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ public IsPatternExpressionGeneralLocalRewriter(

internal BoundExpression LowerGeneralIsPattern(BoundIsPatternExpression node, BoundDecisionDag decisionDag)
{
var oldSyntax = _factory.Syntax;
_factory.Syntax = node.Syntax;
var resultBuilder = ArrayBuilder<BoundStatement>.GetInstance();
var inputExpression = _localRewriter.VisitExpression(node.Expression);
Expand All @@ -128,7 +129,9 @@ internal BoundExpression LowerGeneralIsPattern(BoundIsPatternExpression node, Bo
resultBuilder.Add(_factory.Assignment(_factory.Local(resultTemp), _factory.Literal(false)));
resultBuilder.Add(_factory.Label(afterIsPatternExpression));
_localRewriter._needsSpilling = true;
return _factory.SpillSequence(_tempAllocator.AllTemps().Add(resultTemp), resultBuilder.ToImmutableAndFree(), _factory.Local(resultTemp));
var result = _factory.SpillSequence(_tempAllocator.AllTemps().Add(resultTemp), resultBuilder.ToImmutableAndFree(), _factory.Local(resultTemp));
_factory.Syntax = oldSyntax;
return result;
}
}

Expand Down Expand Up @@ -188,14 +191,15 @@ private void AddConjunct(BoundExpression test)
/// </summary>
private void LowerOneTest(BoundDagTest test, bool invert = false)
{
var oldSyntax = _factory.Syntax;
_factory.Syntax = test.Syntax;
switch (test)
{
case BoundDagEvaluation eval:
{
var sideEffect = LowerEvaluation(eval);
_sideEffectBuilder.Add(sideEffect);
return;
break;
}
case var _:
{
Expand All @@ -207,10 +211,10 @@ private void LowerOneTest(BoundDagTest test, bool invert = false)

AddConjunct(testExpression);
}

return;
break;
}
}
_factory.Syntax = oldSyntax;
}

public BoundExpression LowerIsPatternAsLinearTestSequence(
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_PatternSwitchStatement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ private SwitchStatementLocalRewriter(BoundSwitchStatement node, LocalRewriter lo

private BoundStatement LowerSwitchStatement(BoundSwitchStatement node)
{
var oldSyntax = _factory.Syntax;
_factory.Syntax = node.Syntax;
var result = ArrayBuilder<BoundStatement>.GetInstance();
var outerVariables = ArrayBuilder<LocalSymbol>.GetInstance();
Expand Down Expand Up @@ -176,6 +177,7 @@ private BoundStatement LowerSwitchStatement(BoundSwitchStatement node)
if (GenerateInstrumentation)
translatedSwitch = _localRewriter._instrumenter.InstrumentSwitchStatement(node, translatedSwitch);

_factory.Syntax = oldSyntax;
return translatedSwitch;
}
}
Expand Down
152 changes: 84 additions & 68 deletions src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_StringInterpolation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ internal sealed partial class LocalRewriter
private BoundExpression RewriteInterpolatedStringConversion(BoundConversion conversion)
{
Debug.Assert(conversion.ConversionKind == ConversionKind.InterpolatedString);
var oldSyntax = _factory.Syntax;
_factory.Syntax = conversion.Syntax;

BoundExpression format;
ArrayBuilder<BoundExpression> expressions;
MakeInterpolatedStringFormat((BoundInterpolatedString)conversion.Operand, out format, out expressions);
Expand All @@ -35,6 +38,7 @@ private BoundExpression RewriteInterpolatedStringConversion(BoundConversion conv
result = MakeImplicitConversionForInterpolatedString(result, conversion.Type);
}

_factory.Syntax = oldSyntax;
return result;
}

Expand Down Expand Up @@ -207,7 +211,9 @@ private bool CanLowerToStringConcatenation(BoundInterpolatedString node)

private void MakeInterpolatedStringFormat(BoundInterpolatedString node, out BoundExpression format, out ArrayBuilder<BoundExpression> expressions)
{
var oldSyntax = _factory.Syntax;
_factory.Syntax = node.Syntax;

int n = node.Parts.Length - 1;
var formatString = PooledStringBuilder.GetInstance();
var stringBuilder = formatString.Builder;
Expand Down Expand Up @@ -249,97 +255,107 @@ private void MakeInterpolatedStringFormat(BoundInterpolatedString node, out Boun
}

format = _factory.StringLiteral(formatString.ToStringAndFree());
_factory.Syntax = oldSyntax;
}

public override BoundNode VisitInterpolatedString(BoundInterpolatedString node)
{
Debug.Assert(node.Type is { SpecialType: SpecialType.System_String }); // if target-converted, we should not get here.

BoundExpression? result;

if (node.InterpolationData is InterpolatedStringHandlerData data)
{
return LowerPartsToString(data, node.Parts, node.Syntax, node.Type);
}
else if (CanLowerToStringConcatenation(node))
var oldSyntax = _factory.Syntax;
_factory.Syntax = node.Syntax;
try
{
// All fill-ins, if any, are strings, and none of them have alignment or format specifiers.
// We can lower to a more efficient string concatenation
// The normal pattern for lowering is to lower subtrees before the enclosing tree. However in this case
// we want to lower the entire concatenation so we get the optimizations done by that lowering (e.g. constant folding).
BoundExpression? result;

int length = node.Parts.Length;
if (length == 0)
if (node.InterpolationData is InterpolatedStringHandlerData data)
{
// $"" -> ""
return _factory.StringLiteral("");
return LowerPartsToString(data, node.Parts, node.Syntax, node.Type);
}

result = null;
for (int i = 0; i < length; i++)
else if (CanLowerToStringConcatenation(node))
{
var part = node.Parts[i];
if (part is BoundStringInsert fillin)
// All fill-ins, if any, are strings, and none of them have alignment or format specifiers.
// We can lower to a more efficient string concatenation
// The normal pattern for lowering is to lower subtrees before the enclosing tree. However in this case
// we want to lower the entire concatenation so we get the optimizations done by that lowering (e.g. constant folding).

int length = node.Parts.Length;
if (length == 0)
{
// this is one of the filled-in expressions
part = fillin.Value;
// $"" -> ""
return _factory.StringLiteral("");
}
else

result = null;
for (int i = 0; i < length; i++)
{
// this is one of the literal parts
Debug.Assert(part is BoundLiteral && part.ConstantValue is { StringValue: { } });
part = _factory.StringLiteral(ConstantValueUtils.UnescapeInterpolatedStringLiteral(part.ConstantValue.StringValue));
var part = node.Parts[i];
if (part is BoundStringInsert fillin)
{
// this is one of the filled-in expressions
part = fillin.Value;
}
else
{
// this is one of the literal parts
Debug.Assert(part is BoundLiteral && part.ConstantValue is { StringValue: { } });
part = _factory.StringLiteral(ConstantValueUtils.UnescapeInterpolatedStringLiteral(part.ConstantValue.StringValue));
}

result = result == null ?
part :
_factory.Binary(BinaryOperatorKind.StringConcatenation, node.Type, result, part);
}

result = result == null ?
part :
_factory.Binary(BinaryOperatorKind.StringConcatenation, node.Type, result, part);
// We need to ensure that the result of the interpolated string is not null. If the single part has a non-null constant value
// or is itself an interpolated string (which by proxy cannot be null), then there's nothing else that needs to be done. Otherwise,
// we need to test for null and ensure "" if it is.
if (length == 1 && result is not ({ Kind: BoundKind.InterpolatedString } or { ConstantValue: { IsString: true } }))
{
Debug.Assert(result is not null);
Debug.Assert(result.Type is not null);
Debug.Assert(result.Type.SpecialType == SpecialType.System_String || result.Type.IsErrorType());
var placeholder = new BoundValuePlaceholder(result.Syntax, result.Type);
result = new BoundNullCoalescingOperator(result.Syntax, result, _factory.StringLiteral(""), leftPlaceholder: placeholder, leftConversion: placeholder, BoundNullCoalescingOperatorResultKind.LeftType, result.Type) { WasCompilerGenerated = true };
}
}
else
{
//
// We lower an interpolated string into an invocation of String.Format. For example, we translate the expression
//
// $"Jenny don\'t change your number { 8675309 }"
//
// into
//
// String.Format("Jenny don\'t change your number {0}", new object[] { 8675309 })
//

MakeInterpolatedStringFormat(node, out BoundExpression format, out ArrayBuilder<BoundExpression> expressions);

// The normal pattern for lowering is to lower subtrees before the enclosing tree. However we cannot lower
// the arguments first in this situation because we do not know what conversions will be
// produced for the arguments until after we've done overload resolution. So we produce the invocation
// and then lower it along with its arguments.
expressions.Insert(0, format);
var stringType = node.Type;
result = _factory.StaticCall(stringType, "Format", expressions.ToImmutableAndFree(),
allowUnexpandedForm: false // if an interpolation expression is the null literal, it should not match a params parameter.
);
}

// We need to ensure that the result of the interpolated string is not null. If the single part has a non-null constant value
// or is itself an interpolated string (which by proxy cannot be null), then there's nothing else that needs to be done. Otherwise,
// we need to test for null and ensure "" if it is.
if (length == 1 && result is not ({ Kind: BoundKind.InterpolatedString } or { ConstantValue: { IsString: true } }))
Debug.Assert(result is { });
if (!result.HasAnyErrors)
{
Debug.Assert(result is not null);
Debug.Assert(result.Type is not null);
Debug.Assert(result.Type.SpecialType == SpecialType.System_String || result.Type.IsErrorType());
var placeholder = new BoundValuePlaceholder(result.Syntax, result.Type);
result = new BoundNullCoalescingOperator(result.Syntax, result, _factory.StringLiteral(""), leftPlaceholder: placeholder, leftConversion: placeholder, BoundNullCoalescingOperatorResultKind.LeftType, result.Type) { WasCompilerGenerated = true };
result = VisitExpression(result); // lower the arguments AND handle expanded form, argument conversions, etc.
result = MakeImplicitConversionForInterpolatedString(result, node.Type);
}
return result;
}
else
{
//
// We lower an interpolated string into an invocation of String.Format. For example, we translate the expression
//
// $"Jenny don\'t change your number { 8675309 }"
//
// into
//
// String.Format("Jenny don\'t change your number {0}", new object[] { 8675309 })
//

MakeInterpolatedStringFormat(node, out BoundExpression format, out ArrayBuilder<BoundExpression> expressions);

// The normal pattern for lowering is to lower subtrees before the enclosing tree. However we cannot lower
// the arguments first in this situation because we do not know what conversions will be
// produced for the arguments until after we've done overload resolution. So we produce the invocation
// and then lower it along with its arguments.
expressions.Insert(0, format);
var stringType = node.Type;
result = _factory.StaticCall(stringType, "Format", expressions.ToImmutableAndFree(),
allowUnexpandedForm: false // if an interpolation expression is the null literal, it should not match a params parameter.
);
}

Debug.Assert(result is { });
if (!result.HasAnyErrors)
finally
{
result = VisitExpression(result); // lower the arguments AND handle expanded form, argument conversions, etc.
result = MakeImplicitConversionForInterpolatedString(result, node.Type);
_factory.Syntax = oldSyntax;
}
return result;
}

private BoundExpression LowerPartsToString(InterpolatedStringHandlerData data, ImmutableArray<BoundExpression> parts, SyntaxNode syntax, TypeSymbol type)
Expand Down
5 changes: 4 additions & 1 deletion src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_SwitchExpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ private BoundExpression LowerSwitchExpression(BoundConvertedSwitchExpression nod
// When compiling for Debug (not Release), we produce the most detailed sequence points.
var produceDetailedSequencePoints =
GenerateInstrumentation && _localRewriter._compilation.Options.OptimizationLevel != OptimizationLevel.Release;
var oldSyntax = _factory.Syntax;
_factory.Syntax = node.Syntax;
var result = ArrayBuilder<BoundStatement>.GetInstance();
var outerVariables = ArrayBuilder<LocalSymbol>.GetInstance();
Expand Down Expand Up @@ -142,7 +143,9 @@ private BoundExpression LowerSwitchExpression(BoundConvertedSwitchExpression nod

outerVariables.Add(resultTemp);
outerVariables.AddRange(_tempAllocator.AllTemps());
return _factory.SpillSequence(outerVariables.ToImmutableAndFree(), result.ToImmutableAndFree(), _factory.Local(resultTemp));
var sequence = _factory.SpillSequence(outerVariables.ToImmutableAndFree(), result.ToImmutableAndFree(), _factory.Local(resultTemp));
_factory.Syntax = oldSyntax;
return sequence;

bool implicitConversionExists(BoundExpression expression, TypeSymbol type)
{
Expand Down
Loading