Skip to content

Commit

Permalink
Take double conversions into account for overload resolution
Browse files Browse the repository at this point in the history
Fixes test failures for cases where separate overloads have args like Nullable<int> and Nullable<int64> in the same position
  • Loading branch information
NinoFloris committed Aug 17, 2022
1 parent 8ae92d0 commit a81a9ce
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 16 deletions.
2 changes: 1 addition & 1 deletion src/Compiler/Checking/CheckExpressions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,7 @@ let UnifyOverallType cenv (env: TcEnv) m overallTy actualTy =
| None -> ()

match usesTDC with
| TypeDirectedConversionUsed.Yes warn -> warning(warn env.DisplayEnv)
| TypeDirectedConversionUsed.Yes(warn, _) -> warning(warn env.DisplayEnv)
| TypeDirectedConversionUsed.No -> ()

if AddCxTypeMustSubsumeTypeUndoIfFailed env.DisplayEnv cenv.css m reqdTy2 actualTy then
Expand Down
12 changes: 8 additions & 4 deletions src/Compiler/Checking/ConstraintSolver.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2738,7 +2738,7 @@ and ArgsMustSubsumeOrConvert
msg csenv.DisplayEnv
| None -> ()
match usesTDC with
| TypeDirectedConversionUsed.Yes warn -> do! WarnD(warn csenv.DisplayEnv)
| TypeDirectedConversionUsed.Yes(warn, _) -> do! WarnD(warn csenv.DisplayEnv)
| TypeDirectedConversionUsed.No -> ()
do! SolveTypeSubsumesTypeWithReport csenv ndeep m trace cxsln (Some calledArg.CalledArgumentType) calledArgTy callerArg.CallerArgumentType
if calledArg.IsParamArray && isArray1DTy g calledArgTy && not (isArray1DTy g callerArg.CallerArgumentType) then
Expand Down Expand Up @@ -2769,7 +2769,7 @@ and ArgsMustSubsumeOrConvertWithContextualReport
msg csenv.DisplayEnv
| None -> ()
match usesTDC with
| TypeDirectedConversionUsed.Yes warn -> do! WarnD(warn csenv.DisplayEnv)
| TypeDirectedConversionUsed.Yes(warn, _) -> do! WarnD(warn csenv.DisplayEnv)
| TypeDirectedConversionUsed.No -> ()
do! SolveTypeSubsumesTypeWithWrappedContextualReport csenv ndeep m trace cxsln (Some calledArg.CalledArgumentType) calledArgTy callerArgTy (fun e -> ArgDoesNotMatchError(e :?> _, calledMeth, calledArg, callerArg))
return usesTDC
Expand All @@ -2796,7 +2796,7 @@ and ReturnTypesMustSubsumeOrConvert (csenv: ConstraintSolverEnv) ad ndeep trace
msg csenv.DisplayEnv
| None -> ()
match usesTDC with
| TypeDirectedConversionUsed.Yes warn -> do! WarnD(warn csenv.DisplayEnv)
| TypeDirectedConversionUsed.Yes(warn, _) -> do! WarnD(warn csenv.DisplayEnv)
| TypeDirectedConversionUsed.No -> ()
do! SolveTypeSubsumesTypeWithReport csenv ndeep m trace cxsln None reqdTy actualTy
return usesTDC
Expand All @@ -2813,7 +2813,7 @@ and ArgsEquivOrConvert (csenv: ConstraintSolverEnv) ad ndeep trace cxsln isConst
msg csenv.DisplayEnv
| None -> ()
match usesTDC with
| TypeDirectedConversionUsed.Yes warn -> do! WarnD(warn csenv.DisplayEnv)
| TypeDirectedConversionUsed.Yes(warn, _) -> do! WarnD(warn csenv.DisplayEnv)
| TypeDirectedConversionUsed.No -> ()
if not (typeEquiv csenv.g calledArgTy callerArgTy) then
return! ErrorD(Error(FSComp.SR.csArgumentTypesDoNotMatch(), m))
Expand Down Expand Up @@ -3223,6 +3223,10 @@ and GetMostApplicableOverload csenv ndeep candidates applicableMeths calledMethG
// Prefer methods that don't use type-directed conversion
let c = compare (match usesTDC1 with TypeDirectedConversionUsed.No -> 1 | _ -> 0) (match usesTDC2 with TypeDirectedConversionUsed.No -> 1 | _ -> 0)
if c <> 0 then c else

// Prefer methods that need less type-directed conversion
let c = compare (match usesTDC1 with TypeDirectedConversionUsed.Yes(_, false) -> 1 | _ -> 0) (match usesTDC2 with TypeDirectedConversionUsed.Yes(_, false) -> 1 | _ -> 0)
if c <> 0 then c else

