Skip to content

Commit

Permalink
Add handling for open class type vs value type constant pattern
Browse files Browse the repository at this point in the history
  • Loading branch information
gafter committed Apr 11, 2018
1 parent 4ce5edd commit c6b91e1
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 30 deletions.
17 changes: 17 additions & 0 deletions docs/compilers/CSharp/Compiler Breaking Changes - Milestone 16.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,20 @@ Each entry should include a short description of the break, followed by either a
switch (a, b)
```
Due to this the `OpenParenToken` and `CloseParenToken` fields of a `SwitchStatementSyntax` node may now sometimes be empty.

4. https://github.com/dotnet/roslyn/issues/26098 In C# 8, we give a warning when an is-type expression is always `false` because the input type is an open class type and the type it is tested against is a value type:
``` c#
class C<T> { }
void M<T>(C<T> x)
{
if (x is int) { } // warning: the given expression is never of the provided ('int') type.
}
```
previously, we gave the warning only in the reverse case
``` c#
void M<T>(int x)
{
if (x is C<T>) { } // warning: the given expression is never of the provided ('C<T>') type.
}
```

17 changes: 12 additions & 5 deletions src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2890,12 +2890,18 @@ private BoundExpression BindIsOperator(BinaryExpressionSyntax node, DiagnosticBa
// nevertheless, X could be constructed as an enumerated type.
// However, we can sometimes know that the result will be false.
//
// Scenario 2: Constrained type parameter compared to reference type.
// Scenario 2a: Constrained type parameter compared to reference type.
//
// bool M2<X>(X x) where X : struct { return x is string; }
// bool M2a<X>(X x) where X : struct { return x is string; }
//
// We know that X, constrained to struct, will never be string.
//
// Scenario 2b: Reference type compared to constrained type parameter.
//
// bool M2b<X>(string x) where X : struct { return x is X; }
//
// We know that string will never be X, constrained to struct.
//
// Scenario 3: Value type compared to type parameter.
//
// bool M3<T>(int x) { return x is T; }
Expand Down Expand Up @@ -2936,10 +2942,11 @@ private BoundExpression BindIsOperator(BinaryExpressionSyntax node, DiagnosticBa
}

// * Otherwise, at least one of them is of an open type. If the operand is of value type
// and the target is a class type other than System.Enum, then we are in scenario 2,
// not scenario 1, and can correctly deduce that the result is false.
// and the target is a class type other than System.Enum, or vice versa, then we are
// in scenario 2, not scenario 1, and can correctly deduce that the result is false.

if (operandType.IsValueType && targetType.IsClassType() && targetType.SpecialType != SpecialType.System_Enum)
if (operandType.IsValueType && targetType.IsClassType() && targetType.SpecialType != SpecialType.System_Enum ||
targetType.IsValueType && operandType.IsClassType() && operandType.SpecialType != SpecialType.System_Enum)
{
return ConstantValue.False;
}
Expand Down
20 changes: 17 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,25 @@ internal BoundExpression ConvertPatternExpression(TypeSymbol inputType, CSharpSy

// If we are pattern-matching against an open type, we do not convert the constant to the type of the input.
// This permits us to match a value of type `IComparable<T>` with a pattern of type `int`.
if (inputType.ContainsTypeParameter() &&
// But do not permit matching null against a struct type.
(expression.ConstantValue != ConstantValue.Null || !inputType.IsNonNullableValueType()))
bool inputContainsTypeParameter = inputType.ContainsTypeParameter();
HashSet<DiagnosticInfo> useSiteDiagnostics = null;
if (inputContainsTypeParameter)
{
convertedExpression = expression;
if (expression.ConstantValue == ConstantValue.Null)
{
if (inputType.IsNonNullableValueType())
{
// We do not permit matching null against a struct type.
diagnostics.Add(ErrorCode.ERR_ValueCantBeNull, expression.Syntax.Location, inputType);
}
}
else if (ExpressionOfTypeMatchesPatternType(Conversions, inputType, expression.Type, ref useSiteDiagnostics, out _, operandConstantValue: null) == false)
{
diagnostics.Add(ErrorCode.ERR_PatternWrongType, expression.Syntax.Location, inputType, expression.Display);
}

diagnostics.Add(node, useSiteDiagnostics);
}
else
{
Expand Down
22 changes: 0 additions & 22 deletions src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1543,27 +1543,5 @@ .locals init (int V_0)
IL_001f: ret
}");
}

[Fact]
[WorkItem(24550, "https://github.com/dotnet/roslyn/issues/24550")]
[WorkItem(1284, "https://github.com/dotnet/csharplang/issues/1284")]
public void ConstantPatternVsUnconstrainedTypeParameter03()
{
var source =
@"class C<T>
{
internal struct S { }
static bool Test(S s)
{
return s is null;
}
}";
var compilation = CreateCompilation(source, options: TestOptions.ReleaseDll);
compilation.VerifyDiagnostics(
// (6,21): error CS0037: Cannot convert null to 'C<T>.S' because it is a non-nullable value type
// return s is null;
Diagnostic(ErrorCode.ERR_ValueCantBeNull, "null").WithArguments("C<T>.S").WithLocation(6, 21)
);
}
}
}
20 changes: 20 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/OperatorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5285,6 +5285,26 @@ public enum E { }
Diagnostic(ErrorCode.WRN_IsAlwaysFalse, "e2 is Outer<long>.E").WithArguments("Outer<long>.E").WithLocation(32, 13));
}

[Fact]
public void TestIsOperatorWithGenericClassAndValueType()
{
var source = @"
class Program
{
static bool Goo<T>(C<T> c)
{
return c is int; // always false
}
}
class C<T> { }
";
CreateCompilation(source).VerifyDiagnostics(
// (6,16): warning CS0184: The given expression is never of the provided ('int') type
// return c is int; // always false
Diagnostic(ErrorCode.WRN_IsAlwaysFalse, "c is int").WithArguments("int").WithLocation(6, 16)
);
}

[WorkItem(844635, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/844635")]
[Fact()]
public void TestIsOperatorWithTypesThatCannotUnify()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1560,6 +1560,49 @@ static void Main()
);
}

[Fact]
[WorkItem(24550, "https://github.com/dotnet/roslyn/issues/24550")]
[WorkItem(1284, "https://github.com/dotnet/csharplang/issues/1284")]
public void ConstantPatternVsUnconstrainedTypeParameter03()
{
var source =
@"class C<T>
{
internal struct S { }
static bool Test(S s)
{
return s is null;
}
}";
var compilation = CreateCompilation(source, options: TestOptions.ReleaseDll);
compilation.VerifyDiagnostics(
// (6,21): error CS0037: Cannot convert null to 'C<T>.S' because it is a non-nullable value type
// return s is null;
Diagnostic(ErrorCode.ERR_ValueCantBeNull, "null").WithArguments("C<T>.S").WithLocation(6, 21)
);
}

[Fact]
[WorkItem(24550, "https://github.com/dotnet/roslyn/issues/24550")]
[WorkItem(1284, "https://github.com/dotnet/csharplang/issues/1284")]
public void ConstantPatternVsUnconstrainedTypeParameter04()
{
var source =
@"class C<T>
{
static bool Test(C<T> x)
{
return x is 1;
}
}";
var compilation = CreateCompilation(source, options: TestOptions.ReleaseDll);
compilation.VerifyDiagnostics(
// (5,21): error CS8121: An expression of type 'C<T>' cannot be handled by a pattern of type 'int'.
// return x is 1;
Diagnostic(ErrorCode.ERR_PatternWrongType, "1").WithArguments("C<T>", "int").WithLocation(5, 21)
);
}

// PROTOTYPE(patterns2): Need to have tests that exercise:
// PROTOTYPE(patterns2): Building the decision tree for the var-pattern
// PROTOTYPE(patterns2): Definite assignment for the var-pattern
Expand Down

0 comments on commit c6b91e1

Please sign in to comment.