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

Implement Async.Choice #807

Merged
merged 10 commits into from
Jan 20, 2016
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,128 @@
namespace FSharp.Core.Unittests.FSharp_Core.Microsoft_FSharp_Control

open System
open System.Threading
open FSharp.Core.Unittests.LibraryTestFx
open NUnit.Framework
#if !(FSHARP_CORE_PORTABLE || FSHARP_CORE_NETCORE_PORTABLE)
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 not available in Portable profile 7?

Copy link
Member Author

Choose a reason for hiding this comment

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

Build fails because it can't find the FsCheck dependency

Copy link
Member

Choose a reason for hiding this comment

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

Well it should go in each of the profiles, if it's a test issue ... perhaps test a different way or fix the test issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, if we're using FsCheck we should make it work on PCLs too? I understand that's a bigger issue, but maybe link to an FsCheck issue that we can track?

Copy link
Member Author

Choose a reason for hiding this comment

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

An easy solution for now would be to incorporate the non-FsCheck tests from #744

open FsCheck
#endif

#if !(FSHARP_CORE_PORTABLE || FSHARP_CORE_NETCORE_PORTABLE)
[<AutoOpen>]
module ChoiceUtils =

// FsCheck driven Async.Choice specification test

exception ChoiceExn of index:int

/// represents a child computation of a choice workflow
type ChoiceOp =
| NoneResultAfter of timeout:int
| SomeResultAfter of timeout:int
| ExceptionAfter of timeout:int

member c.Timeout =
match c with
| NoneResultAfter t -> t
| SomeResultAfter t -> t
| ExceptionAfter t -> t

/// represent a choice worfklow
type ChoiceWorkflow = ChoiceWorkflow of children:ChoiceOp list * cancelAfter:int option

/// normalizes random timeout arguments
let normalize (ChoiceWorkflow(ops, cancelAfter)) =
let ms t = 2000 * (abs t % 15) // timeouts only positive multiples of 2 seconds, up to 30 seconds
let mkOp op =
match op with
| NoneResultAfter t -> NoneResultAfter (ms t)
| SomeResultAfter t -> SomeResultAfter (ms t)
| ExceptionAfter t -> ExceptionAfter (ms t)

let ops = ops |> List.map mkOp
let cancelAfter = cancelAfter |> Option.map ms
ChoiceWorkflow(ops, cancelAfter)

/// runs specified choice workflow and checks that
/// Async.Choice spec is satisfied
let runChoice (ChoiceWorkflow(ops, cancelAfter)) =
// Step 1. build a choice workflow from the abstract representation
let completed = ref 0
Copy link
Contributor

Choose a reason for hiding this comment

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

we can use F# 4 i think, so let mutable completed instead of ref

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm OK with refs being explicit if the author wishes.

Copy link
Contributor

Choose a reason for hiding this comment

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

@dsyme you too in the ref club with @forki , @eiriktsarpalis and @mexx ? sigh 😄

let returnAfter time f = async {
do! Async.Sleep time
let _ = Interlocked.Increment completed
return f ()
}

let mkOp (index : int) = function
| NoneResultAfter t -> returnAfter t (fun () -> None)
| SomeResultAfter t -> returnAfter t (fun () -> Some index)
| ExceptionAfter t -> returnAfter t (fun () -> raise (ChoiceExn index))

let choiceWorkflow = ops |> List.mapi mkOp |> Async.Choice

// Step 2. run the choice workflow and keep the results
let result =
let cancellationToken =
match cancelAfter with
| Some ca ->
let cts = new CancellationTokenSource()
cts.CancelAfter(ca)
Some cts.Token
| None -> None

try Async.RunSynchronously(choiceWorkflow, ?cancellationToken = cancellationToken) |> Choice1Of2
with e -> Choice2Of2 e

// Step 3. check that results are up to spec
let getMinTime() =
seq {
yield Int32.MaxValue // "infinity": avoid exceptions if list is empty

for op in ops do
match op with
| NoneResultAfter _ -> ()
| op -> yield op.Timeout

match cancelAfter with Some t -> yield t | None -> ()
} |> Seq.min

let verifyIndex index =
if index < 0 || index >= ops.Length then
Assert.Fail "Returned choice index is out of bounds."

// Step 3a. check that output is up to spec
match result with
| Choice1Of2 (Some index) ->
verifyIndex index
match ops.[index] with
| SomeResultAfter timeout -> Assert.AreEqual(getMinTime(), timeout)
| op -> Assert.Fail <| sprintf "Should be 'Some' but got %A" op

| Choice1Of2 None ->
Assert.True(ops |> List.forall (function NoneResultAfter _ -> true | _ -> false))

| Choice2Of2 (:? OperationCanceledException) ->
match cancelAfter with
| None -> Assert.Fail "Got unexpected cancellation exception."
| Some ca -> Assert.AreEqual(getMinTime(), ca)

