From e758c51f22b02ce10061bee5fee8c2eb2e3a89e0 Mon Sep 17 00:00:00 2001
From: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>
Date: Thu, 17 Dec 2015 23:50:45 +0200
Subject: [PATCH] incorporate feedback

---
 .../Microsoft.FSharp.Control/AsyncModule.fs   | 14 +++++++------
 src/fsharp/FSharp.Core/control.fs             | 20 +++++++++++--------
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/src/fsharp/FSharp.Core.Unittests/FSharp.Core/Microsoft.FSharp.Control/AsyncModule.fs b/src/fsharp/FSharp.Core.Unittests/FSharp.Core/Microsoft.FSharp.Control/AsyncModule.fs
index 9eaa098d1a8..703788c4dec 100644
--- a/src/fsharp/FSharp.Core.Unittests/FSharp.Core/Microsoft.FSharp.Control/AsyncModule.fs
+++ b/src/fsharp/FSharp.Core.Unittests/FSharp.Core/Microsoft.FSharp.Control/AsyncModule.fs
@@ -11,11 +11,13 @@ open FSharp.Core.Unittests.LibraryTestFx
 open NUnit.Framework
 #if !(FSHARP_CORE_PORTABLE || FSHARP_CORE_NETCORE_PORTABLE)
 open FsCheck
+#endif
 
+#if !(FSHARP_CORE_PORTABLE || FSHARP_CORE_NETCORE_PORTABLE)
 [<AutoOpen>]
 module ChoiceUtils =
 
-    // FsCheck driven Async.Choice tests
+    // FsCheck driven Async.Choice specification test
 
     exception ChoiceExn of index:int
 
@@ -24,7 +26,7 @@ module ChoiceUtils =
         | NoneResultAfter of timeout:int
         | SomeResultAfter of timeout:int
         | ExceptionAfter of timeout:int
-    with
+
         member c.Timeout =
             match c with
             | NoneResultAfter t -> t
@@ -61,7 +63,7 @@ module ChoiceUtils =
         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)
+            | ExceptionAfter t -> returnAfter t (fun () -> raise (ChoiceExn index))
 
         let choiceWorkflow = ops |> List.mapi mkOp |> Async.Choice
 
@@ -92,7 +94,7 @@ module ChoiceUtils =
             } |> Seq.min
 
         let verifyIndex index =
-            if index < 0 || index >=  ops.Length then
+            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
@@ -413,8 +415,8 @@ type AsyncModule() =
         testErrorAndCancelRace (Async.Sleep (-5))
 
 #if !(FSHARP_CORE_PORTABLE || FSHARP_CORE_NETCORE_PORTABLE)
-    [<Test>]
-    member this.``Async.Choice correctness and cancellation``() =
+    [<Test; Category("CHOICE")>]
+    member this.``Async.Choice specification test``() =
         Check.QuickThrowOnFailure (normalize >> runChoice)
 #endif
 
diff --git a/src/fsharp/FSharp.Core/control.fs b/src/fsharp/FSharp.Core/control.fs
index 40809908e81..5ba22fb7dcc 100644
--- a/src/fsharp/FSharp.Core/control.fs
+++ b/src/fsharp/FSharp.Core/control.fs
@@ -1465,13 +1465,12 @@ namespace Microsoft.FSharp.Control
         static member Choice(computations : Async<'T option> seq) : Async<'T option> =
             unprotectedPrimitive(fun args ->
                 let result =
-                    try Choice1Of2 <| Seq.toArray computations
-                    with exn -> Choice2Of2 <| ExceptionDispatchInfo.RestoreOrCapture(exn)
+                    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 [|P body|] -> body args
                 | Choice1Of2 computations ->
                     protectedPrimitiveCore args (fun args ->
                         let ({ aux = aux } as args) = delimitSyncContext args
@@ -1482,12 +1481,17 @@ namespace Microsoft.FSharp.Control
 
                         let scont (result : 'T option) =
                             match result with
-                            | Some _ when Interlocked.Increment exnCount = 1 -> 
-                                innerCts.Cancel(); trampolineHolder.Protect(fun () -> args.cont result)
-                            | None when Interlocked.Increment noneCount = computations.Length -> 
-                                innerCts.Cancel(); trampolineHolder.Protect(fun () -> args.cont None)
+                            | Some _ -> 
+                                if Interlocked.Increment exnCount = 1 then
+                                    innerCts.Cancel(); trampolineHolder.Protect(fun () -> args.cont result)
+                                else
+                                    FakeUnit
 
-                            | _ -> 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