Skip to content

Commit

Permalink
Save and restore '_factory.Syntax' in LocalRewriter methods
Browse files Browse the repository at this point in the history
  • Loading branch information
@cston
cston committed Feb 7, 2022
1 parent b8d2e61 commit ea559d7
Show file tree
Hide file tree
Showing 8 changed files with 170 additions and 92 deletions.
9 changes: 9 additions & 0 deletions src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter.DecisionDagRewriter.cs
View file
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
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
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
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
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
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

0 comments on commit ea559d7

Please sign in to comment.