Check GetConstantValue and GetConversion on target-typed default #17205

jcouv · 2017-02-17T05:45:51Z

I noticed two more semantic model APIs that need coverage.

GetConstantValue should behave as it does on default(...).
In the current implementation, when the type of the default literal is inferred, the bound node remains the same (typeless and without a constant value) and it is wrapped with a bound conversion. I'd like to get your feedback on that.

Relates to umbrella item for "default" (#13602)
@dotnet/roslyn-compiler for review.

jcouv · 2017-02-21T17:52:04Z

@AlekseyTs Small semantic model change for "default" literal.
Another design would be to update the type in the bound node at the time the type the type is inferred. Let me know if you think that'd be better.

AlekseyTs · 2017-02-21T22:39:42Z

src/Compilers/Core/Portable/ConstantValue.cs

@@ -439,6 +439,7 @@ public object Value
                {
                    case ConstantValueTypeDiscriminator.Bad: return null;
                    case ConstantValueTypeDiscriminator.Null: return null;
+                    case ConstantValueTypeDiscriminator.DefaultLiteral: return null;


case ConstantValueTypeDiscriminator.DefaultLiteral: return null; [](start = 20, length = 64)

case ConstantValueTypeDiscriminator.DefaultLiteral: return null; [](start = 20, length = 64)

It feels unexpected that we ended up with a special discriminator for this feature. This is pretty much the same as Nothing literal in VB and we managed to support it without special discriminator. #Closed

Right. The value should be what ConstantValue.Default returns.

I'm having some more trouble with constant values. The trouble is that expressions like default(int?) and default(int*) don't have constant values (see GetDefaultValue).

That means that default in int? x = default; shouldn't have a constant value. Does that seem right?

Also, I explored binding default as a BoundConversion with a null literal conversion (instead of creating a new conversion kind) and a BoundLiteralExpression as operand. But I think that literals should have a constant value. Is that correct?

I'm having some more trouble with constant values.

I don't think of default as a literal. It represents a default value of a type. Each type has a default value, but not all default values are constants from the language point of view. Let's talk offline in more details. #Closed

AlekseyTs · 2017-02-21T22:49:23Z

src/Compilers/CSharp/Portable/Compilation/MemberSemanticModel.cs

@@ -980,6 +980,11 @@ internal override Optional<object> GetConstantValueWorker(CSharpSyntaxNode node,

            if (boundExpr == null) return default(Optional<object>);

+            if (boundExpr.Kind == BoundKind.DefaultLiteral)


if (boundExpr.Kind == BoundKind.DefaultLiteral) [](start = 12, length = 47)

It is not clear why can't we make this decision based on the syntax.

AlekseyTs · 2017-02-21T22:52:40Z

src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs

+            var model = comp.GetSemanticModel(tree);
+            var def = tree.GetCompilationUnitRoot().DescendantNodes().OfType<DefaultLiteralSyntax>().Single();
+
+            Assert.Equal(0, model.GetConstantValue(def).Value);


model.GetConstantValue(def).Value [](start = 28, length = 33)

model.GetConstantValue(def).Value [](start = 28, length = 33)

What type is returned by SemanticModel for the "def" node here? That type and the type of the constant value should agree. #Closed

AlekseyTs · 2017-02-21T22:53:15Z

src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs

@@ -325,6 +354,10 @@ static void Main()
            Assert.Null(model.GetTypeInfo(def).Type);
            Assert.Equal("System.Int32", model.GetTypeInfo(def).ConvertedType.ToTestDisplayString());
            Assert.Null(model.GetSymbolInfo(def).Symbol);
+            Assert.Equal(0, model.GetConstantValue(def).Value);


Assert.Equal(0, model.GetConstantValue(def).Value); [](start = 12, length = 51)

Assert.Equal(0, model.GetConstantValue(def).Value); [](start = 12, length = 51)

What type is returned by SemanticModel for the "def" node here? That type and the type of the constant value should agree. #Closed

It doesn't look like they agree.

In reply to: 102340395 [](ancestors = 102340395)

AlekseyTs · 2017-02-21T22:54:03Z

src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs

@@ -427,6 +460,7 @@ static void Main()
            Assert.Null(model.GetTypeInfo(def).Type);
            Assert.Null(model.GetSymbolInfo(def).Symbol);
            Assert.Null(model.GetDeclaredSymbol(def));
+            Assert.Equal(0, model.GetConstantValue(def).Value);


Assert.Equal(0, model.GetConstantValue(def).Value); [](start = 12, length = 51)

Assert.Equal(0, model.GetConstantValue(def).Value); [](start = 12, length = 51)

What type is returned by SemanticModel for the "def" node here? That type and the type of the constant value should agree. #Closed

It doesn't look like they agree.

In reply to: 102340554 [](ancestors = 102340554)

I don't think I fully agree with that. The type of the constant value should agree with the converted type of def.

For comparison, for null, the Type is null, the ConvertedType is the inferred type and the constant is null (which agrees with ConvertedType):

public void AssignNullToString() { string source = @" class C { static void Main() { string x = null; System.Console.Write(x); } } "; var comp = CreateCompilationWithMscorlib(source, parseOptions: TestOptions.ExperimentalParseOptions); comp.VerifyDiagnostics(); var tree = comp.SyntaxTrees.First(); var model = comp.GetSemanticModel(tree); var def = tree.GetCompilationUnitRoot().DescendantNodes().OfType<LiteralExpressionSyntax>().Single(); Assert.Null(model.GetConstantValue(def).Value); Assert.Equal("System.String", model.GetTypeInfo(def).ConvertedType.ToTestDisplayString()); Assert.Null(model.GetTypeInfo(def).Type); }

The null literal is special, it will only "match" reference types and will match any reference type. So, its constant value is null regardless of the target type. As in these scenarios, for example:

[Fact] public void AssignNullToInt() { string source = @" class C { static void Main() { int x = null; System.Console.Write(x); } } "; var comp = CreateCompilationWithMscorlib(source, parseOptions: TestOptions.ExperimentalParseOptions); var tree = comp.SyntaxTrees.First(); var model = comp.GetSemanticModel(tree); var def = tree.GetCompilationUnitRoot().DescendantNodes().OfType<LiteralExpressionSyntax>().Single(); Assert.Equal("null", def.ToString()); Assert.True(model.GetConstantValue(def).HasValue); Assert.Null(model.GetConstantValue(def).Value); Assert.Equal("System.Int32", model.GetTypeInfo(def).ConvertedType.ToTestDisplayString()); Assert.Null(model.GetTypeInfo(def).Type); } [Fact] public void AssignNullToNullableInt() { string source = @" class C { static void Main() { int? x = null; System.Console.Write(x); } } "; var comp = CreateCompilationWithMscorlib(source, parseOptions: TestOptions.ExperimentalParseOptions); var tree = comp.SyntaxTrees.First(); var model = comp.GetSemanticModel(tree); var def = tree.GetCompilationUnitRoot().DescendantNodes().OfType<LiteralExpressionSyntax>().Single(); Assert.Equal("null", def.ToString()); Assert.True(model.GetConstantValue(def).HasValue); Assert.Null(model.GetConstantValue(def).Value); Assert.Equal("System.Int32?", model.GetTypeInfo(def).ConvertedType.ToTestDisplayString()); Assert.Null(model.GetTypeInfo(def).Type); }

Clearly null value doesn't match the ConvertedType in cases like this, its the value of the null literal regardless of the target type.

However, if we look at literals of value types:

[Fact] public void AssignIntToLong() { string source = @" class C { static void Main() { long x = 1; System.Console.Write(x); } } "; var comp = CreateCompilationWithMscorlib(source, parseOptions: TestOptions.ExperimentalParseOptions); comp.VerifyDiagnostics(); var tree = comp.SyntaxTrees.First(); var model = comp.GetSemanticModel(tree); var def = tree.GetCompilationUnitRoot().DescendantNodes().OfType<LiteralExpressionSyntax>().Single(); Assert.Equal(1, model.GetConstantValue(def).Value); Assert.Equal(typeof(int), model.GetConstantValue(def).Value.GetType()); Assert.Equal("System.Int64", model.GetTypeInfo(def).ConvertedType.ToTestDisplayString()); Assert.Equal("System.Int32", model.GetTypeInfo(def).Type.ToTestDisplayString()); }

We can observe that the constant value matches the Type, not the ConvertedType.

I understand that default is special in the way that it doesn't have natural type (similar to null). At the same time it doesn't have a "natural" constant value that is going to be valid for any target type in an non-error scenario. In fact, for some target types there could be no valid constant value. Given that, it would feel right to me If the Type (not Converted type) and the ConstantValue of the node were both based on the target type. But I could be convinced that the Type should always bee null. I would still like to avoid any special cases in SemanticModel to get this behavior. #Closed

AlekseyTs · 2017-02-21T23:07:36Z

@jcouv

Another design would be to update the type in the bound node at the time the type the type is inferred. Let me know if you think that'd be better.

It is hard to say at the moment. It feels like it would be ideal to avoid adding special handling into SemanticModel because it is very likely that we will have to do that for different APIs so that they work in concert. #Closed

gafter · 2017-02-22T00:48:15Z

This is analogous to the way null is handled: E1 = null is bound to

assignmentOperator
├─left
│ └─eventAccess
│   ├─receiverOpt
│   │ └─thisReference
│   │   └─type: Program
│   ├─eventSymbol: event System.EventHandler Program.E1
│   ├─isUsableAsField: True
│   ├─resultKind: Viable
│   └─type: System.EventHandler
├─right
│ └─conversion
│   ├─operand
│   │ └─literal
│   │   ├─constantValueOpt: ConstantValueNull(null: Null)
│   │   └─type
│   ├─conversion: ImplicitReference
│   ├─isBaseConversion: False
│   ├─@checked: False
│   ├─explicitCastInCode: False
│   ├─constantValueOpt: ConstantValueNull(null: Null)
│   └─type: System.EventHandler
├─refKind: None
└─type: System.EventHandler

AlekseyTs · 2017-02-22T00:57:25Z

@gafter

This is analogous to the way null is handled: E1 = null is bound to ...

Could you please elaborate what source location or message this is related to, if any.
#Closed

gafter · 2017-02-22T17:28:59Z

This is related to the suggestion that GetTypeInfo on default should return a value whose Type property describes the type of the resulting constant value. I think that should be true of the ConvertedType property of the returned value from GetTypeInfo, not the Type property.

jcouv · 2017-02-23T18:54:09Z

@AlekseyTs @gafter I've re-purposed and renamed the "null literal" conversion as we discussed.

gafter

Looks good, presuming this is the right approach to take.

gafter · 2017-02-23T20:26:56Z

src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/ExpressionLambdaRewriter.cs

@@ -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:
+                    // TODO REVIEW


// PROTOTYPE(default): REVIEW

gafter · 2017-02-23T20:28:04Z

src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs

+            var nodes = tree.GetCompilationUnitRoot().DescendantNodes();
+
+            var def = nodes.OfType<DefaultLiteralSyntax>().Single();
+            Assert.Equal("System.Int32", model.GetTypeInfo(def).Type.ToTestDisplayString());


Is this similar to how we handle the null literal?

Not exactly. See discussion above.

When we create a null literal conversion, we create a BoundConversion which wraps the BoundLiteralExpression, but that literal node isn't changed. It will have ConstantValue.Null and no Type. The Type is stored in the conversion node, but not in the literal node.

For default, when we create the conversion, we update the operand to have a ConstantValue (if one exists for that type) and a Type.

Here's what we get for null:

public void Null() { string source = @" class C { static void Main() { C x = null; System.Console.Write(x); } } "; var comp = CreateCompilationWithMscorlib(source, parseOptions: TestOptions.ExperimentalParseOptions, options: TestOptions.DebugExe); comp.VerifyDiagnostics(); var tree = comp.SyntaxTrees.First(); var model = comp.GetSemanticModel(tree); var nodes = tree.GetCompilationUnitRoot().DescendantNodes(); var def = nodes.OfType<LiteralExpressionSyntax>().Single(); Assert.Null(model.GetTypeInfo(def).Type); Assert.Equal("C", model.GetTypeInfo(def).ConvertedType.ToTestDisplayString()); Assert.Null(model.GetSymbolInfo(def).Symbol); Assert.Null(model.GetConstantValue(def).Value); }

AlekseyTs · 2017-02-23T22:56:47Z

    DefaultLiteral,

Do we still use this? #Closed

Refers to: src/Compilers/Core/Portable/ConstantValue.cs:30 in 7eb2e3f. [](commit_id = 7eb2e3f, deletion_comment = False)

AlekseyTs · 2017-02-23T23:07:20Z

src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs

+            {
+                if (conversion.Kind == ConversionKind.DefaultOrNullLiteral && source.Kind == BoundKind.DefaultLiteral)
+                {
+                    return destination.GetDefaultValue();


GetDefaultValue [](start = 39, length = 15)

Is this method new to this branch? Could you provide a link to its implementation? #Closed

This is the existing method used for making a ConstantValue for default() operator.
Code is here http://source.roslyn.io/#Microsoft.CodeAnalysis.CSharp/Symbols/TypeSymbolExtensions.cs,264ab804a03fe275

AlekseyTs · 2017-02-23T23:10:32Z

src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs

@@ -216,7 +216,7 @@ internal BoundExpression ConvertPatternExpression(TypeSymbol inputType, CSharpSy
                    case ConversionKind.Identity:
                    case ConversionKind.ImplicitReference:
                    case ConversionKind.Unboxing:
-                    case ConversionKind.NullLiteral:
+                    case ConversionKind.DefaultOrNullLiteral:


case ConversionKind.DefaultOrNullLiteral: [](start = 20, length = 41)

Do we have a test that would go through here with default? #Closed

Apparently this line affects no tests whatsoever. I'll see if I can hit it, or I'll file an issue.

This method called only from BindDeclarationPattern (and more specifically when there is a problem with the type in the declaration pattern), so I don't think this line is needed for either null or default.

AlekseyTs · 2017-02-23T23:16:17Z

src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs

@@ -2991,7 +2991,7 @@ private TypeSymbol StackMergeType(BoundExpression expr)
                    var conversionKind = conversion.ConversionKind;
                    if (conversionKind.IsImplicitConversion() &&
                        conversionKind != ConversionKind.MethodGroup &&
-                        conversionKind != ConversionKind.NullLiteral)
+                        conversionKind != ConversionKind.DefaultOrNullLiteral)


conversionKind != ConversionKind.DefaultOrNullLiteral) [](start = 24, length = 54)

Do we have a test that covers this path for default? #Closed

I didn't manage to reach this with null or default either. The logic above in StackMergeType exits early except for array, interface or delegate types. But the local rewriter strips the bound conversion on an array, interface and delegate types (because they have a constant value).

AlekseyTs · 2017-02-23T23:17:48Z

src/Compilers/CSharp/Portable/Lowering/LambdaRewriter/ExpressionLambdaRewriter.cs

@@ -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.ConversionKind != ConversionKind.DefaultOrNullLiteral && [](start = 20, length = 67)

Do we have a test that covers this code path with default? #Resolved

AlekseyTs · 2017-02-23T23:20:48Z

src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs

+            Assert.Equal("System.Int32", model.GetTypeInfo(def).Type.ToTestDisplayString());
+            Assert.Equal("System.Int32", model.GetTypeInfo(def).ConvertedType.ToTestDisplayString());
+            Assert.Null(model.GetSymbolInfo(def).Symbol);
+            Assert.Equal("0", model.GetConstantValue(def).Value.ToString());


Assert.Equal("0", model.GetConstantValue(def).Value.ToString()); [](start = 12, length = 64)

It would be good to test conversion as well. #Closed

AlekseyTs · 2017-02-23T23:21:24Z

src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs

+            Assert.Equal("S", model.GetTypeInfo(def).Type.ToTestDisplayString());
+            Assert.Equal("S", model.GetTypeInfo(def).ConvertedType.ToTestDisplayString());
+            Assert.Null(model.GetSymbolInfo(def).Symbol);
+            Assert.False(model.GetConstantValue(def).HasValue);


Assert.False(model.GetConstantValue(def).HasValue); [](start = 3, length = 60)

It would be good to test conversion as well. #Closed

AlekseyTs · 2017-02-23T23:21:41Z

src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs

+            Assert.Equal("T", model.GetTypeInfo(def).Type.ToTestDisplayString());
+            Assert.Equal("T", model.GetTypeInfo(def).ConvertedType.ToTestDisplayString());
+            Assert.Null(model.GetSymbolInfo(def).Symbol);
+            Assert.False(model.GetConstantValue(def).HasValue);


Assert.False(model.GetConstantValue(def).HasValue); [](start = 12, length = 51)

It would be good to test conversion as well. #Closed

AlekseyTs · 2017-02-23T23:23:35Z

src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs

@@ -787,7 +859,7 @@ static void Main()
            var tree = comp.SyntaxTrees.First();
            var model = comp.GetSemanticModel(tree);
            var def = tree.GetCompilationUnitRoot().DescendantNodes().OfType<DefaultLiteralSyntax>().Single();
-            Assert.Null(model.GetTypeInfo(def).Type);
+            Assert.Equal("System.Int32?", model.GetTypeInfo(def).Type.ToTestDisplayString());


Assert.Equal("System.Int32?", model.GetTypeInfo(def).Type.ToTestDisplayString()); [](start = 12, length = 81)

It would be good to test conversion and constant value as well. #Closed

It doesn't look like constant value is tested.

In reply to: 102845219 [](ancestors = 102845219)

AlekseyTs · 2017-02-24T19:28:58Z

src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs

@@ -988,7 +1064,7 @@ static void Main()
            var nodes = tree.GetCompilationUnitRoot().DescendantNodes();

            var def = nodes.OfType<DefaultLiteralSyntax>().Single();
-            Assert.Null(model.GetTypeInfo(def).Type);
+            Assert.Equal("System.Int16", model.GetTypeInfo(def).Type.ToTestDisplayString());
            Assert.Null(model.GetSymbolInfo(def).Symbol);
            Assert.Null(model.GetDeclaredSymbol(def));


Assert.Null(model.GetDeclaredSymbol(def)); [](start = 12, length = 42)

It would be good to test conversion and constant value for this scenario because it is using explicit cast. #Closed

AlekseyTs · 2017-02-24T19:29:33Z

        Assert.Equal("System.Int32", conversionTypeInfo.ConvertedType.ToTestDisplayString());

It would be good to test constant value as well. #Closed

Refers to: src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs:1074 in d980c02. [](commit_id = d980c02, deletion_comment = False)

…lt conversion

jcouv · 2017-02-24T19:57:20Z

src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs

+            Assert.Equal("System.Int16", model.GetTypeInfo(def).ConvertedType.ToTestDisplayString());
+            Assert.Null(model.GetSymbolInfo(def).Symbol);
+            Assert.Equal((short)0, model.GetConstantValue(def).Value);
+            Assert.True(model.GetConversion(def).IsIdentity);


@AlekseyTs This one surprised me. This behavior comes from GetTypeInfoForNode when type and converted type match.

AlekseyTs · 2017-02-27T18:19:24Z

src/Compilers/CSharp/Test/Semantic/Semantics/TargetTypedDefaultTests.cs

+            Assert.Equal("System.Int16", model.GetTypeInfo(def).ConvertedType.ToTestDisplayString());
+            Assert.Null(model.GetSymbolInfo(def).Symbol);
+            Assert.Equal((short)0, model.GetConstantValue(def).Value);
+            Assert.True(model.GetConversion(def).IsIdentity);


Assert.True(model.GetConversion(def).IsIdentity); [](start = 12, length = 49)

Is this consistent with behavior when explicit cast is applied to a null literal? #Closed

Yes, in terms of conversions, the behavior is similar with null (there is an identity conversion on the literal).
The main difference with null as we already discussed is that we give default a type.

[Fact] public void ExplicitCastOnNull() { string source = @" class Base { } class Derived : Base { static void Main() { Base x = (Derived)null; System.Console.Write(x); } } "; var comp = CreateCompilationWithMscorlib(source, parseOptions: TestOptions.ExperimentalParseOptions, options: TestOptions.DebugExe); comp.VerifyDiagnostics(); var tree = comp.SyntaxTrees.First(); var model = comp.GetSemanticModel(tree); var nodes = tree.GetCompilationUnitRoot().DescendantNodes(); var def = nodes.OfType<LiteralExpressionSyntax>().Single(); Assert.Equal(null, model.GetTypeInfo(def).Type); Assert.Null(model.GetSymbolInfo(def).Symbol); Assert.Null(model.GetDeclaredSymbol(def)); Assert.Equal(null, model.GetTypeInfo(def).ConvertedType); Assert.Null(model.GetSymbolInfo(def).Symbol); Assert.True(model.GetConstantValue(def).HasValue); Assert.Equal(null, model.GetConstantValue(def).Value); Assert.True(model.GetConversion(def).IsIdentity); var conversionSyntax = nodes.OfType<CastExpressionSyntax>().Single(); var conversionTypeInfo = model.GetTypeInfo(conversionSyntax); Assert.Equal("Derived", conversionTypeInfo.Type.ToTestDisplayString()); Assert.Equal("Base", conversionTypeInfo.ConvertedType.ToTestDisplayString()); Assert.True(model.GetConstantValue(conversionSyntax).HasValue); Assert.Equal(null, model.GetConstantValue(conversionSyntax).Value); Conversion conversion = model.GetConversion(conversionSyntax); Assert.False(conversion.IsNumeric); Assert.True(conversion.IsImplicit); }

Thanks for confirming this.

AlekseyTs

LGTM

jcouv · 2017-02-28T19:01:24Z

@gafter Could you take another look and approve the change if looks good?

jcouv · 2017-03-01T17:22:34Z

@gafter Still good to go?

gafter

LGTM

…net#17205) Conflicts: src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionKind.cs src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionKindExtensions.cs src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs

Add test for GetConstantValue

27f1b2a

jcouv added the Area-Compilers label Feb 17, 2017

jcouv added this to the 2.1 milestone Feb 17, 2017

jcouv self-assigned this Feb 17, 2017

dnfclas added the cla-already-signed label Feb 17, 2017

AlekseyTs reviewed Feb 21, 2017

View reviewed changes

Use NullLiteral conversion

ff13f93

jcouv force-pushed the default-constvalue branch from a0d7113 to ff13f93 Compare February 23, 2017 07:20

Rename null literal conversion to 'default or null'

a1f39a5

jcouv force-pushed the default-constvalue branch from 9856c50 to 9ba6ed9 Compare February 23, 2017 18:40

Tweaks

6f6d28d

jcouv force-pushed the default-constvalue branch from 9ba6ed9 to 6f6d28d Compare February 23, 2017 18:47

jcouv added 2 commits February 23, 2017 11:23

Fix usages of now obsolete method

087f541

Let's not make the API obsolete

7eb2e3f

gafter reviewed Feb 23, 2017

View reviewed changes

Fix comment

634d389

AlekseyTs reviewed Feb 23, 2017

View reviewed changes

PR feedback from Aleksey

d980c02

AlekseyTs reviewed Feb 24, 2017

View reviewed changes

Updating code paths that can't be reached with neither null nor defau…

1b391dc

…lt conversion

jcouv commented Feb 24, 2017

View reviewed changes

Add some checks

7bbaf29

jcouv force-pushed the default-constvalue branch from 2ec1199 to 7bbaf29 Compare February 24, 2017 20:38

AlekseyTs reviewed Feb 27, 2017

View reviewed changes

AlekseyTs approved these changes Feb 28, 2017

View reviewed changes

Add test for default == expr

341dcf3

gafter approved these changes Mar 3, 2017

View reviewed changes

jcouv merged commit 533aa4b into dotnet:features/default Mar 3, 2017

jcouv deleted the default-constvalue branch March 3, 2017 00:20

Neme12 mentioned this pull request Apr 14, 2018

QuickInfo shows type for default literal, but not null literal #26027

Closed

Neme12 mentioned this pull request Nov 7, 2018

GetTypeInfo().Type should return null for default literal #31029

Closed

		@@ -980,6 +980,11 @@ internal override Optional<object> GetConstantValueWorker(CSharpSyntaxNode node,

		if (boundExpr == null) return default(Optional<object>);

		if (boundExpr.Kind == BoundKind.DefaultLiteral)

Check GetConstantValue and GetConversion on target-typed default #17205

Check GetConstantValue and GetConversion on target-typed default #17205

Conversation

jcouv commented Feb 17, 2017 • edited Loading

jcouv commented Feb 21, 2017

AlekseyTs Feb 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Feb 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Feb 21, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Feb 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Feb 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Feb 22, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs commented Feb 21, 2017 • edited Loading

gafter commented Feb 22, 2017

AlekseyTs commented Feb 22, 2017 • edited Loading

gafter commented Feb 22, 2017

jcouv commented Feb 23, 2017

gafter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs commented Feb 23, 2017 • edited Loading

AlekseyTs Feb 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Feb 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Feb 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Feb 23, 2017 • edited by jcouv Loading

Choose a reason for hiding this comment

AlekseyTs Feb 23, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Feb 23, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Feb 23, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Feb 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Feb 24, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs commented Feb 24, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Feb 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs left a comment

Choose a reason for hiding this comment

jcouv commented Feb 28, 2017

jcouv commented Mar 1, 2017

gafter left a comment

Choose a reason for hiding this comment

jcouv commented Feb 17, 2017 •

edited

Loading

AlekseyTs Feb 21, 2017 •

edited

Loading

AlekseyTs Feb 22, 2017 •

edited

Loading

AlekseyTs Feb 21, 2017 •

edited

Loading

AlekseyTs Feb 21, 2017 •

edited

Loading

AlekseyTs Feb 21, 2017 •

edited

Loading

AlekseyTs Feb 22, 2017 •

edited

Loading

AlekseyTs commented Feb 21, 2017 •

edited

Loading

AlekseyTs commented Feb 22, 2017 •

edited

Loading

AlekseyTs commented Feb 23, 2017 •

edited

Loading

AlekseyTs Feb 23, 2017 •

edited

Loading

AlekseyTs Feb 23, 2017 •

edited

Loading

AlekseyTs Feb 23, 2017 •

edited

Loading

AlekseyTs Feb 23, 2017 •

edited by jcouv

Loading

AlekseyTs Feb 23, 2017 •

edited

Loading

AlekseyTs Feb 23, 2017 •

edited

Loading

AlekseyTs Feb 23, 2017 •

edited

Loading

AlekseyTs Feb 23, 2017 •

edited

Loading

AlekseyTs Feb 24, 2017 •

edited

Loading

AlekseyTs commented Feb 24, 2017 •

edited

Loading

AlekseyTs Feb 27, 2017 •

edited

Loading