| Choice2Of2 (ChoiceExn index) ->
verifyIndex index
match ops.[index] with
| ExceptionAfter timeout -> Assert.AreEqual(getMinTime(), timeout)
| op -> Assert.Fail <| sprintf "Should be 'Exception' but got %A" op

| Choice2Of2 e -> Assert.Fail(sprintf "Unexpected exception %O" e)

// Step 3b. check that nested cancellation happens as expected
if not <| List.isEmpty ops then
let minTimeout = getMinTime()
let minTimeoutOps = ops |> Seq.filter (fun op -> op.Timeout <= minTimeout) |> Seq.length
Assert.LessOrEqual(!completed, minTimeoutOps)

#endif

module LeakUtils =
// when testing for liveness, the things that we want to observe must always be created in
Expand Down Expand Up @@ -294,6 +414,11 @@ type AsyncModule() =
member this.``RaceBetweenCancellationAndError.Sleep``() =
testErrorAndCancelRace (Async.Sleep (-5))

#if !(FSHARP_CORE_PORTABLE || FSHARP_CORE_NETCORE_PORTABLE)
[<Test; Category("CHOICE")>]
member this.``Async.Choice specification test``() =
Check.QuickThrowOnFailure (normalize >> runChoice)
#endif

[<Test>]
member this.``error on one workflow should cancel all others``() =
Expand Down
16 changes: 10 additions & 6 deletions src/fsharp/FSharp.Core.Unittests/LibraryTestFx.fs
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,13 @@ module SurfaceArea =
|> Array.map (fun v -> sprintf "%s: %s" (v.ReflectedType.ToString()) (v.ToString()))
#endif

types
|> Array.collect getTypeMemberStrings
|> Array.sort
|> String.concat "\r\n"
let actual =
types
|> Array.collect getTypeMemberStrings
|> Array.sort
|> String.concat "\r\n"

asm,actual

// verify public surface area matches expected
let verify expected platform fileName =
Expand All @@ -133,7 +136,8 @@ module SurfaceArea =
let normalize (s:string) =
Regex.Replace(s, "(\\r\\n|\\n)+", "\r\n").Trim([|'\r';'\n'|])

let actual = getActual () |> normalize
let asm, actualNotNormalized = getActual ()
let actual = actualNotNormalized |> normalize
let expected = expected |> normalize

