diff --git a/docs/compilers/CSharp/Compiler Breaking Changes - post VS2017.md b/docs/compilers/CSharp/Compiler Breaking Changes - post VS2017.md new file mode 100644 index 0000000000000000000000000000000000000000..cba5c7d18cbdb099959975948e22bc1e2a8679ba --- /dev/null +++ b/docs/compilers/CSharp/Compiler Breaking Changes - post VS2017.md @@ -0,0 +1,5 @@ +Changes since VS2017 (C# 7) +=========================== + +- https://github.com/dotnet/roslyn/issues/17089 +In C# 7, the compiler accepted a pattern of the form `dynamic identifier`, e.g. `if (e is dynamic x)`. This was accepted only if the expression `e` was statically of type `dynamic`. The compiler now rejects the use of the type `dynamic` for a pattern variable declaration, as no object's runtime type is `dynamic`. diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs index 2aa9f187e8e192d97d613e3886b3c4cde1e367ec..7864640018f4cef49d66615e4fb66f3a3e31bcb2 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs @@ -128,6 +128,9 @@ internal BoundExpression ConvertPatternExpression(TypeSymbol inputType, CSharpSy return convertedExpression; } + /// + /// Check that the pattern type is valid for the operand. Return true if an error was reported. + /// private bool CheckValidPatternType( CSharpSyntaxNode typeSyntax, BoundExpression operand, @@ -139,6 +142,11 @@ internal BoundExpression ConvertPatternExpression(TypeSymbol inputType, CSharpSy { Debug.Assert((object)operandType != null); Debug.Assert((object)patternType != null); + + // Because we do not support recursive patterns, we always have an operand + Debug.Assert((object)operand != null); + // Once we support recursive patterns that will be relaxed. + if (operandType.IsErrorType() || patternType.IsErrorType()) { return false; @@ -156,6 +164,12 @@ internal BoundExpression ConvertPatternExpression(TypeSymbol inputType, CSharpSy } else if (!isVar) { + if (patternType.IsDynamic()) + { + Error(diagnostics, ErrorCode.ERR_PatternDynamicType, typeSyntax); + return true; + } + HashSet useSiteDiagnostics = null; Conversion conversion = operand != null @@ -164,6 +178,10 @@ internal BoundExpression ConvertPatternExpression(TypeSymbol inputType, CSharpSy diagnostics.Add(typeSyntax, useSiteDiagnostics); switch (conversion.Kind) { + case ConversionKind.ExplicitDynamic: + case ConversionKind.ImplicitDynamic: + // Since the input was `dynamic`, which is equivalent to `object`, there must also + // exist some unboxing, identity, or reference conversion as well, making the conversion legal. case ConversionKind.Boxing: case ConversionKind.ExplicitNullable: case ConversionKind.ExplicitReference: diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs index 1d071d30e50f34ed9d1e7a58c4d21599ddea0c97..6eda21576a669734ceed84e5cd169e7b9f98ac97 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/Conversions/ConversionsBase.cs @@ -528,11 +528,6 @@ private Conversion ClassifyImplicitBuiltInConversionSlow(TypeSymbol source, Type return conversion; } - //if (HasImplicitDynamicConversion(source, destination)) - //{ - // return Conversion.ImplicitDynamic; - //} - return Conversion.NoConversion; } @@ -1861,14 +1856,13 @@ private static bool HasImplicitDynamicConversionFromExpression(TypeSymbol expres // An implicit dynamic conversion exists from an expression of type dynamic to any type T. Debug.Assert((object)destination != null); - return (object)expressionType != null && expressionType.Kind == SymbolKind.DynamicType && !destination.IsPointerType(); + return expressionType?.Kind == SymbolKind.DynamicType && !destination.IsPointerType(); } private static bool HasExplicitDynamicConversion(TypeSymbol source, TypeSymbol destination) { - // SPEC: An explicit dynamic conversion exists from an expression of type dynamic to any type T. - // ISSUE: Note that this is exactly the same as an implicit dynamic conversion. Conversion of - // ISSUE: dynamic to int is both an explicit dynamic conversion and an implicit dynamic conversion. + // SPEC: An explicit dynamic conversion exists from an expression of [sic] type dynamic to any type T. + // ISSUE: The "an expression of" part of the spec is probably an error; see https://github.com/dotnet/csharplang/issues/132 Debug.Assert((object)source != null); Debug.Assert((object)destination != null); diff --git a/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs b/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs index 92a0774c3d8fa6933fc8e1059d18ef6058eff565..05f0ae7abe233d6feb4ba18ed219aee97cf3d7ec 100644 --- a/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs +++ b/src/Compilers/CSharp/Portable/CSharpResources.Designer.cs @@ -7495,6 +7495,15 @@ internal class CSharpResources { } } + /// + /// Looks up a localized string similar to It is not legal to use the type 'dynamic' in a pattern.. + /// + internal static string ERR_PatternDynamicType { + get { + return ResourceManager.GetString("ERR_PatternDynamicType", resourceCulture); + } + } + /// /// Looks up a localized string similar to The switch case has already been handled by a previous case.. /// diff --git a/src/Compilers/CSharp/Portable/CSharpResources.resx b/src/Compilers/CSharp/Portable/CSharpResources.resx index 4a782f374669aba571ae724d5e87c8161e5a023b..f5ced08a92dff018f8f6f7e3e32a257cd17aa004 100644 --- a/src/Compilers/CSharp/Portable/CSharpResources.resx +++ b/src/Compilers/CSharp/Portable/CSharpResources.resx @@ -5017,4 +5017,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ Module '{0}' in assembly '{1}' is forwarding the type '{2}' to multiple assemblies: '{3}' and '{4}'. + + It is not legal to use the type 'dynamic' in a pattern. + \ No newline at end of file diff --git a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs index 27c870578cf9c9a6b9c5d8861167c10edaae5670..3036a614c69cd5e59f7e18d562f7ba21d047b58d 100644 --- a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs +++ b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs @@ -1457,6 +1457,7 @@ internal enum ErrorCode ERR_AttributesInLocalFuncDecl = 8205, ERR_TypeForwardedToMultipleAssemblies = 8206, ERR_ExpressionTreeContainsDiscard = 8207, + ERR_PatternDynamicType = 8208, #endregion more stragglers for C# 7 ERR_Merge_conflict_marker_encountered = 8300, diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests.cs index b6b1cc3de5f4eedc46c8b3703b431927141b8320..f73c422c4800c9c37d73901441c0d1c04b856817 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests.cs @@ -5619,5 +5619,60 @@ public static void Main() var expectedOutput = @"eval"; var comp = CompileAndVerify(compilation, expectedOutput: expectedOutput); } + + [Fact] + [WorkItem(17089, "https://github.com/dotnet/roslyn/issues/17089")] + public void Dynamic_01() + { + var source = +@"using System; +public class X +{ + static void M1(dynamic d) + { + if (d is 1) + { + Console.Write('r'); + } + else if (d is int i) + { + Console.Write('o'); + } + else if (d is var z) + { + long l = z; + Console.Write('s'); + } + } + static void M2(dynamic d) + { + switch (d) + { + case 1: + Console.Write('l'); + break; + case int i: + Console.Write('y'); + break; + case var z: + long l = z; + Console.Write('n'); + break; + } + } + public static void Main(string[] args) + { + M1(1); + M1(2); + M1(3L); + M2(1); + M2(2); + M2(3L); + } +} +"; + var compilation = CreateCompilationWithMscorlib45(source, references: new MetadataReference[] { CSharpRef, SystemCoreRef }, options: TestOptions.ReleaseExe); + var comp = CompileAndVerify(compilation, expectedOutput: "roslyn"); + } } } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternSwitchTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternSwitchTests.cs index 0a3d26424c57d77c01062b8284467f71158cbc1d..6b042ff68548ca5cd07717964e689f076080ab61 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/PatternSwitchTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/PatternSwitchTests.cs @@ -2756,7 +2756,7 @@ static void Main() var comp = CompileAndVerify(compilation, expectedOutput: @"pass"); } - [Fact(Skip = "https://github.com/dotnet/roslyn/issues/17089")] + [Fact] [WorkItem(17089, "https://github.com/dotnet/roslyn/issues/17089")] public void Dynamic_01() { @@ -2769,12 +2769,79 @@ static void Main() dynamic d = 1; switch (d) { - case dynamic x: // should be an error + case dynamic x: // error 1 break; } + if (d is dynamic y) {} // error 2 + if (d is var z) // ok + { + long l = z; + string s = z; + } + } +} +"; + var compilation = CreateCompilationWithMscorlib45(source, options: TestOptions.ReleaseExe); + compilation.VerifyDiagnostics( + // (9,18): error CS8207: It is not legal to use the type 'dynamic' in a pattern. + // case dynamic x: // error 1 + Diagnostic(ErrorCode.ERR_PatternDynamicType, "dynamic"), + // (12,18): error CS8207: It is not legal to use the type 'dynamic' in a pattern. + // if (d is dynamic y) {} // error 2 + Diagnostic(ErrorCode.ERR_PatternDynamicType, "dynamic").WithLocation(12, 18) + ); + } + + [Fact] + [WorkItem(17089, "https://github.com/dotnet/roslyn/issues/17089")] + public void Dynamic_02() + { + var source = +@" +class Program +{ + static void Main() + { + dynamic d = 1; switch (d) { - case int i: // should not be an error + case object x: + case var y: // OK, catches null + case var z: // error: subsumed + break; + } + switch (d) + { + case object x: + case null: + case var y: // error: subsumed + break; + } + switch (d) + { + case object x: + case 1: // error: subsumed + break; + } + switch (d) + { + case object x: + case int y: // error: subsumed + break; + } + switch (d) + { + case object x: + case (dynamic)null: + case (string)null: // error: subsumed + break; + } + switch (d) + { + case int i: + case long l: + case object o: + case var v: break; } } @@ -2782,7 +2849,21 @@ static void Main() "; var compilation = CreateCompilationWithMscorlib45(source, options: TestOptions.ReleaseExe); compilation.VerifyDiagnostics( - // Per https://github.com/dotnet/roslyn/issues/17089, the compiler does not produce correct diagnostics for this. + // (11,18): error CS8120: The switch case has already been handled by a previous case. + // case var z: // error: subsumed + Diagnostic(ErrorCode.ERR_PatternIsSubsumed, "var z").WithLocation(11, 18), + // (18,18): error CS8120: The switch case has already been handled by a previous case. + // case var y: // error: subsumed + Diagnostic(ErrorCode.ERR_PatternIsSubsumed, "var y").WithLocation(18, 18), + // (24,13): error CS8120: The switch case has already been handled by a previous case. + // case 1: // error: subsumed + Diagnostic(ErrorCode.ERR_PatternIsSubsumed, "case 1:"), + // (30,18): error CS8120: The switch case has already been handled by a previous case. + // case int y: // error: subsumed + Diagnostic(ErrorCode.ERR_PatternIsSubsumed, "int y").WithLocation(30, 18), + // (37,13): error CS0152: The switch statement contains multiple cases with the label value 'null' + // case (string)null: // error: subsumed + Diagnostic(ErrorCode.ERR_DuplicateCaseLabel, "case (string)null:").WithArguments("null").WithLocation(37, 13) ); } }