// Prefer methods that don't give "this code is less generic" warnings
// Note: Relies on 'compare' respecting true > false
Expand Down
23 changes: 13 additions & 10 deletions src/Compiler/Checking/MethodCalls.fs
Original file line number Diff line number Diff line change
Expand Up @@ -236,12 +236,15 @@ type TypeDirectedConversion =

[<RequireQualifiedAccess>]
type TypeDirectedConversionUsed =
| Yes of (DisplayEnv -> exn)
| Yes of (DisplayEnv -> exn) * isDoubleConversion: bool
| No
static member Combine a b =
match a with
| Yes _ -> a
| No -> b
match a, b with
| Yes(_,true), _ -> a
| _, Yes(_,true) -> b
| Yes _, _ -> a
| _, Yes _ -> b
| No, No -> a

let MapCombineTDCD mapper xs =
MapReduceD mapper TypeDirectedConversionUsed.No TypeDirectedConversionUsed.Combine xs
Expand Down Expand Up @@ -279,33 +282,33 @@ let rec AdjustRequiredTypeForTypeDirectedConversions (infoReader: InfoReader) ad

// Adhoc int32 --> int64
elif g.langVersion.SupportsFeature LanguageFeature.AdditionalTypeDirectedConversions && typeEquiv g g.int64_ty reqdTy && typeEquiv g g.int32_ty actualTy then
g.int32_ty, TypeDirectedConversionUsed.Yes(warn TypeDirectedConversion.BuiltIn), None
g.int32_ty, TypeDirectedConversionUsed.Yes(warn TypeDirectedConversion.BuiltIn, false), None

// Adhoc int32 --> nativeint
elif g.langVersion.SupportsFeature LanguageFeature.AdditionalTypeDirectedConversions && typeEquiv g g.nativeint_ty reqdTy && typeEquiv g g.int32_ty actualTy then
g.int32_ty, TypeDirectedConversionUsed.Yes(warn TypeDirectedConversion.BuiltIn), None
g.int32_ty, TypeDirectedConversionUsed.Yes(warn TypeDirectedConversion.BuiltIn, false), None

// Adhoc int32 --> float64
elif g.langVersion.SupportsFeature LanguageFeature.AdditionalTypeDirectedConversions && typeEquiv g g.float_ty reqdTy && typeEquiv g g.int32_ty actualTy then
g.int32_ty, TypeDirectedConversionUsed.Yes(warn TypeDirectedConversion.BuiltIn), None
g.int32_ty, TypeDirectedConversionUsed.Yes(warn TypeDirectedConversion.BuiltIn, false), None

elif g.langVersion.SupportsFeature LanguageFeature.NullableOptionalInterop && isMethodArg && isNullableTy g reqdTy && not (isNullableTy g actualTy) then
let underlyingTy = destNullableTy g reqdTy
// shortcut
if typeEquiv g underlyingTy actualTy then
actualTy, TypeDirectedConversionUsed.Yes(warn TypeDirectedConversion.BuiltIn), None
actualTy, TypeDirectedConversionUsed.Yes(warn TypeDirectedConversion.BuiltIn, false), None
else
let adjustedTy, _, _ = AdjustRequiredTypeForTypeDirectedConversions infoReader ad isMethodArg isConstraint underlyingTy actualTy m
if typeEquiv g adjustedTy actualTy then
actualTy, TypeDirectedConversionUsed.Yes(warn TypeDirectedConversion.BuiltIn), None
actualTy, TypeDirectedConversionUsed.Yes(warn TypeDirectedConversion.BuiltIn, true), None
else
reqdTy, TypeDirectedConversionUsed.No, None

// Adhoc based on op_Implicit, perhaps returing a new equational type constraint to
// eliminate articifical constrained type variables.
elif g.langVersion.SupportsFeature LanguageFeature.AdditionalTypeDirectedConversions then
match TryFindRelevantImplicitConversion infoReader ad reqdTy actualTy m with
| Some (minfo, _staticTy, eqn) -> actualTy, TypeDirectedConversionUsed.Yes(warn (TypeDirectedConversion.Implicit minfo)), Some eqn
| Some (minfo, _staticTy, eqn) -> actualTy, TypeDirectedConversionUsed.Yes(warn (TypeDirectedConversion.Implicit minfo), false), Some eqn
| None -> reqdTy, TypeDirectedConversionUsed.No, None

else reqdTy, TypeDirectedConversionUsed.No, None
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Checking/MethodCalls.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ type CallerArgs<'T> =
/// has been used in F# code
[<RequireQualifiedAccess>]
type TypeDirectedConversionUsed =
| Yes of (DisplayEnv -> exn)
| Yes of (DisplayEnv -> exn) * isDoubleConversion: bool
| No

static member Combine: TypeDirectedConversionUsed -> TypeDirectedConversionUsed -> TypeDirectedConversionUsed
Expand Down

0 comments on commit a81a9ce

Please sign in to comment.