dotnet/roslyn

Slightly confusing comments in BetterConversionTarget(TypeSymbol, TypeSymbol, ...)

Open

#7,859 opened on Jan 8, 2016

View on GitHub
 (0 comments) (0 reactions) (0 assignees)C# (20,414 stars) (4,257 forks)batch import
Area-CompilersDocumentationhelp wanted

Description

The quoted method in src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs has code like

             // Given two different types T1 and T2, T1 is a better conversion target than T2 if no implicit conversion from T2 to T1 exists, 
              // 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;

and

                 // - An implicit conversion from T1 to T2 exists 
                 okToDowngradeToNeither = lambdaOpt != null && CanDowngradeConversionFromLambdaToNeither(BetterResult.Left, lambdaOpt, type1, type2, ref useSiteDiagnostics, true);
                 return BetterResult.Left;

where you might expect that the names in the quoted spec correspond to the similarly-named variables in the code. However, there are other places in the code where we reverse the correspondence between the quoted spec and the names of the variables. For example we have

                     // - T1 is either a delegate type D1 or an expression tree type Expression<D1>,
                     //   T2 is either a delegate type D2 or an expression tree type Expression<D2>,
                     //   D1 has a return type S1 and one of the following holds:
                     MethodSymbol invoke1 = d1.DelegateInvokeMethod;
                     MethodSymbol invoke2 = d2.DelegateInvokeMethod;
                     if ((object)invoke1 != null && (object)invoke2 != null)
                     {
                         TypeSymbol r1 = invoke1.ReturnType;
                         TypeSymbol r2 = invoke2.ReturnType;

and then later

                       else if (r2.SpecialType != SpecialType.System_Void)
                       {
                             // - D2 is void returning
                             delegateResult = BetterResult.Right;
                        }

The comment says "D2 is void-returning", but it is inside an if-statement that ensures that the return type of the delegate type type2 does not return void.

This is confusing. It would be helpful to have something said here that alerts the reader to the way the spec should be understood to correspond to the code.

Contributor guide