-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow mutating structs through read-only property setters in non-mutable contexts #1410
Comments
Is there a real-life API which is widely used and requires this pattern? Design like that does not fit F# programming, so I would be after interop reasons that need it. |
@T-Gro It is true that mutation does not fit F# programming, but this is about correctness. In situations where you have a type like This situation applies when you have an existing mutable container, for example: open System
type ListSpan<'T>(list: Collections.Generic.IList<'T>, start: int, length: int) =
member this.Item
with get (index) = list[if index >= length then -1 else start + index]
and set index value = list[if index >= length then -1 else start + index] <- value
let list = Collections.Generic.List<int>()
list.Add(1)
list.Add(2)
list.Add(3)
list.Add(4)
let view = ListSpan<_>(list, 1, 2)
view[0] <- 2
printf "%d" list[1] // 2 You would use a type like Now, why does F# allow mutation here in the first place, even without But what if you want to use a value type for performance? You add // error FS0256: A value must be mutable in order to mutate the contents or take the address of a value type, e.g. 'let mutable x = ...'
view[0] <- 2 But you are not mutating the contents of view! Adding Of course F# cannot assess whether an arbitrary property mutates or not; it has to assume the worst and it is only logical to assume that a property mutates the contents of the value. However, it can do that if the type (or property) has |
And there are existing APIs where structs are not used even though they could. One is even in the docs: open System.Collections.Generic
/// Basic implementation of a sparse matrix based on a dictionary
type SparseMatrix() =
let table = new Dictionary<(int * int), float>()
member _.Item
// Because the key is comprised of two values, 'get' has two index values
with get(key1, key2) = table[(key1, key2)]
// 'set' has two index values and a new value to place in the key's position
and set (key1, key2) value = table[(key1, key2)] <- value
let sm = new SparseMatrix()
for i in 1..1000 do
sm[i, i] <- float i * float i This API seems reasonable to me, but it cannot be used conveniently as a It should also be noted that C# already allows this syntax: public readonly struct ListMember<T>(IList<T> list, int index)
{
public T Value {
get => list[index];
set => list[index] = Value;
}
}
public static ListMember<int> GetMember() => default;
GetMember().Value = 20; // ok but removing readonly makes it an error F# is simply over-restrictive here without any apparent reason (except for historical). |
Also linking to the original C# discussion that made the above code work, and the follow-up: dotnet/csharplang#2068 dotnet/roslyn#45284 |
Could you provide the example for the listspan where you actually complete the example? it's not immediately obvious to me how your ListSpan breaks with a struct without an example. |
@voronoipotato Sorry ‒ the change is just adding open System
[<Struct; IsReadOnly>]
type ListSpan<'T>(list: Collections.Generic.IList<'T>, start: int, length: int) =
member this.Item
with get (index) = list[if index >= length then -1 else start + index]
and set index value = list[if index >= length then -1 else start + index] <- value
let list = Collections.Generic.List<int>()
list.Add(1)
list.Add(2)
list.Add(3)
list.Add(4)
let view = ListSpan<_>(list, 1, 2)
view[0] <- 2 // error My point there is that Compare this with spans: do
let span = [| 1 |].AsSpan()
span[0] <- 2 // completely fine |
Interesting, thanks |
I propose we allow the mutation (via
<-
) of structs through properties (and indexers) that are[<IsReadOnly>]
even in situations when the location is notmutable
(or the struct is the result of an expression). This parallels dotnet/csharplang#8364 in the reasoning and usage.Currently, this code is prohibited:
The reason F# even prohibits that operation in the first place has to do with avoiding errors in code that might look like it is mutating a value, but it is working on a copy of it and thus the mutation is not sensible.
However, in this situation, the struct is
IsReadOnly
so its contents cannot be mutated. This means that the setter, if sensibly defined, must operate on a value that is detached from the struct's contents, and thus any arbitrary copy of the struct is as good to be "mutated" through this property as the original.Therefore, if the struct is mutated through an
IsReadOnly
property, or the struct itself isIsReadOnly
, both of these operations should be allowed:In other words, for such properties, the fact that the type is a struct should be irrelevant.
Also compare equivalent C# code that is valid:
The existing way of approaching this problem in F# is to define a method instead of a property, which hides the mutation from the language.
Pros and Cons
The advantages of making this adjustment to F# are better syntax for utilizing "proxy structs" (slim values that point into other locations), like
ArraySegment
, or theListMember
above, all of which are conceptuallyIsReadOnly
, as well as parity with properties/indexers that returnbyref
values (such as the one onSpan
).The disadvantages of making this adjustment to F# are adding an exception to the mutability rules based on custom attributes.
Extra information
Estimated cost (XS, S, M, L, XL, XXL): S
Related suggestions: C# ‒ dotnet/csharplang#2068 dotnet/roslyn#45284
Affidavit (please submit!)
This is not a question (e.g. like one you might ask on StackOverflow) and I have searched StackOverflow for discussions of this issue
This is a language change and not purely a tooling change (e.g. compiler bug, editor support, warning/error messages, new warning, non-breaking optimisation) belonging to the compiler and tooling repository
This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it
I have searched both open and closed suggestions on this site and believe this is not a duplicate
This is not a breaking change to the F# language design
I or my company would be willing to help implement and/or test this
For Readers
If you would like to see this issue implemented, please click the 👍 emoji on this issue. These counts are used to generally order the suggestions by engagement.
The text was updated successfully, but these errors were encountered: