Evaluate traverse left to right
#418
Conversation
|
@wallymathieu , it seems that I fail on one Fable test and I don't know why. Is it something obvious that I'm missing? |
|
I don't think it's an obvious thing. It looks as if the Fable code assumes that you have a Js iterator and fails due to the fact that what it gets is not something you can iterate over. While debugging locally I get that the value is |
|
Looks like the type inference in this branch differs from master. On master the check is if it's not null or undefined, while on this branch it infers the check to be if it's a non empty sequence. |
| let cons x y = seq {yield x; yield! y} | ||
| let cons_f x ys = Map.Invoke (cons: 'a->seq<_>->seq<_>) (f x) <*> ys | ||
| Map.Invoke NonEmptySeq.ofSeq (Seq.foldBack cons_f t (result Seq.empty)) | ||
|
|
There was a problem hiding this comment.
Here's something that might be causing the issue.
Try not changing the overloads (not adding or removing) and their signature and there should be no reason to change the overload resolution logic in Fable.
If you have to repeat code, go ahead.
| let rec loop acc = function | ||
| | [] -> acc | ||
| | x::xs -> let v = f x | ||
| if v |> IsLeftZero.Invoke |> not |
There was a problem hiding this comment.
Using IsLeftZero here changes the signature and hence the overload resolution logic.
I know it would make it more optimal but optimization should be a separate PR.
|
Here's a partial explanation of what happened:
Introduction of additional constraints might affect overload resolution logic. Lesson learned: let's don't try to be too smart in these sensible scenarios. Better to apply small changes, in this case try only to change the desired behavior and leave optimizations for other PRs. A possible micro optimization here is to change Array.cons definition to something like this will avoid the singleton array allocation, but we have to test if it really impacts execution times. But this is for anothter PR ;) |
Fixes #414