Skip to content

Commit

Permalink
PR feedback (1)
Browse files Browse the repository at this point in the history
  • Loading branch information
jcouv authored and jcouv committed Feb 7, 2017
1 parent 4dc80f1 commit 79effee
Show file tree
Hide file tree
Showing 30 changed files with 67 additions and 62 deletions.
11 changes: 8 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ private BoundExpression BindExpressionInternal(ExpressionSyntax node, Diagnostic
return BindDefaultExpression((DefaultExpressionSyntax)node, diagnostics);

case SyntaxKind.DefaultLiteral:
return new BoundDefaultOperator((DefaultLiteralSyntax)node, ConstantValue.DefaultLiteral, type: null);
return BindDefaultLiteral((DefaultLiteralSyntax)node);

case SyntaxKind.TypeOfExpression:
return BindTypeOf((TypeOfExpressionSyntax)node, diagnostics);
Expand Down Expand Up @@ -656,6 +656,11 @@ private BoundExpression BindExpressionInternal(ExpressionSyntax node, Diagnostic
}
}

private static BoundExpression BindDefaultLiteral(DefaultLiteralSyntax node)
{
return new BoundDefaultLiteral(node, ConstantValue.DefaultLiteral, type: null);
}

private BoundExpression BindTupleExpression(TupleExpressionSyntax node, DiagnosticBag diagnostics)
{
SeparatedSyntaxList<ArgumentSyntax> arguments = node.Arguments;
Expand Down Expand Up @@ -970,7 +975,7 @@ internal static ConstantValue GetConstantSizeOf(TypeSymbol type)
private BoundExpression BindDefaultExpression(DefaultExpressionSyntax node, DiagnosticBag diagnostics)
{
TypeSymbol type = this.BindType(node.Type, diagnostics);
return new BoundDefaultOperator(node, type);
return new BoundDefaultLiteral(node, type);
}

/// <summary>
Expand Down Expand Up @@ -5008,7 +5013,7 @@ private BoundExpression BindMemberAccessWithBoundLeft(

private static void WarnOnAccessOfOffDefault(SyntaxNode node, BoundExpression boundLeft, DiagnosticBag diagnostics)
{
if (boundLeft != null && boundLeft.Kind == BoundKind.DefaultOperator && boundLeft.ConstantValue == ConstantValue.Null)
if (boundLeft != null && boundLeft.Kind == BoundKind.DefaultLiteral && boundLeft.ConstantValue == ConstantValue.Null)
{
Error(diagnostics, ErrorCode.WRN_DotOnDefault, node, boundLeft.Type);
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3176,7 +3176,7 @@ private BoundStatement BindReturn(ReturnStatementSyntax syntax, DiagnosticBag di
var interactiveInitializerMethod = this.ContainingMemberOrLambda as SynthesizedInteractiveInitializerMethod;
if (interactiveInitializerMethod != null)
{
arg = new BoundDefaultOperator(interactiveInitializerMethod.GetNonNullSyntaxNode(), interactiveInitializerMethod.ResultType);
arg = new BoundDefaultLiteral(interactiveInitializerMethod.GetNonNullSyntaxNode(), interactiveInitializerMethod.ResultType);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -815,8 +815,8 @@ private Conversion ClassifyImplicitBuiltInConversionFromExpression(BoundExpressi
}
break;

case BoundKind.DefaultOperator:
var defaultExpression = (BoundDefaultOperator)sourceExpression;
case BoundKind.DefaultLiteral:
var defaultExpression = (BoundDefaultLiteral)sourceExpression;
if ((object)defaultExpression.Type == null)
{
return Conversion.DefaultLiteral;
Expand Down Expand Up @@ -975,7 +975,7 @@ internal static bool HasImplicitConstantExpressionConversion(BoundExpression sou
{
var constantValue = source.ConstantValue;

if (constantValue == null || constantValue.IsDefaultLiteral)
if (constantValue == null || (object)source.Type == null)
{
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/BoundTree/BoundExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ public override Symbol ExpressionSymbol
}
}

internal partial class BoundDefaultOperator
internal partial class BoundDefaultLiteral
{
public override ConstantValue ConstantValue
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public static bool IsLiteralNull(this BoundExpression node)

public static bool IsLiteralDefault(this BoundExpression node)
{
return node.Kind == BoundKind.DefaultOperator && (object)node.Type == null;
return node.Kind == BoundKind.DefaultLiteral && (object)node.Type == null;
}

// returns true when expression has no side-effects and produces
Expand All @@ -27,7 +27,7 @@ public static bool IsLiteralDefault(this BoundExpression node)
// after some folding/propagation/algebraic transformations.
public static bool IsDefaultValue(this BoundExpression node)
{
if (node.Kind == BoundKind.DefaultOperator)
if (node.Kind == BoundKind.DefaultLiteral)
{
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@
<Field Name="GetFieldFromHandle" Type="MethodSymbol" Null="allow"/>
</Node>

<Node Name="BoundDefaultOperator" Base="BoundExpression">
<Node Name="BoundDefaultLiteral" Base="BoundExpression">
<!-- Type is null in the case of a default literal, and non-null in the case of a fully-spelled out default operator. -->
<Field Name="Type" Type="TypeSymbol" Override="true" Null="allow"/>
<Field Name="ConstantValueOpt" Type="ConstantValue" Null="allow"/>
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/BoundTree/BoundTreeVisitors.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ public virtual R Visit(BoundNode node, A arg)
return VisitArrayAccess(node as BoundArrayAccess, arg);
case BoundKind.TypeOfOperator:
return VisitTypeOfOperator(node as BoundTypeOfOperator, arg);
case BoundKind.DefaultOperator:
return VisitDefaultOperator(node as BoundDefaultOperator, arg);
case BoundKind.DefaultLiteral:
return VisitDefaultLiteral(node as BoundDefaultLiteral, arg);
case BoundKind.IsOperator:
return VisitIsOperator(node as BoundIsOperator, arg);
case BoundKind.AsOperator:
Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/BoundTree/Constructors.cs
Original file line number Diff line number Diff line change
Expand Up @@ -523,14 +523,14 @@ public static BoundBlock SynthesizedNoLocals(SyntaxNode syntax, params BoundStat
}
}

internal partial class BoundDefaultOperator
internal partial class BoundDefaultLiteral
{
public BoundDefaultOperator(SyntaxNode syntax, TypeSymbol type, bool hasErrors = false)
public BoundDefaultLiteral(SyntaxNode syntax, TypeSymbol type, bool hasErrors = false)
: this(syntax, type.GetDefaultValue(), type, hasErrors)
{
}

public BoundDefaultOperator(SyntaxNode syntax)
public BoundDefaultLiteral(SyntaxNode syntax)
: this(syntax, constantValueOpt: null, type: null, hasErrors: false)
{
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/BoundTree/Expression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -937,7 +937,7 @@ public override TResult Accept<TArgument, TResult>(OperationVisitor<TArgument, T
}
}

internal partial class BoundDefaultOperator : IDefaultValueExpression
internal partial class BoundDefaultLiteral : IDefaultValueExpression
{
protected override OperationKind ExpressionKind => OperationKind.DefaultValueExpression;

Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/BoundTree/Formatting.cs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public override object Display
}
}

internal partial class BoundDefaultOperator
internal partial class BoundDefaultLiteral
{
public override object Display
{
Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,8 @@ private void EmitExpressionCore(BoundExpression expression, bool used)
EmitAsExpression((BoundAsOperator)expression, used);
break;

case BoundKind.DefaultOperator:
EmitDefaultExpression((BoundDefaultOperator)expression, used);
case BoundKind.DefaultLiteral:
EmitDefaultExpression((BoundDefaultLiteral)expression, used);
break;

case BoundKind.TypeOfOperator:
Expand Down Expand Up @@ -2617,7 +2617,7 @@ private void EmitDefaultValue(TypeSymbol type, bool used, SyntaxNode syntaxNode)
}
}

private void EmitDefaultExpression(BoundDefaultOperator expression, bool used)
private void EmitDefaultExpression(BoundDefaultLiteral expression, bool used)
{
Debug.Assert(expression.Type.SpecialType == SpecialType.System_Decimal ||
expression.Type.GetDefaultValue() == null, "constant should be set on this expression");
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/CodeGen/Optimizer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1405,7 +1405,7 @@ public override BoundNode VisitUnaryOperator(BoundUnaryOperator node)
if (node.OperatorKind.IsChecked() && node.OperatorKind.Operator() == UnaryOperatorKind.UnaryMinus)
{
var origStack = StackDepth();
PushEvalStack(new BoundDefaultOperator(node.Syntax, node.Operand.Type), ExprContext.Value);
PushEvalStack(new BoundDefaultLiteral(node.Syntax, node.Operand.Type), ExprContext.Value);
BoundExpression operand = (BoundExpression)this.Visit(node.Operand);
return node.Update(node.OperatorKind, operand, node.ConstantValueOpt, node.MethodOpt, node.ResultKind, node.Type);
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1173,7 +1173,7 @@ private static BoundStatement ChainImplicitStructConstructor(MethodSymbol method
new BoundAssignmentOperator(
syntax,
new BoundThisReference(syntax, containingType),
new BoundDefaultOperator(syntax, containingType),
new BoundDefaultLiteral(syntax, containingType),
RefKind.None,
containingType));
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,7 @@ internal static bool WriteConsideredUse(TypeSymbol type, BoundExpression value)
}
return WriteConsideredUse(null, boundConversion.Operand);
}
case BoundKind.DefaultOperator:
case BoundKind.DefaultLiteral:
return false;
case BoundKind.ObjectCreationExpression:
var init = (BoundObjectCreationExpression)value;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public static BoundBlock Rewrite(
{
Debug.Assert(submissionResultType.SpecialType != SpecialType.System_Void);

var trailingExpression = new BoundDefaultOperator(method.GetNonNullSyntaxNode(), submissionResultType);
var trailingExpression = new BoundDefaultLiteral(method.GetNonNullSyntaxNode(), submissionResultType);
var newStatements = block.Statements.Add(new BoundReturnStatement(trailingExpression.Syntax, RefKind.None, trailingExpression));
block = new BoundBlock(block.Syntax, ImmutableArray<LocalSymbol>.Empty, newStatements) { WasCompilerGenerated = true };
#if DEBUG
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2449,7 +2449,7 @@ public override BoundNode VisitYieldReturnStatement(BoundYieldReturnStatement no
return null;
}

public override BoundNode VisitDefaultOperator(BoundDefaultOperator node)
public override BoundNode VisitDefaultLiteral(BoundDefaultLiteral node)
{
return null;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Lowering/Extensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public static bool NullableNeverHasValue(this BoundExpression expr)
}

// "default(int?)" never has a value.
if (expr.Kind == BoundKind.DefaultOperator)
if (expr.Kind == BoundKind.DefaultLiteral)
{
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ private BoundExpression VisitExpressionWithoutStackGuard(BoundExpression node)
case BoundKind.UnaryOperator:
return VisitUnaryOperator((BoundUnaryOperator)node);

case BoundKind.DefaultOperator:
case BoundKind.DefaultLiteral:
case BoundKind.HostObjectMemberReference:
case BoundKind.Literal:
case BoundKind.Local:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ private T IntroduceFrame<T>(BoundNode node, LambdaFrame frame, Func<ArrayBuilder
if (frame.Constructor == null)
{
Debug.Assert(frame.TypeKind == TypeKind.Struct);
newFrame = new BoundDefaultOperator(syntax: syntax, type: frameType);
newFrame = new BoundDefaultLiteral(syntax: syntax, type: frameType);
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ private BoundExpression MakeAsOperator(
if (constantValue != null)
{
Debug.Assert(constantValue.IsNull);
BoundExpression result = rewrittenType.IsNullableType() ? new BoundDefaultOperator(syntax, rewrittenType) : MakeLiteral(syntax, constantValue, rewrittenType);
BoundExpression result = rewrittenType.IsNullableType() ? new BoundDefaultLiteral(syntax, rewrittenType) : MakeLiteral(syntax, constantValue, rewrittenType);

if (rewrittenOperand.ConstantValue != null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1158,7 +1158,7 @@ private BoundExpression TrivialLiftedBinaryArithmeticOptimizations(
if (leftAlwaysNull && rightAlwaysNull)
{
// default(R?)
return new BoundDefaultOperator(syntax, null, type);
return new BoundDefaultLiteral(syntax, null, type);
}

// Optimization #2: If both sides are non-null then we can again eliminate the lifting entirely.
Expand Down Expand Up @@ -1230,14 +1230,14 @@ private static BoundExpression OptimizeLiftedArithmeticOperatorOneNull(

if (sideEffect.ConstantValue != null)
{
return new BoundDefaultOperator(syntax, null, type);
return new BoundDefaultLiteral(syntax, null, type);
}

return new BoundSequence(
syntax: syntax,
locals: ImmutableArray<LocalSymbol>.Empty,
sideEffects: ImmutableArray.Create<BoundExpression>(sideEffect),
value: new BoundDefaultOperator(syntax, null, type),
value: new BoundDefaultLiteral(syntax, null, type),
type: type);
}

Expand Down Expand Up @@ -1302,7 +1302,7 @@ private BoundExpression LowerLiftedBinaryArithmeticOperator(
BoundExpression consequence = MakeLiftedBinaryOperatorConsequence(syntax, kind, callX_GetValueOrDefault, callY_GetValueOrDefault, type, method);

// default(R?)
BoundExpression alternative = new BoundDefaultOperator(syntax, null, type);
BoundExpression alternative = new BoundDefaultLiteral(syntax, null, type);

// tempX.HasValue & tempY.HasValue ?
// new R?(tempX.GetValueOrDefault() OP tempY.GetValueOrDefault()) :
Expand Down Expand Up @@ -1478,7 +1478,7 @@ private BoundExpression MakeNewNullableBoolean(SyntaxNode syntax, bool? value)
NamedTypeSymbol nullableBoolType = nullableType.Construct(boolType);
if (value == null)
{
return new BoundDefaultOperator(syntax, null, nullableBoolType);
return new BoundDefaultLiteral(syntax, null, nullableBoolType);
}
return new BoundObjectCreationExpression(
syntax,
Expand Down Expand Up @@ -1520,7 +1520,7 @@ private BoundExpression OptimizeLiftedBooleanOperatorOneNull(
BoundExpression alwaysNull = leftAlwaysNull ? left : right;
BoundExpression notAlwaysNull = leftAlwaysNull ? right : left;
BoundExpression neverNull = NullableAlwaysHasValue(notAlwaysNull);
BoundExpression nullBool = new BoundDefaultOperator(syntax, null, alwaysNull.Type);
BoundExpression nullBool = new BoundDefaultLiteral(syntax, null, alwaysNull.Type);

if (neverNull != null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -990,14 +990,14 @@ private BoundExpression GetDefaultParameterValue(SyntaxNode syntax, ParameterSym
else
{
// The argument to M([Optional] int x) becomes default(int)
defaultValue = new BoundDefaultOperator(syntax, parameterType);
defaultValue = new BoundDefaultLiteral(syntax, parameterType);
}
}
else if (defaultConstantValue.IsNull && parameterType.IsValueType)
{
// We have something like M(int? x = null) or M(S x = default(S)),
// so replace the argument with default(int?).
defaultValue = new BoundDefaultOperator(syntax, parameterType);
defaultValue = new BoundDefaultLiteral(syntax, parameterType);
}
else if (parameterType.IsNullableType())
{
Expand Down Expand Up @@ -1053,20 +1053,20 @@ private BoundExpression GetDefaultParameterSpecial(SyntaxNode syntax, ParameterS
if (parameter.IsMarshalAsObject)
{
// default(object)
defaultValue = new BoundDefaultOperator(syntax, parameter.Type);
defaultValue = new BoundDefaultLiteral(syntax, parameter.Type);
}
else if (parameter.IsIUnknownConstant)
{
// new UnknownWrapper(default(object))
var methodSymbol = (MethodSymbol)_compilation.GetWellKnownTypeMember(WellKnownMember.System_Runtime_InteropServices_UnknownWrapper__ctor);
var argument = new BoundDefaultOperator(syntax, parameter.Type);
var argument = new BoundDefaultLiteral(syntax, parameter.Type);
defaultValue = new BoundObjectCreationExpression(syntax, methodSymbol, argument);
}
else if (parameter.IsIDispatchConstant)
{
// new DispatchWrapper(default(object))
var methodSymbol = (MethodSymbol)_compilation.GetWellKnownTypeMember(WellKnownMember.System_Runtime_InteropServices_DispatchWrapper__ctor);
var argument = new BoundDefaultOperator(syntax, parameter.Type);
var argument = new BoundDefaultLiteral(syntax, parameter.Type);
defaultValue = new BoundObjectCreationExpression(syntax, methodSymbol, argument);
}
else
Expand Down
Loading

0 comments on commit 79effee

Please sign in to comment.