Assert.AreEqual(expected, actual, sprintf "\r\n%s\r\n\r\n Expected and actual surface area don't match. To see the delta, run\r\nwindiff %s %s" actual fileName logFile)
Assert.AreEqual(expected, actual, sprintf "\r\nAssembly: %A\r\n\r\n%s\r\n\r\n Expected and actual surface area don't match. To see the delta, run\r\nwindiff %s %s" asm actual fileName logFile)
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,7 @@ Microsoft.FSharp.Control.FSharpAsync: Boolean Equals(System.Object)
Microsoft.FSharp.Control.FSharpAsync: Int32 GetHashCode()
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Control.FSharpAsync`1[T]] StartChild[T](Microsoft.FSharp.Control.FSharpAsync`1[T], Microsoft.FSharp.Core.FSharpOption`1[System.Int32])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpChoice`2[T,System.Exception]] Catch[T](Microsoft.FSharp.Control.FSharpAsync`1[T])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpOption`1[T]] Choice[T](System.Collections.Generic.IEnumerable`1[Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpOption`1[T]]])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] Ignore[T](Microsoft.FSharp.Control.FSharpAsync`1[T])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] Sleep(Int32)
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] SwitchToContext(System.Threading.SynchronizationContext)
Expand Down
1 change: 1 addition & 0 deletions src/fsharp/FSharp.Core.Unittests/SurfaceArea.net20.fs
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,7 @@ Microsoft.FSharp.Control.FSharpAsync: Boolean Equals(System.Object)
Microsoft.FSharp.Control.FSharpAsync: Int32 GetHashCode()
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Control.FSharpAsync`1[T]] StartChild[T](Microsoft.FSharp.Control.FSharpAsync`1[T], Microsoft.FSharp.Core.FSharpOption`1[System.Int32])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpChoice`2[T,System.Exception]] Catch[T](Microsoft.FSharp.Control.FSharpAsync`1[T])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpOption`1[T]] Choice[T](System.Collections.Generic.IEnumerable`1[Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpOption`1[T]]])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] Ignore[T](Microsoft.FSharp.Control.FSharpAsync`1[T])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] Sleep(Int32)
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] SwitchToContext(System.Threading.SynchronizationContext)
Expand Down
1 change: 1 addition & 0 deletions src/fsharp/FSharp.Core.Unittests/SurfaceArea.net40.fs
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,7 @@ Microsoft.FSharp.Control.FSharpAsync: Boolean Equals(System.Object)
Microsoft.FSharp.Control.FSharpAsync: Int32 GetHashCode()
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Control.FSharpAsync`1[T]] StartChild[T](Microsoft.FSharp.Control.FSharpAsync`1[T], Microsoft.FSharp.Core.FSharpOption`1[System.Int32])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpChoice`2[T,System.Exception]] Catch[T](Microsoft.FSharp.Control.FSharpAsync`1[T])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpOption`1[T]] Choice[T](System.Collections.Generic.IEnumerable`1[Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpOption`1[T]]])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] AwaitTask(System.Threading.Tasks.Task)
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] Ignore[T](Microsoft.FSharp.Control.FSharpAsync`1[T])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] Sleep(Int32)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,7 @@ Microsoft.FSharp.Control.FSharpAsync: Boolean Equals(System.Object)
Microsoft.FSharp.Control.FSharpAsync: Int32 GetHashCode()
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Control.FSharpAsync`1[T]] StartChild[T](Microsoft.FSharp.Control.FSharpAsync`1[T], Microsoft.FSharp.Core.FSharpOption`1[System.Int32])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpChoice`2[T,System.Exception]] Catch[T](Microsoft.FSharp.Control.FSharpAsync`1[T])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpOption`1[T]] Choice[T](System.Collections.Generic.IEnumerable`1[Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpOption`1[T]]])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] AwaitTask(System.Threading.Tasks.Task)
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] Ignore[T](Microsoft.FSharp.Control.FSharpAsync`1[T])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] Sleep(Int32)
Expand Down
1 change: 1 addition & 0 deletions src/fsharp/FSharp.Core.Unittests/SurfaceArea.portable47.fs
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,7 @@ Microsoft.FSharp.Control.FSharpAsync: Boolean Equals(System.Object)
Microsoft.FSharp.Control.FSharpAsync: Int32 GetHashCode()
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Control.FSharpAsync`1[T]] StartChild[T](Microsoft.FSharp.Control.FSharpAsync`1[T], Microsoft.FSharp.Core.FSharpOption`1[System.Int32])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpChoice`2[T,System.Exception]] Catch[T](Microsoft.FSharp.Control.FSharpAsync`1[T])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpOption`1[T]] Choice[T](System.Collections.Generic.IEnumerable`1[Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpOption`1[T]]])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] AwaitTask(System.Threading.Tasks.Task)
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] Ignore[T](Microsoft.FSharp.Control.FSharpAsync`1[T])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] Sleep(Int32)
Expand Down
1 change: 1 addition & 0 deletions src/fsharp/FSharp.Core.Unittests/SurfaceArea.portable7.fs
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,7 @@ Microsoft.FSharp.Control.FSharpAsync: Boolean Equals(System.Object)
Microsoft.FSharp.Control.FSharpAsync: Int32 GetHashCode()
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Control.FSharpAsync`1[T]] StartChild[T](Microsoft.FSharp.Control.FSharpAsync`1[T], Microsoft.FSharp.Core.FSharpOption`1[System.Int32])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpChoice`2[T,System.Exception]] Catch[T](Microsoft.FSharp.Control.FSharpAsync`1[T])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpOption`1[T]] Choice[T](System.Collections.Generic.IEnumerable`1[Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpOption`1[T]]])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] AwaitTask(System.Threading.Tasks.Task)
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] Ignore[T](Microsoft.FSharp.Control.FSharpAsync`1[T])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] Sleep(Int32)
Expand Down
1 change: 1 addition & 0 deletions src/fsharp/FSharp.Core.Unittests/SurfaceArea.portable78.fs
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,7 @@ Microsoft.FSharp.Control.FSharpAsync: Boolean Equals(System.Object)
Microsoft.FSharp.Control.FSharpAsync: Int32 GetHashCode()
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Control.FSharpAsync`1[T]] StartChild[T](Microsoft.FSharp.Control.FSharpAsync`1[T], Microsoft.FSharp.Core.FSharpOption`1[System.Int32])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpChoice`2[T,System.Exception]] Catch[T](Microsoft.FSharp.Control.FSharpAsync`1[T])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpOption`1[T]] Choice[T](System.Collections.Generic.IEnumerable`1[Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.FSharpOption`1[T]]])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] AwaitTask(System.Threading.Tasks.Task)
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] Ignore[T](Microsoft.FSharp.Control.FSharpAsync`1[T])
Microsoft.FSharp.Control.FSharpAsync: Microsoft.FSharp.Control.FSharpAsync`1[Microsoft.FSharp.Core.Unit] Sleep(Int32)
Expand Down
48 changes: 48 additions & 0 deletions src/fsharp/FSharp.Core/control.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1462,6 +1462,54 @@ namespace Microsoft.FSharp.Control
|> unfake);
FakeUnit))

