Replace (nearly) all ref cells in the compiler with mutable values

[cartermp]

The large majority of ref cells in the compiler codebase are trivially replaceable with mutable values. A few are not, and so those are mostly left untouched. I think this helps with codebase readability.

Excludes test code

[NatElkins]

Is there any performance benefit to this? Or is it just a stylistic change?

[cartermp]

Mostly stylistic, and most of the uses seem to predate let mutable. In many cases this is a micro optimization since it avoids a small heap allocation, but in it won't make a noticeable difference.

[heronbpv]

Taking the opportunity to also make a question: is there any remaining case (or cases) where usage of a ref cell is recommended over a mutable let?

[cartermp]

@heronbpv There are, but I'd say the uses are limited. The syntax just looks kind of wonky and the dereference operator ! is just plain confusing to people.

For bogstandard mutation that's local to a function or method, typically done for perf reasons, mutable values are preferred. Same goes for ref in a record label versus just making it a mutable label.

I'd also say that using a byref<'T> to mutate when passing by reference is also preferred. However, this does mean you are constrained in a few ways:

• Taking an address of something with & only gives a byref<'T> on let mutable-bound values. It will produce an inref<'T> for everything else, which is readonly.
• You cannot use higher-order functions with the data you're mutating
• You cannot allow a reference to escape the scope it was declared in

In those cases, you'll need to declared something as a ref if you need to "pass this thing by reference" as a scenario.

Additionally, tupled values for DUs do not support mutable values, so you need to declare them as a ref if you want a DU case that can be mutated.

type DU =
    | A of int * mutable int // Not supported, needs to be 'int ref'
    | B of int * string

Additionally, there is a pattern in this codebase where a value is locked via lock and mutated within the lock expression. It's fairly convenient to just lock on a ref cell to the value being mutated within a lock:

fsharp/src/fsharp/FSharp.Core/seq.fs

Lines 1015 to 1021 in e69e4f1

	lock enumeratorR (fun () ->
	prefix.Clear()
	begin match !enumeratorR with
	| Some (Some e) -> IEnumerator.dispose e
	| _ -> ()
	end
	enumeratorR := None)

Finally, there are some cases where a mutable value will actually be converted into a ref cell, such as enclosing the value in a lambda function that is also the return value:

let f =
    let mutable x = 0
    fun _ -> x <- x + 1
    
let g =
    let x = ref 0
    fun _ -> x := !x + 1
    
f()
f()
g()
g()

Both function values are equivalent.

This is getting into pretty niche territory though.

I'd say that these cases tend to be pretty niche in F# programming, since mutating values is usually done local to a function or method and done for performance reasons. In those cases, let mutable generally looks cleaner and avoids an additional allocation.

[heronbpv]

Thanks for the answer, @cartermp !

[KevinRansom]

@cartermp , this is good work. I will mark it approved, and leave it to you to deal with:

   let mutable table = mtyp.ActivePatternElemRefLookupTable
    cacheOptRef &table (fun () ->

I think just reverting the change should be good enough.

Kevin

[KevinRansom]

Subject to you dealing with:

   let mutable table = mtyp.ActivePatternElemRefLookupTable
    cacheOptRef &table (fun () ->

[cartermp]

@KevinRansom Thanks, yeah it's been kinda fun digging through what are surely some of the older parts of the codebase. The isFirst function is my personal favorite.

[cartermp]

I will incorporate parts of #8062 into this

[forki]

Great work

[cartermp]

Just need to figure out why some parts of absil are so touchy :)