C# IOperation nodes representing values of a nullable type sometimes have constant values

[AlekseyTs]

Example 1:

ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: System.Int32?) (Syntax: 'input = null')
  Left: 
    IParameterReferenceOperation: input (OperationKind.ParameterReference, Type: System.Int32?) (Syntax: 'input')
  Right: 
    IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: System.Int32?, Constant: null, IsImplicit) (Syntax: 'null')
      Conversion: CommonConversion (Exists: True, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null)
      Operand: 
        ILiteralOperation (OperationKind.Literal, Type: null, Constant: null) (Syntax: 'null')

Example 2:

IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: System.Int32?, Constant: null) (Syntax: '(int?)null')
  Conversion: CommonConversion (Exists: True, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null)
  Operand: 
    ILiteralOperation (OperationKind.Literal, Type: null, Constant: null) (Syntax: 'null')

Example 3:

IIsPatternOperation (OperationKind.IsPattern, Type: System.Boolean) (Syntax: 'input is null')
  Expression: 
    IParameterReferenceOperation: input (OperationKind.ParameterReference, Type: System.Int32?) (Syntax: 'input')
  Pattern: 
    IConstantPatternOperation (OperationKind.ConstantPattern, Type: null) (Syntax: 'null')
      Value: 
        IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: System.Int32?, Constant: null, IsImplicit) (Syntax: 'null')
          Conversion: CommonConversion (Exists: True, IsIdentity: False, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null)
          Operand: 
            ILiteralOperation (OperationKind.Literal, Type: null, Constant: null) (Syntax: 'null')

Note that conversion nodes like the one below have constant values, even though the type of the value is int?, which is not valid for constant expressions.

