diff --git a/src/Compilers/CSharp/Portable/Lowering/DiagnosticsPass_Warnings.cs b/src/Compilers/CSharp/Portable/Lowering/DiagnosticsPass_Warnings.cs index 60bb1bfe6ef326eeb2bf38ffad46ed8777d7c605..b28728f9cf26924f8bc564bcc0299666a955d3a5 100644 --- a/src/Compilers/CSharp/Portable/Lowering/DiagnosticsPass_Warnings.cs +++ b/src/Compilers/CSharp/Portable/Lowering/DiagnosticsPass_Warnings.cs @@ -744,13 +744,16 @@ private void CheckLiftedBinOp(BoundBinaryOperator node) string always = node.OperatorKind.Operator() == BinaryOperatorKind.NotEqual ? "true" : "false"; - if (node.Right.NullableNeverHasValue() && node.Left.NullableAlwaysHasValue()) + if (this._compilation.FeatureStrictEnabled || !node.OperatorKind.IsUserDefined()) { - Error(node.OperatorKind.IsUserDefined() ? ErrorCode.WRN_NubExprIsConstBool2 : ErrorCode.WRN_NubExprIsConstBool, node, always, node.Left.Type.GetNullableUnderlyingType(), GetTypeForLiftedComparisonWarning(node.Right)); - } - else if (node.Left.NullableNeverHasValue() && node.Right.NullableAlwaysHasValue()) - { - Error(node.OperatorKind.IsUserDefined() ? ErrorCode.WRN_NubExprIsConstBool2 : ErrorCode.WRN_NubExprIsConstBool, node, always, node.Right.Type.GetNullableUnderlyingType(), GetTypeForLiftedComparisonWarning(node.Left)); + if (node.Right.NullableNeverHasValue() && node.Left.NullableAlwaysHasValue()) + { + Error(node.OperatorKind.IsUserDefined() ? ErrorCode.WRN_NubExprIsConstBool2 : ErrorCode.WRN_NubExprIsConstBool, node, always, node.Left.Type.GetNullableUnderlyingType(), GetTypeForLiftedComparisonWarning(node.Right)); + } + else if (node.Left.NullableNeverHasValue() && node.Right.NullableAlwaysHasValue()) + { + Error(node.OperatorKind.IsUserDefined() ? ErrorCode.WRN_NubExprIsConstBool2 : ErrorCode.WRN_NubExprIsConstBool, node, always, node.Right.Type.GetNullableUnderlyingType(), GetTypeForLiftedComparisonWarning(node.Left)); + } } break; case BinaryOperatorKind.Or: diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenOptimizedNullableOperators.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenOptimizedNullableOperators.cs index 5d131c82f195b5bde64e8ae1fe571c891514615e..6868c83fe63cf9fa035b4a8de5fd9a01a47449d6 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenOptimizedNullableOperators.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenOptimizedNullableOperators.cs @@ -990,7 +990,7 @@ .maxstack 2 comp.VerifyIL("Program.M6", expectedIL6); } - [Fact] + [Fact, WorkItem(663, "https://github.com/dotnet/roslyn/issues/663")] public void TestNullableComparisonOpsOneNullOneNonNull() { // An ==, !=, <, >, <= or >= operation where one operand is null and the @@ -1000,9 +1000,10 @@ public void TestNullableComparisonOpsOneNullOneNonNull() // Note that Roslyn produces considerably more warnings here than the // native compiler; the native compiler only produces warnings for // "((int?)null) < new decimal?(N3())" and "((S?)null) < new S?(N4())". - // However, the native compiler does not produce the warning - // "comparing null with S? always produces false" -- it incorrectly warns - // that it produces a null of type bool? ! Roslyn does not reproduce this bug. + // For compatibility Roslyn reports the same diagnostics by default, + // but in "strict" mode (which will be part of the "warning waves" once + // we do that) Roslyn will report warnings for + // new S() == null and new S() != null. string source = @" struct S // User-defined relational ops @@ -1092,7 +1093,21 @@ .maxstack 1 }"; string expectedIL6 = expectedIL4; - var comp = CompileAndVerify(source, expectedOutput: expectedOutput); + CompileAndVerify(source, expectedOutput: expectedOutput).VerifyDiagnostics( + // (21,16): warning CS0472: The result of the expression is always 'false' since a value of type 'int' is never equal to 'null' of type 'short?' + // return new int?(N1()) == new short?(); + Diagnostic(ErrorCode.WRN_NubExprIsConstBool, "new int?(N1()) == new short?()").WithArguments("false", "int", "short?").WithLocation(21, 16), + // (25,16): warning CS0472: The result of the expression is always 'true' since a value of type 'double' is never equal to 'null' of type 'double?' + // return default(double?) != new short?(N2()); + Diagnostic(ErrorCode.WRN_NubExprIsConstBool, "default(double?) != new short?(N2())").WithArguments("true", "double", "double?").WithLocation(25, 16), + // (29,16): warning CS0464: Comparing with null of type 'int?' always produces 'false' + // return ((int?)null) < new decimal?(N3()); + Diagnostic(ErrorCode.WRN_CmpAlwaysFalse, "((int?)null) < new decimal?(N3())").WithArguments("int?").WithLocation(29, 16), + // (41,16): warning CS0464: Comparing with null of type 'S?' always produces 'false' + // return ((S?)null) < new S?(N4()); + Diagnostic(ErrorCode.WRN_CmpAlwaysFalse, "((S?)null) < new S?(N4())").WithArguments("S?").WithLocation(41, 16) + ); + var comp = CompileAndVerify(source, expectedOutput: expectedOutput, options: TestOptions.ReleaseExe.WithStrictMode()); comp.VerifyDiagnostics( // (21,16): warning CS0472: The result of the expression is always 'false' since a value of type 'int' is never equal to 'null' of type 'short?' // return new int?(N1()) == new short?(); diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs index 2d65688a713cd15c0912ae797d96298865e91c18..247a43454fff185b86bfa7882c14cab7072ca716 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/SemanticErrorTests.cs @@ -19034,12 +19034,13 @@ static void Main() new ErrorDescription[] { new ErrorDescription { Code = (int)ErrorCode.WRN_GotoCaseShouldConvert, Line = 13, Column = 13, IsWarning = true } }); } - [Fact] + [Fact, WorkItem(663, "https://github.com/dotnet/roslyn/issues/663")] public void CS0472WRN_NubExprIsConstBool() { - // Due to a long-standing bug, the native compiler does not produce warnings for "guid == null", but does - // for "int == null". Roslyn corrects this lapse and produces warnings for both built-in and - // user-defined lifted equality operators. + // Due to a long-standing bug, the native compiler does not produce warnings for "guid == null", + // but does for "int == null". Roslyn corrects this lapse and produces warnings for both built-in + // and user-defined lifted equality operators, but the new warnings for user-defined types are + // only given in "strict" more. var text = @" using System; @@ -19145,8 +19146,7 @@ public static void Main() ftftftftftftftftftftftft tf ftftftft"; - var comp = this.CompileAndVerify(source: text, expectedOutput: expected); - comp.VerifyDiagnostics( + var fullExpected = new DiagnosticDescription[] { // (19,11): warning CS0472: The result of the expression is always 'false' since a value of type 'int' is never equal to 'null' of type 'int?' // W(i == null); // CS0472 Diagnostic(ErrorCode.WRN_NubExprIsConstBool, "i == null").WithArguments("false", "int", "int?").WithLocation(19, 11), @@ -19243,7 +19243,10 @@ public static void Main() // (96,11): warning CS0472: The result of the expression is always 'true' since a value of type 'MyClass.E' is never equal to 'null' of type 'MyClass.E?' // W((E?)null != 0); Diagnostic(ErrorCode.WRN_NubExprIsConstBool, "(E?)null != 0").WithArguments("true", "MyClass.E", "MyClass.E?").WithLocation(96, 11) - ); + }; + var compatibleExpected = fullExpected.Where(d => !d.Code.Equals((int)ErrorCode.WRN_NubExprIsConstBool2)).ToArray(); + this.CompileAndVerify(source: text, expectedOutput: expected).VerifyDiagnostics(compatibleExpected); + this.CompileAndVerify(source: text, expectedOutput: expected, options: TestOptions.ReleaseExe.WithStrictMode()).VerifyDiagnostics(fullExpected); } [Fact] diff --git a/src/Compilers/Test/Utilities/CSharp/TestOptions.cs b/src/Compilers/Test/Utilities/CSharp/TestOptions.cs index 7ff118593839e427ca3a616a5355f2511a9a6cd9..f14705aa70fe38c8d0f44b6d4af8f50c02e41cb5 100644 --- a/src/Compilers/Test/Utilities/CSharp/TestOptions.cs +++ b/src/Compilers/Test/Utilities/CSharp/TestOptions.cs @@ -1,5 +1,7 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Collections.Immutable; + namespace Microsoft.CodeAnalysis.CSharp.Test.Utilities { public static class TestOptions @@ -32,5 +34,10 @@ public static class TestOptions public static readonly CSharpCompilationOptions UnsafeDebugDll = DebugDll.WithAllowUnsafe(true); public static readonly CSharpCompilationOptions UnsafeDebugExe = DebugExe.WithAllowUnsafe(true); + + public static CSharpCompilationOptions WithStrictMode(this CSharpCompilationOptions options) + { + return options.WithFeatures(options.Features.Add("strict")); + } } } diff --git a/src/Test/Utilities/DiagnosticDescription.cs b/src/Test/Utilities/DiagnosticDescription.cs index f2bcc2cf67246780c04d9e1404b8bc73f935f076..4a768c9a8ff7bb49a0f03fb287db7b4d77c1a8ea 100644 --- a/src/Test/Utilities/DiagnosticDescription.cs +++ b/src/Test/Utilities/DiagnosticDescription.cs @@ -192,6 +192,8 @@ public DiagnosticDescription WhereSyntax(Func syntaxPredicate) return new DiagnosticDescription(_code, _isWarningAsError, _squiggledText, _arguments, _startPosition, syntaxPredicate, _argumentOrderDoesNotMatter, _errorCodeType); } + public object Code => _code; + public override bool Equals(object obj) { var d = obj as DiagnosticDescription;