fsharp icon indicating copy to clipboard operation
fsharp copied to clipboard

Mutable use binding compiles

Open cartermp opened this issue 7 years ago • 11 comments

open System.Drawing

let f() =
    use mutable font = new Font("Arial", 10.0f)
    font <- new Font("Arial", 11.0f)

f()

The above code compiles. Note that in C# it is not:

screen shot 2018-07-14 at 11 46 15

Using latest Mono and FCS from Ionide

cartermp avatar Jul 14 '18 18:07 cartermp

Also, I returned out the font here:

open System.Drawing

let f() =
    use mutable font = new Font("Arial", 10.0f)
    font <- new Font("Arial", 11.0f)
    font

f()

Which hangs FSI indefinitely when the above code is executed.

If I do not make it mutable, it executes:

open System.Drawing

let f() =
    use font = new Font("Arial", 10.0f)
    //font <- new Font("Arial", 11.0f)
    font

f()

Result:

val f : unit -> Font
val it : Font =
  [Font: Name=Arial, Size=10, Units=3, GdiCharSet=1, GdiVerticalFont=False]

So we definitely allow code that cannot execute correctly.

cartermp avatar Jul 14 '18 18:07 cartermp

It's definitely should not compile, but it's a breaking change.

vasily-kirichenko avatar Jul 14 '18 19:07 vasily-kirichenko

On Windows this doesn't seem to "hang" and the "semantics" of use mutable seem to be: "Call dispose on the last reference". So I'd assume this "hang" to be a mono bug.

matthid avatar Jul 14 '18 20:07 matthid

Yep, looks like the hang is a mono issue, so adding a restriction on this would have to be a warning.

cartermp avatar Jul 14 '18 23:07 cartermp

Actually, the right behaviour would be to call Dispose on the previously bind object just before assigning a new one (ideally, atomically), that's what Rust does:

#[derive(Debug)]
struct Foo(String);

impl Drop for Foo {
    fn drop(&mut self) {
        println!("Dropping {:?}", self)
    }
}

fn main() {
    {
        let mut foo = Foo("1".to_string());
        foo = Foo("2".to_string());
        foo = Foo("3".to_string());
    }
    println!("Done")
}
Dropping Foo("1")
Dropping Foo("2")
Dropping Foo("3")
Done

vasily-kirichenko avatar Jul 15 '18 06:07 vasily-kirichenko

I'm with Vasily here but this would be a new language suggestion

realvictorprm avatar Jul 15 '18 07:07 realvictorprm

Interestingly in the language spec 6.6.4 ‘use mutable’ doesn’t exist. So removing this thing would not be a breaking change in the spec...

matthid avatar Jul 15 '18 08:07 matthid

would be a new language suggestion

Please don't let us assign any meaning to this abomination, let's introduce a compiler error (if allowed due to spec) or introduce a warning.

matthid avatar Jul 15 '18 08:07 matthid

Seems like the compiler accepts pretty much everything with use it would with let:

module M =
    use internal d<'T> = { new System.IDisposable with member __.Dispose() = printfn "disposing 1, %O" typeof<'T>.Name }
    use rec d1 = { new System.IDisposable with member __.Dispose() = printfn "disposing 1" }
    and d2 = { new System.IDisposable with member __.Dispose() = printfn "disposing 2" }

At least we get:

stdin(82,5): warning FS0524: 'use' bindings are not permitted in modules and are treated as 'let' bindings
stdin(83,5): warning FS0524: 'use' bindings are not permitted in modules and are treated as 'let' bindings

But in principle the compiler seems to allow it (which is against the spec)

matthid avatar Jul 15 '18 08:07 matthid

I'm not sure what to think here. Any change to this would be a breaking change. This is somewhat similar to my proposal on disabling let mutable x : byref<int> suggestion.

TIHan avatar Jul 19 '18 23:07 TIHan

Agreed on this turning into a warning.

cartermp avatar Jul 20 '18 02:07 cartermp