Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CompilerPerf] Pretify ConstrainsSolver method #5141

Merged
merged 2 commits into from
Jun 12, 2018
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 63 additions & 57 deletions src/fsharp/ConstraintSolver.fs
Original file line number Diff line number Diff line change
@@ -1914,67 +1914,73 @@ and CanMemberSigsMatchUpToCheck
let unnamedCalledOutArgs = calledMeth.UnnamedCalledOutArgs

// First equate the method instantiation (if any) with the method type parameters
if minst.Length <> uminst.Length then ErrorD(Error(FSComp.SR.csTypeInstantiationLengthMismatch(), m)) else

Iterate2D unifyTypes minst uminst ++ (fun () ->

if not (permitOptArgs || isNil unnamedCalledOptArgs) then ErrorD(Error(FSComp.SR.csOptionalArgumentNotPermittedHere(), m)) else


let calledObjArgTys = calledMeth.CalledObjArgTys(m)
if minst.Length <> uminst.Length then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this outside the trackErrors?

ErrorD(Error(FSComp.SR.csTypeInstantiationLengthMismatch(), m))
else
trackErrors {
do! Iterate2D unifyTypes minst uminst
do!
if not (permitOptArgs || isNil unnamedCalledOptArgs) then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you don't need the do!, you can go straight into the if and then return! on each branch

        trackErrors {
            do! Iterate2D unifyTypes minst uminst
            if not (permitOptArgs || isNil unnamedCalledOptArgs) then 
                return! ErrorD(Error(FSComp.SR.csOptionalArgumentNotPermittedHere(), m)) 
            else
             ....

ErrorD(Error(FSComp.SR.csOptionalArgumentNotPermittedHere(), m))
else
let calledObjArgTys = calledMeth.CalledObjArgTys(m)

// Check all the argument types.
// Check all the argument types.

if calledObjArgTys.Length <> callerObjArgTys.Length then
if (calledObjArgTys.Length <> 0) then
ErrorD(Error (FSComp.SR.csMemberIsNotStatic(minfo.LogicalName), m))
else
ErrorD(Error (FSComp.SR.csMemberIsNotInstance(minfo.LogicalName), m))
else
Iterate2D subsumeTypes calledObjArgTys callerObjArgTys ++ (fun () ->
(calledMeth.ArgSets |> IterateD (fun argSet ->
if argSet.UnnamedCalledArgs.Length <> argSet.UnnamedCallerArgs.Length then ErrorD(Error(FSComp.SR.csArgumentLengthMismatch(), m)) else
Iterate2D subsumeArg argSet.UnnamedCalledArgs argSet.UnnamedCallerArgs)) ++ (fun () ->
(calledMeth.ParamArrayCalledArgOpt |> OptionD (fun calledArg ->
if isArray1DTy g calledArg.CalledArgumentType then
let paramArrayElemTy = destArrayTy g calledArg.CalledArgumentType
let reflArgInfo = calledArg.ReflArgInfo // propgate the reflected-arg info to each param array argument
calledMeth.ParamArrayCallerArgs |> OptionD (IterateD (fun callerArg -> subsumeArg (CalledArg((0, 0), false, NotOptional, NoCallerInfo, false, false, None, reflArgInfo, paramArrayElemTy)) callerArg))
else
CompleteD)

) ++ (fun () ->
(calledMeth.ArgSets |> IterateD (fun argSet ->
argSet.AssignedNamedArgs |> IterateD (fun arg -> subsumeArg arg.CalledArg arg.CallerArg))) ++ (fun () ->
(assignedItemSetters |> IterateD (fun (AssignedItemSetter(_, item, caller)) ->
let name, calledArgTy =
match item with
| AssignedPropSetter(_, pminfo, pminst) ->
let calledArgTy = List.head (List.head (pminfo.GetParamTypes(amap, m, pminst)))
pminfo.LogicalName, calledArgTy

| AssignedILFieldSetter(finfo) ->
(* Get or set instance IL field *)
let calledArgTy = finfo.FieldType(amap, m)
finfo.FieldName, calledArgTy
if calledObjArgTys.Length <> callerObjArgTys.Length then
if (calledObjArgTys.Length <> 0) then
ErrorD(Error (FSComp.SR.csMemberIsNotStatic(minfo.LogicalName), m))
else
ErrorD(Error (FSComp.SR.csMemberIsNotInstance(minfo.LogicalName), m))
else
Iterate2D subsumeTypes calledObjArgTys callerObjArgTys
do! calledMeth.ArgSets
|> IterateD (fun argSet ->
if argSet.UnnamedCalledArgs.Length <> argSet.UnnamedCallerArgs.Length then ErrorD(Error(FSComp.SR.csArgumentLengthMismatch(), m)) else
Iterate2D subsumeArg argSet.UnnamedCalledArgs argSet.UnnamedCallerArgs)
do! calledMeth.ParamArrayCalledArgOpt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be best to use

    match calledMeth.ParamArrayCalledArgOpt with 
    | None -> ()
    | Some calledArg -> 
        ...

|> OptionD
(fun calledArg ->
if isArray1DTy g calledArg.CalledArgumentType then
let paramArrayElemTy = destArrayTy g calledArg.CalledArgumentType
let reflArgInfo = calledArg.ReflArgInfo // propgate the reflected-arg info to each param array argument
calledMeth.ParamArrayCallerArgs |> OptionD (IterateD (fun callerArg -> subsumeArg (CalledArg((0, 0), false, NotOptional, NoCallerInfo, false, false, None, reflArgInfo, paramArrayElemTy)) callerArg))
else
CompleteD)
do! calledMeth.ArgSets
|> IterateD (fun argSet ->
argSet.AssignedNamedArgs |> IterateD (fun arg -> subsumeArg arg.CalledArg arg.CallerArg))
do! assignedItemSetters
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use a for loop here, since trackErrors has a For method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to implement more methods so the for do is usable.

|> IterateD (fun (AssignedItemSetter(_, item, caller)) ->
let name, calledArgTy =
match item with
| AssignedPropSetter(_, pminfo, pminst) ->
let calledArgTy = List.head (List.head (pminfo.GetParamTypes(amap, m, pminst)))
pminfo.LogicalName, calledArgTy

| AssignedILFieldSetter(finfo) ->
(* Get or set instance IL field *)
let calledArgTy = finfo.FieldType(amap, m)
finfo.FieldName, calledArgTy

| AssignedRecdFieldSetter(rfinfo) ->
let calledArgTy = rfinfo.FieldType
rfinfo.Name, calledArgTy
| AssignedRecdFieldSetter(rfinfo) ->
let calledArgTy = rfinfo.FieldType
rfinfo.Name, calledArgTy

subsumeArg (CalledArg((-1, 0), false, NotOptional, NoCallerInfo, false, false, Some (mkSynId m name), ReflectedArgInfo.None, calledArgTy)) caller) )) ++ (fun () ->

// - Always take the return type into account for
// -- op_Explicit, op_Implicit
// -- methods using tupling of unfilled out args
// - Never take into account return type information for constructors
match reqdRetTyOpt with
| None -> CompleteD
| Some _ when minfo.IsConstructor -> CompleteD
| Some _ when not alwaysCheckReturn && isNil unnamedCalledOutArgs -> CompleteD
| Some reqdRetTy ->
let methodRetTy = calledMeth.CalledReturnTypeAfterOutArgTupling
unifyTypes reqdRetTy methodRetTy )))))
subsumeArg (CalledArg((-1, 0), false, NotOptional, NoCallerInfo, false, false, Some (mkSynId m name), ReflectedArgInfo.None, calledArgTy)) caller)
do!
// - Always take the return type into account for
// -- op_Explicit, op_Implicit
// -- methods using tupling of unfilled out args
// - Never take into account return type information for constructors
match reqdRetTyOpt with
| None -> CompleteD
| Some _ when minfo.IsConstructor -> CompleteD
| Some _ when not alwaysCheckReturn && isNil unnamedCalledOutArgs -> CompleteD
| Some reqdRetTy ->
let methodRetTy = calledMeth.CalledReturnTypeAfterOutArgTupling
unifyTypes reqdRetTy methodRetTy
}

// Assert a subtype constraint, and wrap an ErrorsFromAddingSubsumptionConstraint error around any failure
// to allow us to report the outer types involved in the constraint