Mutable `let` binding in recursive module error text is confusing

[auduchinok]

module rec M1
    module M2 =
        let mutable x = 123

Library18.fs(3, 21):
[FS0874] Only record fields and simple, non-recursive 'let' bindings may be marked mutable

[cartermp]

cc @isaacabraham

[isaacabraham]

I'm on it Where can I read up more on this error? What should it read instead?

[Lanayx]

Got this error today - it appeared that mutable variables can't be used in recursive modules, while ref cells can be used. It looks strange to me, since ref cells are kinda "deprecated". In case with recursive modules it seems to be the only way to go.

[cartermp]

It's not just let-bound values in modules. https://sharplab.io/#v2:DYLgZgzgNAJiDUAfYBTALgAjBgFASgwF4BYAKAwo1UwCcUBjDAWwFc0BDAI1WaIwAYylXHiA

I think this could just be as simple as updating the text to just say "recursive mutable let bindings are not allowed" or something like that.

The reason why ref cells work is that they're always going to box up into an object, whereas mutable values will not do that unless they're being captured. There's just a lot more flexibility when you're boxing into an object.

[Lanayx]

@cartermp the problem is that let binding is not recursive, it's just mutable :) In my case the module was made recursive because I'm using fable with the following logic: once popup is open - subscribe to close event in open handler, once popup is closed - subscribe to open event in close handler, so I had to use recursive module to define those function handlers, since they mutually reference each other. And the mutable variable defined in that module with no reference to those mutually recursive functions stopped working.
So, if the issue itself is not going to be fixed and we have to use ref cells, then at least the errors should be separated, one saying "mutable variables are not allowed in recursive modules", another one "recursive mutable let bindings are not allowed"

[cartermp]

I think we'd certainly take a simple fix to change the existing message. Splitting into two could be trickier based on how recursively-defined values are checked in the typechecker. Would you be interested in submitting a patch? Even if it were just updating the existing text it would be clearer than today.

[Lanayx]

@cartermp I can try, it looks like there are just 2 files to update :) How about "Mutable 'let' bindings can neither be recursive nor defined in recursive module" ?

[cartermp]

That works!

[cartermp]

Fixed by #9813