diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs index 23e6573dd27d1aa45025b0158d406e3811d40095..776baa3c90e3c489c010202960e726d0ed74d3d4 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs @@ -555,7 +555,7 @@ private static bool WasImplicitReceiver(BoundExpression receiverOpt) /// /// This method implements the checks in spec section 15.2. /// - private bool MethodGroupIsCompatibleWithDelegate(BoundExpression receiverOpt, bool isExtensionMethod, MethodSymbol method, NamedTypeSymbol delegateType, Location errorLocation, DiagnosticBag diagnostics) + internal bool MethodGroupIsCompatibleWithDelegate(BoundExpression receiverOpt, bool isExtensionMethod, MethodSymbol method, NamedTypeSymbol delegateType, Location errorLocation, DiagnosticBag diagnostics) { Debug.Assert(delegateType.TypeKind == TypeKind.Delegate); Debug.Assert((object)delegateType.DelegateInvokeMethod != null && !delegateType.DelegateInvokeMethod.HasUseSiteError, @@ -613,22 +613,6 @@ private bool MethodGroupIsCompatibleWithDelegate(BoundExpression receiverOpt, bo } diagnostics.Add(errorLocation, useSiteDiagnostics); - - if (method.IsConditional) - { - // CS1618: Cannot create delegate with '{0}' because it has a Conditional attribute - Error(diagnostics, ErrorCode.ERR_DelegateOnConditional, errorLocation, method); - return false; - } - - var sourceMethod = method as SourceMemberMethodSymbol; - if ((object)sourceMethod != null && sourceMethod.IsPartialWithoutImplementation) - { - // CS0762: Cannot create delegate from method '{0}' because it is a partial method without an implementing declaration - Error(diagnostics, ErrorCode.ERR_PartialMethodToDelegate, errorLocation, method); - return false; - } - return true; } @@ -660,6 +644,21 @@ private bool MethodGroupIsCompatibleWithDelegate(BoundExpression receiverOpt, bo return true; } + if (selectedMethod.IsConditional) + { + // CS1618: Cannot create delegate with '{0}' because it has a Conditional attribute + Error(diagnostics, ErrorCode.ERR_DelegateOnConditional, syntax.Location, selectedMethod); + return true; + } + + var sourceMethod = selectedMethod as SourceMemberMethodSymbol; + if ((object)sourceMethod != null && sourceMethod.IsPartialWithoutImplementation) + { + // CS0762: Cannot create delegate from method '{0}' because it is a partial method without an implementing declaration + Error(diagnostics, ErrorCode.ERR_PartialMethodToDelegate, syntax.Location, selectedMethod); + return true; + } + if (selectedMethod.HasUnsafeParameter() || selectedMethod.ReturnType.IsUnsafe()) { return ReportUnsafeIfNotAllowed(syntax, diagnostics); diff --git a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs index e4a407274b8ab731266b25988d9f80cf4a30de41..761fb3acbd387ddcc5968f2cc4117c65bb5cfd22 100644 --- a/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs +++ b/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs @@ -1807,7 +1807,7 @@ private BetterResult BetterConversionFromExpression(BoundExpression node, TypeSy } // - T1 is a better conversion target than T2 - return BetterConversionTarget(lambdaOpt, t1, t2, ref useSiteDiagnostics, out okToDowngradeToNeither); + return BetterConversionTarget(node, t1, conv1, t2, conv2, ref useSiteDiagnostics, out okToDowngradeToNeither); } private bool ExpressionMatchExactly(BoundExpression node, TypeSymbol t, ref HashSet useSiteDiagnostics) @@ -1946,7 +1946,13 @@ public override BoundNode VisitReturnStatement(BoundReturnStatement node) } } - private BetterResult BetterConversionTarget(UnboundLambda lambdaOpt, TypeSymbol type1, TypeSymbol type2, ref HashSet useSiteDiagnostics, out bool okToDowngradeToNeither) + private BetterResult BetterConversionTarget(TypeSymbol type1, TypeSymbol type2, ref HashSet useSiteDiagnostics) + { + bool okToDowngradeToNeither; + return BetterConversionTarget(null, type1, default(Conversion), type2, default(Conversion), ref useSiteDiagnostics, out okToDowngradeToNeither); + } + + private BetterResult BetterConversionTarget(BoundExpression node, TypeSymbol type1, Conversion conv1, TypeSymbol type2, Conversion conv2, ref HashSet useSiteDiagnostics, out bool okToDowngradeToNeither) { okToDowngradeToNeither = false; @@ -1960,6 +1966,7 @@ private BetterResult BetterConversionTarget(UnboundLambda lambdaOpt, TypeSymbol // and at least one of the following holds: bool type1ToType2 = Conversions.ClassifyImplicitConversion(type1, type2, ref useSiteDiagnostics).IsImplicit; bool type2ToType1 = Conversions.ClassifyImplicitConversion(type2, type1, ref useSiteDiagnostics).IsImplicit; + UnboundLambda lambdaOpt = node as UnboundLambda; if (type1ToType2) { @@ -1980,7 +1987,6 @@ private BetterResult BetterConversionTarget(UnboundLambda lambdaOpt, TypeSymbol return BetterResult.Right; } - bool ignore; var task_T = Compilation.GetWellKnownType(WellKnownType.System_Threading_Tasks_Task_T); if ((object)task_T != null) @@ -1990,11 +1996,9 @@ private BetterResult BetterConversionTarget(UnboundLambda lambdaOpt, TypeSymbol if (type2.OriginalDefinition == task_T) { // - T1 is Task, T2 is Task, and S1 is a better conversion target than S2 - return BetterConversionTarget(null, - ((NamedTypeSymbol)type1).TypeArgumentsNoUseSiteDiagnostics[0], + return BetterConversionTarget(((NamedTypeSymbol)type1).TypeArgumentsNoUseSiteDiagnostics[0], ((NamedTypeSymbol)type2).TypeArgumentsNoUseSiteDiagnostics[0], - ref useSiteDiagnostics, - out ignore); + ref useSiteDiagnostics); } // A shortcut, Task type cannot satisfy other rules. @@ -2025,23 +2029,49 @@ private BetterResult BetterConversionTarget(UnboundLambda lambdaOpt, TypeSymbol { TypeSymbol r1 = invoke1.ReturnType; TypeSymbol r2 = invoke2.ReturnType; + BetterResult delegateResult = BetterResult.Neither; if (r1.SpecialType != SpecialType.System_Void) { if (r2.SpecialType == SpecialType.System_Void) { // - D2 is void returning - return BetterResult.Left; + delegateResult = BetterResult.Left; } } else if (r2.SpecialType != SpecialType.System_Void) { // - D2 is void returning - return BetterResult.Right; + delegateResult = BetterResult.Right; + } + + if (delegateResult == BetterResult.Neither) + { + // - D2 has a return type S2, and S1 is a better conversion target than S2 + delegateResult = BetterConversionTarget(r1, r2, ref useSiteDiagnostics); + } + + // Downgrade result to Neither if conversion used by the winner isn't actually valid method group conversion. + // This is necessary to preserve compatibility, otherwise we might dismiss "worse", but trully applicable candidate + // based on a "better", but, in reality, erroneous one. + if (node?.Kind == BoundKind.MethodGroup) + { + var group = (BoundMethodGroup)node; + + if (delegateResult == BetterResult.Left) + { + if (IsMethodGroupConversionIncompatibleWithDelegate(group, d1, conv1)) + { + return BetterResult.Neither; + } + } + else if (delegateResult == BetterResult.Right && IsMethodGroupConversionIncompatibleWithDelegate(group, d2, conv2)) + { + return BetterResult.Neither; + } } - // - D2 has a return type S2, and S1 is a better conversion target than S2 - return BetterConversionTarget(null, r1, r2, ref useSiteDiagnostics, out ignore); + return delegateResult; } } @@ -2074,6 +2104,19 @@ private BetterResult BetterConversionTarget(UnboundLambda lambdaOpt, TypeSymbol return BetterResult.Neither; } + private bool IsMethodGroupConversionIncompatibleWithDelegate(BoundMethodGroup node, NamedTypeSymbol delegateType, Conversion conv) + { + if (conv.IsMethodGroup) + { + DiagnosticBag ignore = DiagnosticBag.GetInstance(); + bool result = !_binder.MethodGroupIsCompatibleWithDelegate(node.ReceiverOpt, conv.IsExtensionMethod, conv.Method, delegateType, Location.None, ignore); + ignore.Free(); + return result; + } + + return false; + } + private bool CanDowngradeConversionFromLambdaToNeither(BetterResult currentResult, UnboundLambda lambda, TypeSymbol type1, TypeSymbol type2, ref HashSet useSiteDiagnostics, bool fromTypeAnalysis) { // DELIBERATE SPEC VIOLATION: See bug 11961. @@ -2149,7 +2192,6 @@ private bool CanDowngradeConversionFromLambdaToNeither(BetterResult currentResul #if DEBUG if (fromTypeAnalysis) { - bool ignore; // Since we are dealing with variance delegate conversion and delegates have identical parameter // lists, return types must be implicitly convertible in the same direction. // Or we might be dealing with error return types and we may have one error delegate matching exactly @@ -2157,7 +2199,7 @@ private bool CanDowngradeConversionFromLambdaToNeither(BetterResult currentResul Debug.Assert( r1.IsErrorType() || r2.IsErrorType() || - currentResult == BetterConversionTarget(null, r1, r2, ref useSiteDiagnostics, out ignore)); + currentResult == BetterConversionTarget(r1, r2, ref useSiteDiagnostics)); } #endif } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionTests.cs index ee9c47b8ae531422817daa0931178b6d11722b6a..f916e2d21cf43449b4d39bb0b0e5ff9bd2aaf0f6 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/OverloadResolutionTests.cs @@ -472,7 +472,7 @@ static void Main() } [Fact] - public void BetterDelegateType() + public void BetterDelegateType_01() { string source1 = @" using System; @@ -520,6 +520,61 @@ static void Main() ); } + [Fact, WorkItem(6560, "https://github.com/dotnet/roslyn/issues/6560")] + public void BetterDelegateType_02() + { + string source1 = @" +using System; + +class C +{ + public static void Main() + { + Run1(() => MethodReturnsVoid()); + Run1(MethodReturnsVoid); + Run2(() => MethodReturnsVoid()); + Run2(MethodReturnsVoid); + } + + public static object Run1(Action action) + { + Console.WriteLine(""Run1(Action action)""); + action(); + return null; + } + + public static object Run1(Func action, bool optional = false) + { + Console.WriteLine(""Run1(Func action, bool optional = false)""); + return action(); + } + + public static object Run2(Func action, bool optional = false) + { + Console.WriteLine(""Run2(Func action, bool optional = false)""); + return action(); + } + + public static object Run2(Action action) + { + Console.WriteLine(""Run2(Action action)""); + action(); + return null; + } + + private static void MethodReturnsVoid() + { + } +} +"; + + CompileAndVerify(source1, expectedOutput: +@"Run1(Action action) +Run1(Action action) +Run2(Action action) +Run2(Action action)"); + } + [Fact] public void TestBug9851() {