IConversionOperation (TryCast: False, Unchecked) (OperationKind.Conversion, Type: System.Int32?, Constant: null,

Expected: No constant values for such nodes.

[AlekseyTs]

CC @dotnet/analyzer-ioperation

[gafter]

This may require some LDM work, as null is accepted in places that require a constant value (of a nullable value type), e.g. in a switch statement.

[gafter]

Looking at the C# language specification, a null expression can have a constant value. It says at https://github.com/dotnet/csharplang/blob/master/spec/expressions.md#constant-expressions :

A constant expression must be the null literal or a value with one of the following types: sbyte, byte, short, ushort, int, uint, long, ulong, char, float, double, decimal, bool, object, string, or any enumeration type.

[AlekseyTs]

The spec explicitly names the types that can have constant values and nullable type is not in that list

[gafter]

The spec says that the null literal is a constant, or anything of one of those types. The null literal can be a constant no matter its type.

[AlekseyTs]

A conversion is not a null literal. One cannot write this code today:

const object o1 = (int?)null; // error CS0133: The expression being assigned to 'o1' must be constant
const int? o2 = null; // error CS0283: The type 'int?' cannot be declared const

[gafter]

Agree: an explicit cast in source to int? should not be a constant. Thus the error on the first.

The second runs afoul of a restriction on the declared type of a constant declaration. The problem is not that it’s initializer is not a constant (it is) but that a declared constant may not be that type. See https://github.com/dotnet/csharplang/blob/master/spec/classes.md#constants

[AlekseyTs]

The second is not a constant because it requires an implicit conversion and spec clearly describes what conversions are considered to be a constant expression, this conversion is not on the list. The issue here is specifically about conversions, it doesn't matter whether they are explicit or implicit.

[gafter]

Can you please point to the part of the spec that says that a null literal converted to the type int? is not a constant? I know the cast syntax prevents it from being a constant, but I did not find anything about the conversion itself preventing it from being a constant. The fact that case null: is accepted for an int? switch suggests the converted null is indeed constant.

[AlekseyTs]

The fact that case null: is accepted for an int? switch suggests the converted null is indeed constant.

This can suggest many different things, starting with "this is a bug in compiler" and ending with "this is something special about is pattern". I am leaning towards the pattern peculiarity, perhaps unspecified. And, if unspecified, that happened exactly due to the inaccuracy in the compiler's implementation around constant folding, things just worked as we wanted them and no-one realized that specification for is is incomplete.

Can you please point to the part of the spec that says that a null literal converted to the type int? is not a constant?

From my perspective only nodes representing constant expressions can have constant values associated with them. Here is a quote from the spec that defines what can be a constant expression (https://github.com/dotnet/csharplang/blob/master/spec/expressions.md):

Constant expressions

A constant_expression is an expression that can be fully evaluated at compile-time.

constant_expression
    : expression
    ;

A constant expression must be the null literal or a value with one of the following types: sbyte, byte, short, ushort, int, uint, long, ulong, char, float, double, decimal, bool, object, string, or any enumeration type. Only the following constructs are permitted in constant expressions:

• Literals (including the null literal).
• References to const members of class and struct types.
• References to members of enumeration types.
• References to const parameters or local variables
• Parenthesized sub-expressions, which are themselves constant expressions.
• Cast expressions, provided the target type is one of the types listed above.
• checked and unchecked expressions
• Default value expressions
• Nameof expressions
• The predefined +, -, !, and ~ unary operators.
• The predefined +, -, *, /, %, <<, >>, &, |, ^, &&, ||, ==, !=, <, >, <=, and >= binary operators, provided each operand is of a type listed above.
• The ?: conditional operator.

The following conversions are permitted in constant expressions:

• Identity conversions
• Numeric conversions
• Enumeration conversions
• Constant expression conversions
• Implicit and explicit reference conversions, provided that the source of the conversions is a constant expression that evaluates to the null value.

Other conversions including boxing, unboxing and implicit reference conversions of non-null values are not permitted in constant expressions. For example:

class C {
    const object i = 5;         // error: boxing conversion not permitted
    const object str = "hello"; // error: implicit reference conversion
}

end quote

The conversion nodes in question are not null literals or values with one of the following types: sbyte, byte, short, ushort, int, uint, long, ulong, char, float, double, decimal, bool, object, string, or any enumeration type. They are conversions representing values of nullable types. This on itself is enough to conclude that they are not constant expressions.

However, in addition they do not represent any of the following conversions:

• Identity conversions
• Numeric conversions
• Enumeration conversions
• Constant expression conversions
• Implicit and explicit reference conversions, provided that the source of the conversions is a constant expression that evaluates to the null value.

And the spec explicitly says that other conversions are not permitted in constant expressions.

[gafter]

The conversion nodes in question are not null literals or values with one of the following types:

On the contrary, they are part of the representation of a null literal. The spec applies to C# programs, not bound nodes.

[gafter]

The fact that case null: works for an int? switch is longstanding intentional behavior since nullable types were added to the language. If the spec does not permit that, it is a bug in the spec.

[gafter]

See https://github.com/dotnet/csharplang/issues/1282 for the ECMA bug and proposed fix. That is a copy of an open issue in ECMA for your reference.

[AlekseyTs]

The fact that case null: works for an int? switch is longstanding intentional behavior since nullable types were added to the language. If the spec does not permit that, it is a bug in the spec.

I am not disputing that. However, I do not believe the bug is in the "Constant expressions" section.

The IOperation nodes should reflect the language. If, according to the language, there is no conversion, then there should be no conversion node in the tree. Otherwise, they should not have constant values.

[gafter]

I do not believe the bug is in the "Constant expressions" section

Whether you believe it or not, the only proposed fix to bring the spec in alignment with the longstanding compiler/language behavior involves a change to that section.

[AlekseyTs]

@gafter

Whether you believe it or not, the only proposed fix to bring the spec in alignment with the longstanding compiler/language behavior involves a change to that section.

Please elaborate what is this statement supposed to mean.

[gafter]

What I mean is that the proposed fix in ECMA (which you can view a copy of in https://github.com/dotnet/csharplang/issues/1282) involves a fix to that section. The proposed fix is

1. Modify the quoted section [for the switch statement] to say something like "A compile-time error occurs if two or more case labels in the same switch statement specify the same constant value after the conversion."
2. In the section Constant expressions (ECMA 12.20), in the bullet list following the introductory sentence "The following conversions are permitted in constant expressions:", add a new bullet reading "Null literal conversions".

A PR that shows the proposed fixes in context is at https://github.com/dotnet/csharplang/pull/1283

[AlekseyTs]

What I mean is that the proposed fix in ECMA (which you can view a copy of in dotnet/csharplang#1282) involves a fix to that section. The proposed fix is ...

And I disagree with the proposal. A simple fact of existence of a proposal doesn't mean that the proposal outlines the right way to deal with the problem. Even if it is the only proposal at the given point in time.