static member Choice(computations : Async<'T option> seq) : Async<'T option> =
unprotectedPrimitive(fun args ->
let result =
try Seq.toArray computations |> Choice1Of2
with exn -> ExceptionDispatchInfo.RestoreOrCapture exn |> Choice2Of2

match result with
| Choice2Of2 edi -> args.aux.econt edi
| Choice1Of2 [||] -> args.cont None
| Choice1Of2 computations ->
protectedPrimitiveCore args (fun args ->
let ({ aux = aux } as args) = delimitSyncContext args
let noneCount = ref 0
let exnCount = ref 0
Copy link
Contributor

Choose a reason for hiding this comment

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

mutable instead of ref

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that for the sake of clarity, this should remain an explicit ref cell allocation. It is used precisely to synchronize across multiple threads, hence should be unambiguously heap allocated. In any case, changing this would not change the underlying IL.

Copy link
Contributor

Choose a reason for hiding this comment

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

eirik I think we both are now those old grumpy people who will continue to use ref cells even if mutable might work as well ;-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Damn kids with your mutable bindings

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm in your club too, Enrico also points me to mutable but I want to be ref :)

let innerCts = new LinkedSubSource(aux.token)
let trampolineHolder = aux.trampolineHolder

let scont (result : 'T option) =
match result with
| Some _ ->
if Interlocked.Increment exnCount = 1 then
innerCts.Cancel(); trampolineHolder.Protect(fun () -> args.cont result)
else
FakeUnit

| None ->
if Interlocked.Increment noneCount = computations.Length then
innerCts.Cancel(); trampolineHolder.Protect(fun () -> args.cont None)
else
FakeUnit

let econt (exn : ExceptionDispatchInfo) =
if Interlocked.Increment exnCount = 1 then
innerCts.Cancel(); trampolineHolder.Protect(fun () -> args.aux.econt exn)
else
FakeUnit

let ccont (exn : OperationCanceledException) =
if Interlocked.Increment exnCount = 1 then
innerCts.Cancel(); trampolineHolder.Protect(fun () -> args.aux.ccont exn)
else
FakeUnit

for c in computations do
queueAsync innerCts.Token scont econt ccont c |> unfake

FakeUnit))

#if FX_NO_TASK
#else
// Contains helpers that will attach continuation to the given task.
Expand Down
17 changes: 16 additions & 1 deletion src/fsharp/FSharp.Core/control.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -264,10 +264,25 @@ namespace Microsoft.FSharp.Control
/// The overall computation will respond to cancellation while executing the child computations.
/// If cancelled, the computation will cancel any remaining child computations but will still wait
/// for the other child computations to complete.</remarks>
/// <param name="computationList">A sequence of distinct computations to be parallelized.</param>
/// <param name="computations">A sequence of distinct computations to be parallelized.</param>
/// <returns>A computation that returns an array of values from the sequence of input computations.</returns>
static member Parallel : computations:seq<Async<'T>> -> Async<'T[]>

/// <summary>Creates an asynchronous computation that executes all given asynchronous computations in parallel,
/// returning the result of the first succeeding computation (i.e. the first computation with a result that is 'Some x').</summary>
Copy link
Contributor

Choose a reason for hiding this comment

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

This summary is a bit long. Normally it is one sentence, no i.e.

Copy link
Member Author

Choose a reason for hiding this comment

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

I basically just copied the format from the corresponding Async.Parallel segment.

Copy link
Contributor

Choose a reason for hiding this comment

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

i dont know if better, but returning blabla can be put inside returns xmldoc element

like <returns>the result of the first succeeding computation (i.e. the first computation with a result that is 'Some x'</returns>

it's used by xmldoc generator i think

///
/// <remarks>If all child computations complete with None, the parent computation also returns None.
///
/// If any child computation raises an exception, then the overall computation will trigger an
/// exception, and cancel the others.
///
/// The overall computation will respond to cancellation while executing the child computations.
/// If cancelled, the computation will cancel any remaining child computations but will still wait
/// for the other child computations to complete.</remarks>
/// <param name="computations">A sequence of computations to be parallelized.</param>
/// <returns>A computation that returns the first succeeding computation in the sequence of input computations.</returns>
Copy link
Contributor

Choose a reason for hiding this comment

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

The parameter name is wrong (should be computations)

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in b81dea5

Copy link
Contributor

Choose a reason for hiding this comment

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

in the sequence can be misleading, as it can imply the order is preserved, what is not the case.

Copy link
Member Author

Choose a reason for hiding this comment

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

agreed, we need to use wording in which 'first' clearly indicates the first to complete, as opposed to the first in the sequence. Any suggestions?

static member Choice : computations:seq<Async<'T option>> -> Async<'T option>

//---------- Thread Control

/// <summary>Creates an asynchronous computation that creates a new thread and runs
Expand Down