Skip to content

Remove List.headAndTail and some tuples #9336

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 29, 2020
Merged

Remove List.headAndTail and some tuples #9336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 0 additions & 7 deletions src/absil/illib.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,13 +334,6 @@ module List =
| [] -> None
| h :: t -> if f h then Some (h, List.rev acc @ t) else loop (h :: acc) t
loop [] inp

let headAndTail l =
match l with
| [] ->
Debug.Assert(false, "empty list")
failwith "List.headAndTail"
| h :: t -> h, t

let zip4 l1 l2 l3 l4 =
List.zip l1 (List.zip3 l2 l3 l4) |> List.map (fun (x1, (x2, x3, x4)) -> (x1, x2, x3, x4))
Expand Down
13 changes: 8 additions & 5 deletions src/fsharp/MethodCalls.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1867,12 +1867,15 @@ let GenWitnessExpr amap g m (traitInfo: TraitConstraintInfo) argExprs =

// Fix bug 1281: If we resolve to an instance method on a struct and we haven't yet taken
// the address of the object then go do that
if minfo.IsStruct && minfo.IsInstance && (match argExprs with [] -> false | h :: _ -> not (isByrefTy g (tyOfExpr g h))) then
let h, t = List.headAndTail argExprs
let wrap, h', _readonly, _writeonly = mkExprAddrOfExpr g true false PossiblyMutates h None m
Some (wrap (Expr.Op (TOp.TraitCall traitInfo, [], (h' :: t), m)))
if minfo.IsStruct && minfo.IsInstance then
match argExprs with
Copy link
Contributor

@cartermp cartermp May 29, 2020

Choose a reason for hiding this comment

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

This isn't quite the same, it's missing the [] -> false case.

Copy link
Contributor

@KevinRansom KevinRansom May 29, 2020

Choose a reason for hiding this comment

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

@cartermp , I believe that case is taken care of in isByRefTy but boolean logic has always been my downfall.

let isByrefTy g ty = 
    ty |> stripTyEqns g |> (function 
        | TType_app(tcref, _) when g.byref2_tcr.CanDeref -> tyconRefEq g g.byref2_tcr tcref
        | TType_app(tcref, _) -> tyconRefEq g g.byref_tcr tcref
        | _ -> false)

Copy link
Contributor

@cartermp cartermp May 29, 2020

Choose a reason for hiding this comment

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

I also suffer from boolean logic, which is also why I think it'd be better if the original [] -> false case was restored

Copy link
Contributor Author

@forki forki May 29, 2020

Choose a reason for hiding this comment

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

@cartermp no the [] case is still there. It's just rearranged

Copy link
Contributor Author

@forki forki May 29, 2020

Choose a reason for hiding this comment

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

Or maybe more correct: one of the two tests for [] is still in place

Copy link
Contributor

@KevinRansom KevinRansom May 29, 2020

Choose a reason for hiding this comment

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

It is, I see it now, I was always hopeless at where's waldo.

Copy link
Contributor

@cartermp cartermp May 29, 2020

Choose a reason for hiding this comment

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

aha, I think I see it now too. The original code was so hard to read

| h :: t when not (isByrefTy g (tyOfExpr g h)) ->
let wrap, h', _readonly, _writeonly = mkExprAddrOfExpr g true false PossiblyMutates h None m
Some (wrap (Expr.Op (TOp.TraitCall traitInfo, [], (h' :: t), m)))
| _ ->
Some (MakeMethInfoCall amap m minfo methArgTys argExprs)
else
Some (MakeMethInfoCall amap m minfo methArgTys argExprs )
Some (MakeMethInfoCall amap m minfo methArgTys argExprs)

| Choice2Of5 (tinst, rfref, isSet) ->
match isSet, rfref.RecdField.IsStatic, argExprs.Length with
Expand Down
14 changes: 8 additions & 6 deletions src/fsharp/NicePrint.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1472,12 +1472,14 @@ module InfoMemberPrinting =
let prettifyILMethInfo (amap: Import.ImportMap) m (minfo: MethInfo) typarInst ilMethInfo =
let (ILMethInfo(_, apparentTy, dty, mdef, _)) = ilMethInfo
let (prettyTyparInst, prettyTys), _ = PrettyTypes.PrettifyInstAndTypes amap.g (typarInst, (apparentTy :: minfo.FormalMethodInst))
let prettyApparentTy, prettyFormalMethInst = List.headAndTail prettyTys
let prettyMethInfo =
match dty with
| None -> MethInfo.CreateILMeth (amap, m, prettyApparentTy, mdef)
| Some declaringTyconRef -> MethInfo.CreateILExtensionMeth(amap, m, prettyApparentTy, declaringTyconRef, minfo.ExtensionMemberPriorityOption, mdef)
prettyTyparInst, prettyMethInfo, prettyFormalMethInst
match prettyTys with
| prettyApparentTy :: prettyFormalMethInst ->
let prettyMethInfo =
match dty with
| None -> MethInfo.CreateILMeth (amap, m, prettyApparentTy, mdef)
| Some declaringTyconRef -> MethInfo.CreateILExtensionMeth(amap, m, prettyApparentTy, declaringTyconRef, minfo.ExtensionMemberPriorityOption, mdef)
prettyTyparInst, prettyMethInfo, prettyFormalMethInst
| _ -> failwith "prettifyILMethInfo - prettyTys empty"

/// Format a method to a buffer using "standalone" display style.
/// For example, these are the formats used when printing signatures of methods that have not been overridden,
Expand Down
3 changes: 1 addition & 2 deletions src/fsharp/TypeChecker.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9265,9 +9265,8 @@ and TcNameOfExpr cenv env tpenv (synArg: SynExpr) =
let m = cleanSynArg.Range
let rec check overallTyOpt expr (delayed: DelayedItem list) =
match expr with
| LongOrSingleIdent (false, (LongIdentWithDots(longId, _) as lidd), _, _) when longId.Length > 0 ->
| LongOrSingleIdent (false, (LongIdentWithDots((id::rest) as longId, _) as lidd), _, _) ->
let ad = env.eAccessRights
let id, rest = List.headAndTail longId
match ResolveLongIndentAsModuleOrNamespaceOrStaticClass cenv.tcSink ResultCollectionSettings.AllResults cenv.amap m false true OpenQualified env.eNameResEnv ad id rest true with
| Result modref when delayed.IsEmpty && modref |> List.exists (p23 >> IsEntityAccessible cenv.amap m ad) ->
() // resolved to a module or namespace, done with checks
Expand Down
13 changes: 9 additions & 4 deletions src/fsharp/fsi/fsi.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2266,10 +2266,15 @@ type internal FsiInteractionProcessor

// When the last declaration has a shape of DoExp (i.e., non-binding),
// transform it to a shape of "let it = <exp>", so we can refer it.
let defsA = if defsA.Length <= 1 || not (List.isEmpty defsB) then defsA else
match List.headAndTail (List.rev defsA) with
| SynModuleDecl.DoExpr(_,exp,_), rest -> (rest |> List.rev) @ (fsiDynamicCompiler.BuildItBinding exp)
| _ -> defsA
let defsA =
if not (isNil defsB) then defsA else
match defsA with
| [] -> defsA
| [_] -> defsA
| _ ->
match List.rev defsA with
| SynModuleDecl.DoExpr(_,exp,_) :: rest -> (rest |> List.rev) @ (fsiDynamicCompiler.BuildItBinding exp)
| _ -> defsA

Some (IDefns(defsA,m)),Some (IDefns(defsB,m)),istate

Expand Down