Optimize Seq.Last to behave like Seq.Length (incl. tests) (#7765)

* Optimize Seq.Last like Seq.Length

Add type check to Seq.Last and Seq.TryLast to avoid full iteration if not necessary. Like Seq.Length does it at
https://github.com/dotnet/fsharp/blob/c18e1780b3f3f345364cb1ad8e510ea9f4590d3a/src/fsharp/FSharp.Core/seq.fs#L709

* Add test for Optimized Seq.Last and Seq.TryLast

* style update

* style update 2

* update comments and fix build error

Github Build error was:
 Check failure on line 132 in tests\FSharp.Core.UnitTests\FSharp.Core\Microsoft.FSharp.Collections\SeqModule2.fs

@azure-pipelines
azure-pipelines
/ fsharp-ci (Build Windows vs_release)

tests\FSharp.Core.UnitTests\FSharp.Core\Microsoft.FSharp.Collections\SeqModule2.fs#L132
tests\FSharp.Core.UnitTests\FSharp.Core\Microsoft.FSharp.Collections\SeqModule2.fs(132,58): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'a has been constrained to be type 'unit'.

* include List.last case

* include list in tests

* ensure same exception is raised on empty list

* typo

* Added implementation of List.last since it is not available at this point due to compilation error

The recursive function could also be defined inside but i guess then it would be reallocated on every call to Seq.Last (in case of list match).
An alternative would be to add .Last member on list type (like .Length member)

* inline List.last

reverting my previous attempt  https://github.com/dotnet/fsharp/pull/7765/commits/b329e23c543b41b36143fafc77187acc58585471
tail rec functions should be inlined

* typo in tests(build failed)

* typo2 in test (Build failed)

* move implemnetation to internal module

as suggested by dsyme

* renamed internal tryLast to tryLastV

src/fsharp/FSharp.Core/list.fs

@@ -9,6 +9,7 @@ namespace Microsoft.FSharp.Collections
     open Microsoft.FSharp.Collections
     open Microsoft.FSharp.Core.CompilerServices
     open System.Collections.Generic
+    
 
     [<CompilationRepresentation(CompilationRepresentationFlags.ModuleSuffix)>]
     [<RequireQualifiedAccess>]
@@ -24,18 +25,16 @@ namespace Microsoft.FSharp.Collections
         let length (list: 'T list) = list.Length
 
         [<CompiledName("Last")>]
-        let rec last (list: 'T list) =
-            match list with
-            | [x] -> x
-            | _ :: tail -> last tail
-            | [] -> invalidArg "list" (SR.GetString(SR.inputListWasEmpty))
+        let last (list: 'T list) =
+            match Microsoft.FSharp.Primitives.Basics.List.tryLastV list with
+            | ValueSome x -> x
+            | ValueNone -> invalidArg "list" (SR.GetString(SR.inputListWasEmpty))
 
         [<CompiledName("TryLast")>]
         let rec tryLast (list: 'T list) =
-            match list with
-            | [x] -> Some x
-            | _ :: tail -> tryLast tail
-            | [] -> None
+            match Microsoft.FSharp.Primitives.Basics.List.tryLastV list with
+            | ValueSome x -> Some x
+            | ValueNone -> None            
 
         [<CompiledName("Reverse")>]
         let rev list = Microsoft.FSharp.Primitives.Basics.List.rev list

src/fsharp/FSharp.Core/local.fs

@@ -988,6 +988,12 @@ module internal List =
             takeWhileFreshConsTail cons p xs
             cons
 
+    let rec tryLastV (list: 'T list) = 
+        match list with
+        | [] -> ValueNone
+        | [x] -> ValueSome x        
+        | _ :: tail -> tryLastV tail           
+
 module internal Array =
 
     open System
@@ -1187,3 +1193,26 @@ module internal Array =
                 res.[i] <- subUnchecked !startIndex minChunkSize array
                 startIndex := !startIndex + minChunkSize
             res
+
+module internal Seq =
+    let tryLastV (source : seq<_>) =
+        //checkNonNull "source" source //done in main Seq.tryLast
+        match source with
+        | :? ('T[]) as a -> 
+            if a.Length = 0 then ValueNone
+            else ValueSome(a.[a.Length - 1])
+        
+        | :? ('T IList) as a -> //ResizeArray and other collections
+            if a.Count = 0 then ValueNone
+            else ValueSome(a.[a.Count - 1])
+        
+        | :? ('T list) as a -> List.tryLastV a 
+        
+        | _ -> 
+            use e = source.GetEnumerator()
+            if e.MoveNext() then
+                let mutable res = e.Current
+                while (e.MoveNext()) do res <- e.Current
+                ValueSome(res)
+            else
+                ValueNone
\ No newline at end of file

src/fsharp/FSharp.Core/local.fsi

@@ -65,6 +65,7 @@ module internal List =
     val splitAt : int -> 'T list -> ('T list * 'T list)
     val transpose : 'T list list -> 'T list list
     val truncate : int -> 'T list -> 'T list
+    val tryLastV : 'T list -> 'T ValueOption
 
 module internal Array =
     // The input parameter should be checked by callers if necessary
@@ -101,3 +102,6 @@ module internal Array =
     val stableSortInPlaceWith: comparer:('T -> 'T -> int) -> array:'T[] -> unit
 
     val stableSortInPlace: array:'T[] -> unit when 'T : comparison 
+
+module internal Seq =
+    val tryLastV : 'T seq -> 'T ValueOption

src/fsharp/FSharp.Core/seq.fs

@@ -1377,29 +1377,21 @@ namespace Microsoft.FSharp.Collections
                       invalidArg "source" (SR.GetString(SR.notEnoughElements))
                   while e.MoveNext() do
                       yield e.Current }
-
+                           
         [<CompiledName("Last")>]
         let last (source : seq<_>) =
             checkNonNull "source" source
-            use e = source.GetEnumerator()
-            if e.MoveNext() then
-                let mutable res = e.Current
-                while (e.MoveNext()) do res <- e.Current
-                res
-            else
-                invalidArg "source" LanguagePrimitives.ErrorStrings.InputSequenceEmptyString
-
+            match Microsoft.FSharp.Primitives.Basics.Seq.tryLastV source with
+            | ValueSome x -> x
+            | ValueNone -> invalidArg "source" LanguagePrimitives.ErrorStrings.InputSequenceEmptyString
+        
         [<CompiledName("TryLast")>]
         let tryLast (source : seq<_>) =
             checkNonNull "source" source
-            use e = source.GetEnumerator()
-            if e.MoveNext() then
-                let mutable res = e.Current
-                while (e.MoveNext()) do res <- e.Current
-                Some res
-            else
-                None
-
+            match Microsoft.FSharp.Primitives.Basics.Seq.tryLastV source with
+            | ValueSome x -> Some x
+            | ValueNone -> None
+            
         [<CompiledName("ExactlyOne")>]
         let exactlyOne (source : seq<_>) =
             checkNonNull "source" source

tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/SeqModule2.fs

@@ -99,7 +99,57 @@ type SeqModule2() =
         // null Seq
         let nullSeq:seq<'a> = null
         CheckThrowsArgumentNullException (fun () ->Seq.last nullSeq) 
+                
+        
+        // ------ Test for Array -----
+        let IntArr = Array.ofSeq IntSeq                     
+        if Seq.last IntArr <> 9 then Assert.Fail()
+                 
+        // string Array
+        let strArr = Array.ofSeq strSeq 
+        if Seq.last strArr <> "third" then Assert.Fail()
+         
+        // Empty Array
+        let emptyArr = [| |]
+        CheckThrowsArgumentException ( fun() -> Seq.last emptyArr)
+      
+        // null Array
+        let nullArr: array<'a> = null
+        CheckThrowsArgumentNullException (fun () ->Seq.last nullArr) 
+        
+        // ---- Test for IList -----
+        let IntRarr = ResizeArray(IntSeq)
+        if Seq.last IntRarr <> 9 then Assert.Fail()
+                 
+        // string IList
+        let strRarr = ResizeArray(strSeq) 
+        if Seq.last strRarr <> "third" then Assert.Fail()
+         
+        // Empty IList
+        let emptyRarr = ResizeArray<unit>()
+        CheckThrowsArgumentException ( fun() -> Seq.last emptyRarr)
+      
+        // null IList
+        let nullRarr: ResizeArray<unit> = null
+        CheckThrowsArgumentNullException (fun () ->Seq.last nullRarr) 
+        
+        // ---- Test for list -----
+        let Intlist = List.ofSeq(IntSeq)
+        if Seq.last Intlist <> 9 then Assert.Fail()
+                 
+        // string list
+        let strlist = List.ofSeq(strSeq) 
+        if Seq.last strlist <> "third" then Assert.Fail()
+         
+        // Empty list
+        let emptylist: list<unit> = []
+        CheckThrowsArgumentException ( fun() -> Seq.last emptylist)
+      
+        // null list
+        let nullList: list<unit> = Unchecked.defaultof<list<unit>>
+        CheckThrowsArgumentNullException (fun () ->Seq.last nullList) 
         () 
+        
 
     [<Test>]
     member this.TryLast() =
@@ -120,8 +170,63 @@ type SeqModule2() =
       
         // null Seq
         let nullSeq:seq<'a> = null
-        CheckThrowsArgumentNullException (fun () ->Seq.tryLast nullSeq |> ignore) 
+        CheckThrowsArgumentNullException (fun () ->Seq.tryLast nullSeq |> ignore)
+
+        // ------ Test for Array -----
+        let IntArr = Array.ofSeq IntSeq                     
+        let intResult = Seq.tryLast IntArr
+        Assert.AreEqual(9, intResult.Value)
+                 
+        // string Array
+        let strResult = Seq.tryLast (Array.ofSeq (["first"; "second";  "third"]))
+        Assert.AreEqual("third", strResult.Value)
+         
+        // Empty Array
+        let emptyResult = Seq.tryLast Array.empty
+        Assert.IsTrue(emptyResult.IsNone)
+      
+        // null Array
+        let nullArr:array<unit> = null
+        CheckThrowsArgumentNullException (fun () -> Seq.tryLast nullArr |> ignore) 
+
+
+        // ------ Test for IList -----
+        let IntRarr = ResizeArray( IntSeq )
+        let intResult = Seq.tryLast IntRarr
+        Assert.AreEqual(9, intResult.Value)
+                 
+        // string IList
+        let strResult = Seq.tryLast (ResizeArray (["first"; "second";  "third"]))
+        Assert.AreEqual("third", strResult.Value)
+         
+        // Empty IList
+        let emptyResult = Seq.tryLast (ResizeArray<unit>())
+        Assert.IsTrue(emptyResult.IsNone)
+      
+        // null IList
+        let nullRarr:ResizeArray<unit> = null
+        CheckThrowsArgumentNullException (fun () ->Seq.tryLast nullRarr |> ignore) 
+        
+        // ------ Test for list -----
+        let Intlist= List.ofSeq( IntSeq )
+        let intResult = Seq.tryLast Intlist
+        Assert.AreEqual(9, intResult.Value)
+                 
+        // string list
+        let strResult = Seq.tryLast ["first"; "second";  "third"]
+        Assert.AreEqual("third", strResult.Value)
+         
+        // Empty list
+        let emptylist: list<unit> = []
+        let emptyResult = Seq.tryLast emptylist
+        Assert.IsTrue(emptyResult.IsNone)
+      
+        // null list
+        let nullList: list<unit> = Unchecked.defaultof<list<unit>>
+        CheckThrowsArgumentNullException (fun () ->Seq.tryLast nullList |> ignore) 
         () 
+
+
         
     [<Test>]
     member this.ExactlyOne() =