-
Notifications
You must be signed in to change notification settings - Fork 173
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
Sum types are not instantiable using C interface #2074
Comments
I propose that the C interface for the above type as an example looks something like this (words "futhark" and "opaque" skipped for brevity:
The manifest would list in a predefined order the names and arguments of each variant - not sure where is appropriate. |
This is clearly something that would be very useful, particularly for bridges that can then provide a completely ergonomic interface on top. The proposed interface matches the corresponding one for records. I'm not sure about the |
I was thinking of using getvariant when there's N variants, to optimize from having to call N functions to calling getvariant and knowing immediately which code path to take. |
The number wouldn't tell you which of the projection functions would work. |
Why not? Each projection function is based on the variant, not the type you intend to get out, so the number describing which variant is there would automatically point to the valid function. |
How does the number indicate which function works? |
Suppose we have a type (#none | #some u16 | #foo []u8). It's described in the manifest as having 3 variants in the following order: The |
So the manifest needs to store the constructors as an ordered list, rather than as a dictionary. |
Correct. |
Oh, now I remember why this is a bit tricky to implement: we deduplicate constructor payloads. That does not make this facility impossible, but we have to be quite careful about preserving how the logical representation maps to the physical one. |
See also #1960. |
I don't think the variant function can possibly fail, so I'm inclining towards this type for simplicity:
|
While destruction (what is called "projection" above) is straightforward to implement, construction turns out to have a complicated wrinkle. The reason is that when we have a type such as type sum [n][m] = #foo ([n]i32) | #bar ([m]i32) then even when we construct just the We can't just default unknown sizes to 0, because it is possible a value of this sum type may be passed to an entry point that requires e.g. This is ultimately due to our value representation, which also physically represents even those variants that are not in active use. This representation is a tradeoff, and cannot/should-not be changed. We have some options.
|
For what it's worth, I don't think this problem is avoidable, unless you hide possible members behind a pointer. Otherwise, space big enough for the largest member needs to be allocted, not just the instantiated one. |
We have full control over the representation, so we can hide whatever we want. Currently sum types do not have any special representation, however. I think I will add the API discussed above, as it is the simplest and most elegant thing. Destruction works and is completely reliable, but construction results in types with size 0 for arrays in the the unused variants. This will not result in memory-unsafety, but may lead to controlled errors when they are passed to entry points that are picky about those sizes. This is certainly not great, but it can be worked around by the programmer (by making the entry points less picky), and maybe we can make it better in the future, without changing the API. |
The manual doesn't list them at all in https://futhark.readthedocs.io/en/stable/c-api.html#opaque-values , but for an argument
(f: (#none | #some u16))
the header file contains some procedures:"types":{"(#none | #some u16)":{"ctype":"struct futhark_opaque_d8e... *","kind":"opaque","ops":{"free":"futhark_free_opaque_d8e..","restore":"futhark_restore_opaque_d8e...","store":"futhark_store_opaque_d8e..."}}
This is not enough to create or read values of this type, though.
The text was updated successfully, but these errors were encountered: