Skip to content

Commit

Permalink
Check GetConstantValue and GetConversion on target-typed default (#17205
Browse files Browse the repository at this point in the history
)
  • Loading branch information
jcouv authored Mar 3, 2017
1 parent 582255b commit 533aa4b
Show file tree
Hide file tree
Showing 22 changed files with 212 additions and 80 deletions.
26 changes: 20 additions & 6 deletions src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Roslyn.Utilities;
using System.Collections.Immutable;
using System;

namespace Microsoft.CodeAnalysis.CSharp
{
Expand Down Expand Up @@ -103,6 +104,11 @@ protected BoundExpression CreateConversion(
}

ConstantValue constantValue = this.FoldConstantConversion(syntax, source, conversion, destination, diagnostics);
if (conversion.Kind == ConversionKind.DefaultOrNullLiteral && source.Kind == BoundKind.DefaultLiteral)
{
source = ((BoundDefaultLiteral)source).Update(constantValue, destination);
}

return new BoundConversion(
syntax,
source,
Expand Down Expand Up @@ -863,7 +869,18 @@ public ConstantValue FoldConstantConversion(
}

var sourceConstantValue = source.ConstantValue;
if (sourceConstantValue == null || sourceConstantValue.IsBad)
if (sourceConstantValue == null)
{
if (conversion.Kind == ConversionKind.DefaultOrNullLiteral && source.Kind == BoundKind.DefaultLiteral)
{
return destination.GetDefaultValue();
}
else
{
return sourceConstantValue;
}
}
else if (sourceConstantValue.IsBad)
{
return sourceConstantValue;
}
Expand All @@ -884,13 +901,10 @@ public ConstantValue FoldConstantConversion(
return sourceConstantValue;
}

case ConversionKind.NullLiteral:
case ConversionKind.DefaultOrNullLiteral:
// 'default' case is handled above, 'null' is handled here
return sourceConstantValue;

case ConversionKind.DefaultLiteral:
Debug.Assert(sourceConstantValue.IsDefaultLiteral);
return destination.GetDefaultValue();

case ConversionKind.ImplicitConstant:
return FoldConstantNumericConversion(syntax, sourceConstantValue, destination, diagnostics);

Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ private BoundExpression BindExpressionInternal(ExpressionSyntax node, Diagnostic

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

private BoundExpression BindTupleExpression(TupleExpressionSyntax node, DiagnosticBag diagnostics)
Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2628,7 +2628,7 @@ private BoundExpression BindIsOperator(BinaryExpressionSyntax node, DiagnosticBa

HashSet<DiagnosticInfo> useSiteDiagnostics = null;

if (operand.ConstantValue == ConstantValue.DefaultLiteral)
if (operand.Kind == BoundKind.DefaultLiteral && operand.ConstantValue == null)
{
Error(diagnostics, ErrorCode.ERR_DefaultNotValid, node, targetType);
return new BoundIsOperator(node, operand, typeExpression, Conversion.NoConversion, resultType, hasErrors: true);
Expand Down Expand Up @@ -2936,7 +2936,7 @@ internal static ConstantValue GetIsOperatorConstantResult(TypeSymbol operandType
case ConversionKind.IntegerToPointer:
case ConversionKind.NullToPointer:
case ConversionKind.AnonymousFunction:
case ConversionKind.NullLiteral:
case ConversionKind.DefaultOrNullLiteral:
case ConversionKind.MethodGroup:
// We've either replaced Dynamic with Object, or already bailed out with an error.
throw ExceptionUtilities.UnexpectedValue(conversionKind);
Expand Down Expand Up @@ -3022,7 +3022,7 @@ private BoundExpression BindAsOperator(BinaryExpressionSyntax node, DiagnosticBa
// We do not want to warn for the case "null as TYPE" where the null
// is a literal, because the user might be saying it to cause overload resolution
// to pick a particular method
return new BoundAsOperator(node, operand, typeExpression, Conversion.NullLiteral, resultType);
return new BoundAsOperator(node, operand, typeExpression, Conversion.DefaultOrNullLiteral, resultType);
}

if (operand.Kind == BoundKind.MethodGroup)
Expand Down
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs
Original file line number Diff line number Diff line change
Expand Up @@ -216,10 +216,11 @@ private bool CheckValidPatternType(
case ConversionKind.Identity:
case ConversionKind.ImplicitReference:
case ConversionKind.Unboxing:
case ConversionKind.NullLiteral:
case ConversionKind.ImplicitNullable:
// these are the conversions allowed by a pattern match
break;
case ConversionKind.DefaultOrNullLiteral:
throw ExceptionUtilities.UnexpectedValue(conversion.Kind);
//case ConversionKind.ExplicitNumeric: // we do not perform numeric conversions of the operand
//case ConversionKind.ImplicitConstant:
//case ConversionKind.ImplicitNumeric:
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 @@ -1028,7 +1028,7 @@ private bool IsValidFixedVariableInitializer(TypeSymbol declType, SourceLocalSym
{
Debug.Assert(initializerOpt.Kind == BoundKind.Conversion &&
(((BoundConversion)initializerOpt).Operand.IsLiteralNull() ||
((BoundConversion)initializerOpt).Operand.IsLiteralDefault()),
((BoundConversion)initializerOpt).Operand.Kind == BoundKind.DefaultLiteral),
"All other typeless expressions should have conversion errors");

// CONSIDER: this is a very confusing error message, but it's what Dev10 reports.
Expand Down
19 changes: 10 additions & 9 deletions src/Compilers/CSharp/Portable/Binder/ForEachLoopBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -396,19 +396,20 @@ private bool GetEnumeratorInfo(ref ForEachEnumeratorInfo.Builder builder, BoundE
diagnostics.Add(ErrorCode.ERR_NullNotValid, _syntax.Expression.Location);
return false;
}
else if (collectionExpr.ConstantValue.IsDefaultLiteral)
{
diagnostics.Add(ErrorCode.ERR_DefaultNotValid, _syntax.Expression.Location);
return false;
}
}

if ((object)collectionExprType == null) // There's no way to enumerate something without a type.
{
// The null literal was caught above, so anything else with a null type is a method group or anonymous function
diagnostics.Add(ErrorCode.ERR_AnonMethGrpInForEach, _syntax.Expression.Location, collectionExpr.Display);
// CONSIDER: dev10 also reports ERR_ForEachMissingMember (i.e. failed pattern match).

if (collectionExpr.Kind == BoundKind.DefaultLiteral && (object)collectionExpr.Type == null)
{
diagnostics.Add(ErrorCode.ERR_DefaultNotValid, _syntax.Expression.Location);
}
else
{
// The null and default literals were caught above, so anything else with a null type is a method group or anonymous function
diagnostics.Add(ErrorCode.ERR_AnonMethGrpInForEach, _syntax.Expression.Location, collectionExpr.Display);
// CONSIDER: dev10 also reports ERR_ForEachMissingMember (i.e. failed pattern match).
}
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ private static void AssertTrivialConversion(ConversionKind kind)
case ConversionKind.ImplicitEnumeration:
case ConversionKind.AnonymousFunction:
case ConversionKind.Boxing:
case ConversionKind.NullLiteral:
case ConversionKind.DefaultOrNullLiteral:
case ConversionKind.NullToPointer:
case ConversionKind.PointerToVoid:
case ConversionKind.PointerToPointer:
Expand Down Expand Up @@ -197,8 +197,7 @@ internal static Conversion GetTrivialConversion(ConversionKind kind)
internal static Conversion ImplicitEnumeration => new Conversion(ConversionKind.ImplicitEnumeration);
internal static Conversion AnonymousFunction => new Conversion(ConversionKind.AnonymousFunction);
internal static Conversion Boxing => new Conversion(ConversionKind.Boxing);
internal static Conversion NullLiteral => new Conversion(ConversionKind.NullLiteral);
internal static Conversion DefaultLiteral => new Conversion(ConversionKind.DefaultLiteral);
internal static Conversion DefaultOrNullLiteral => new Conversion(ConversionKind.DefaultOrNullLiteral);
internal static Conversion NullToPointer => new Conversion(ConversionKind.NullToPointer);
internal static Conversion PointerToVoid => new Conversion(ConversionKind.PointerToVoid);
internal static Conversion PointerToPointer => new Conversion(ConversionKind.PointerToPointer);
Expand Down Expand Up @@ -550,16 +549,16 @@ public bool IsUnboxing
}

/// <summary>
/// Returns true if the conversion is an implicit null literal conversion.
/// Returns true if the conversion is an implicit null or default literal conversion.
/// </summary>
/// <remarks>
/// Null literal conversions are described in section 6.1.5 of the C# language specification.
/// Null or default literal conversions are described in section 6.1.5 of the C# language specification.
/// </remarks>
public bool IsNullLiteral
{
get
{
return Kind == ConversionKind.NullLiteral;
return Kind == ConversionKind.DefaultOrNullLiteral;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ internal enum ConversionKind : byte
ExplicitTupleLiteral,
ExplicitTuple,
ImplicitNullable,
NullLiteral,
DefaultOrNullLiteral,
ImplicitReference,
Boxing,
PointerToVoid,
Expand All @@ -41,6 +41,5 @@ internal enum ConversionKind : byte
// implement them for compatibility with the native compiler.
IntPtr,
InterpolatedString, // a conversion from an interpolated string to IFormattable or FormattableString
DefaultLiteral,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public static bool IsImplicitConversion(this ConversionKind conversionKind)
case ConversionKind.ImplicitTuple:
case ConversionKind.ImplicitEnumeration:
case ConversionKind.ImplicitNullable:
case ConversionKind.NullLiteral:
case ConversionKind.DefaultOrNullLiteral:
case ConversionKind.ImplicitReference:
case ConversionKind.Boxing:
case ConversionKind.ImplicitDynamic:
Expand All @@ -40,7 +40,6 @@ public static bool IsImplicitConversion(this ConversionKind conversionKind)
case ConversionKind.PointerToVoid:
case ConversionKind.NullToPointer:
case ConversionKind.InterpolatedString:
case ConversionKind.DefaultLiteral:
return true;

case ConversionKind.ExplicitNumeric:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -807,19 +807,19 @@ private Conversion ClassifyImplicitBuiltInConversionFromExpression(BoundExpressi
}
break;

case BoundKind.TupleLiteral:
var tupleConversion = ClassifyImplicitTupleLiteralConversion((BoundTupleLiteral)sourceExpression, destination, ref useSiteDiagnostics);
if (tupleConversion.Exists)
case BoundKind.DefaultLiteral:
var defaultExpression = (BoundDefaultLiteral)sourceExpression;
if ((object)defaultExpression.Type == null)
{
return tupleConversion;
return Conversion.DefaultOrNullLiteral;
}
break;

case BoundKind.DefaultLiteral:
var defaultExpression = (BoundDefaultLiteral)sourceExpression;
if ((object)defaultExpression.Type == null)
case BoundKind.TupleLiteral:
var tupleConversion = ClassifyImplicitTupleLiteralConversion((BoundTupleLiteral)sourceExpression, destination, ref useSiteDiagnostics);
if (tupleConversion.Exists)
{
return Conversion.DefaultLiteral;
return tupleConversion;
}
break;

Expand Down Expand Up @@ -865,7 +865,7 @@ private static Conversion ClassifyNullLiteralConversion(BoundExpression source,
{
// The spec defines a "null literal conversion" specifically as a conversion from
// null to nullable type.
return Conversion.NullLiteral;
return Conversion.DefaultOrNullLiteral;
}

// SPEC: An implicit conversion exists from the null literal to any reference type.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ private static bool IsEncompassingImplicitConversionKind(ConversionKind kind)
case ConversionKind.PointerToVoid:

// Added to spec in Roslyn timeframe.
case ConversionKind.NullLiteral:
case ConversionKind.DefaultOrNullLiteral: // updated to include "default" in C# 7.1
case ConversionKind.NullToPointer:

// Added for C# 7.
Expand All @@ -606,9 +606,6 @@ private static bool IsEncompassingImplicitConversionKind(ConversionKind kind)
case ConversionKind.ExplicitTuple:
return false;

case ConversionKind.DefaultLiteral:
return true;

default:
throw ExceptionUtilities.UnexpectedValue(kind);
}
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 @@ -735,7 +735,7 @@ Semantics.ConversionKind IConversionExpression.ConversionKind
case CSharp.ConversionKind.ImplicitConstant:
case CSharp.ConversionKind.IntegerToPointer:
case CSharp.ConversionKind.IntPtr:
case CSharp.ConversionKind.NullLiteral:
case CSharp.ConversionKind.DefaultOrNullLiteral:
case CSharp.ConversionKind.NullToPointer:
case CSharp.ConversionKind.PointerToInteger:
case CSharp.ConversionKind.PointerToPointer:
Expand Down
5 changes: 3 additions & 2 deletions src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2989,9 +2989,10 @@ private TypeSymbol StackMergeType(BoundExpression expr)
case BoundKind.Conversion:
var conversion = (BoundConversion)expr;
var conversionKind = conversion.ConversionKind;
Debug.Assert(conversionKind != ConversionKind.DefaultOrNullLiteral);

if (conversionKind.IsImplicitConversion() &&
conversionKind != ConversionKind.MethodGroup &&
conversionKind != ConversionKind.NullLiteral)
conversionKind != ConversionKind.MethodGroup)
{
return StackMergeType(conversion.Operand);
}
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Lowering/Extensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ public static bool NullableNeverHasValue(this BoundExpression expr)
var conversion = (BoundConversion)expr;
switch (conversion.ConversionKind)
{
case ConversionKind.NullLiteral:
// Any "null literal conversion" is a conversion from the literal null to
case ConversionKind.DefaultOrNullLiteral:
// Any "null literal conversion" is a conversion from the literals null/default to
// a nullable value type; obviously it never has a value.
return true;
case ConversionKind.ImplicitNullable:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ private static BoundExpression DemoteEnumOperand(BoundExpression operand)
var conversion = (BoundConversion)operand;
if (!conversion.ConversionKind.IsUserDefinedConversion() &&
conversion.ConversionKind.IsImplicitConversion() &&
conversion.ConversionKind != ConversionKind.NullLiteral &&
conversion.ConversionKind != ConversionKind.DefaultOrNullLiteral &&
conversion.Type.StrippedType().IsEnumType())
{
operand = conversion.Operand;
Expand Down Expand Up @@ -643,7 +643,8 @@ private BoundExpression VisitConversion(BoundConversion node)
var e1 = Convert(Visit(node.Operand), node.Operand.Type, intermediate, node.Checked, false);
return Convert(e1, intermediate, node.Type, node.Checked, false);
}
case ConversionKind.NullLiteral:
case ConversionKind.DefaultOrNullLiteral:
// PROTOTYPE(default): how do we reach this code with `default` and does it need to be fixed to handle it?
return Convert(Constant(_bound.Null(_objectType)), _objectType, node.Type, false, node.ExplicitCastInCode);
default:
return Convert(Visit(node.Operand), node.Operand.Type, node.Type, node.Checked, node.ExplicitCastInCode);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ private static bool IsSafeForReordering(BoundExpression expression, RefKind kind
case ConversionKind.AnonymousFunction:
case ConversionKind.ImplicitConstant:
case ConversionKind.MethodGroup:
case ConversionKind.NullLiteral:
case ConversionKind.DefaultOrNullLiteral:
return true;

case ConversionKind.Boxing:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,7 @@ private BoundExpression MakeConversionNode(
}
break;

case ConversionKind.NullLiteral:
case ConversionKind.DefaultLiteral:
case ConversionKind.DefaultOrNullLiteral:
if (!_inExpressionLambda || !explicitCastInCode)
{
return new BoundDefaultLiteral(syntax, rewrittenType);
Expand Down
43 changes: 43 additions & 0 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenExprLambdaTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4971,6 +4971,49 @@ static void Main(string[] args)
expectedOutput: expectedOutput);
}

[Fact]
public void EnumEqualityWithDefault()
{
string source =
@"
using System;
using System.Linq.Expressions;
namespace ConsoleApplication1
{
enum YesNo
{
Yes,
No
}
class MyType
{
public string Name { get; set; }
public YesNo? YesNo { get; set; }
public int? Age { get; set; }
}
class Program
{
static void Main(string[] args)
{
Expression<Func<MyType, bool>> expr = (MyType x) => x.YesNo == (YesNo?)default;
Console.WriteLine(expr.Dump());
}
}
}";
string expectedOutput = "Equal(Convert(MemberAccess(Parameter(x Type:ConsoleApplication1.MyType).YesNo Type:System.Nullable`1[ConsoleApplication1.YesNo]) Lifted LiftedToNull Type:System.Nullable`1[System.Int32]) Convert(Convert(Constant(null Type:System.Object) Lifted LiftedToNull Type:System.Nullable`1[ConsoleApplication1.YesNo]) Lifted LiftedToNull Type:System.Nullable`1[System.Int32]) Lifted Type:System.Boolean)";
CompileAndVerify(
new[] { source, ExpressionTestLibrary },
new[] { ExpressionAssemblyRef },
expectedOutput: expectedOutput,
parseOptions: TestOptions.ExperimentalParseOptions);
}

[WorkItem(546618, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/546618")]
[Fact]
public void TildeNullableEnum()
Expand Down
Loading

0 comments on commit 533aa4b

Please sign in to comment.