Array.insertManyAt returns original array for empty insertion

[albert-du]

When calling Array.insertManyAt, if the values inserted are empty, the source array is always returned, rather than a copy.

Documented behavior is "Return a new array with new items inserted before the given index." however returning the original source can create unexpected side effects when the values being inserted are empty.

Consider:

    let original = [| 1; 2; 3 |]

    let newArr = Array.insertManyAt 3 [| 4; 5 |] original

    printfn $"%b{original = newArr}"
    //  false

    printfn $"%A{original}"
    printfn  $"%A{newArr}" 
    //  [|1; 2; 3|]
    //  [|1; 2; 3; 4; 5|]

    // do some weird things to newArr

    newArr[0] <- 987654321

    printfn $"%A{original}"
    printfn  $"%A{newArr}" 
    //   [|1; 2; 3|]
    //   [|987654321; 2; 3; 4; 5|]

    // original is unaffected

sharplab

and when the values inserted are empty

    let original = [| 1; 2; 3 |]
    
    // insert empty array
    let newArr = Array.insertManyAt 3 [||] original
    
    printfn $"%b{original = newArr}"
    //  true   (that's strange)
    
    printfn $"%A{original}"
    printfn  $"%A{newArr}" 
    //  [|1; 2; 3|]
    //  [|1; 2; 3|]
    
    // do some weird things to newArr
    
    newArr[0] <- 987654321
    
    printfn $"%A{original}"
    printfn  $"%A{newArr}" 
    //   [|987654321; 2; 3|]
    //   [|987654321; 2; 3|]
    
    // original has been affected by a mutation on the 'new' array

sharplab

Expected behavior

Always create a new array.

Actual behavior

When the inserted values are empty, the original array, rather than a copy, is returned.

Known workarounds

Preemptively copy the source array, or copy the returned value.

Related information

I believe the cause is

fsharp/src/FSharp.Core/array.fs

Lines 1936 to 1938 in 32c2764

	if valuesArray.Length = 0 then
	source
	else

Fix would be adding a call to Array.copy in this branch.

This does not affect Seq.insertManyAt and List.insertManyAt as seq and list immutable.

Provide any related information (optional):

.NET / FSharp.Core 9.0

[muqiuhan]

^_^ Maybe we can just use Clone, because checkNonNull is already done at the beginning of insertManyAt:

fsharp/src/FSharp.Core/array.fs

Lines 292 to 294 in 32c2764

	let copy (array: 'T array) =
	checkNonNull "array" array
	(array.Clone() :?> 'T array) // this is marginally faster