-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add @[Extern]
attribute
#3051
Add @[Extern]
attribute
#3051
Conversation
This sounds very nice and flexible to me. But a problem would be chained assignment, which works with lib-structs if I remember correctly but is impossible to implement with normal structs. |
160ec60
to
8c5d39c
Compare
Tried this out in a project, works well. |
@BlaXpirit Nice! Care to show some code? |
@asterite https://github.com/BlaXpirit/crystal-chipmunk/blob/master/src/chipmunk/structs.cr Hm, I realized that this code probably only uses the simplest |
I've continued work on my library (using this branch), and everything runs without a slightest problem, even in complex configurations. I hope this will be merged. |
I think it's a good addition to the language. We could have Crystal structs mapping C structs, with nice methods for example. Or cast Slices to a Crystal structs (kinda unsafe but fast). It's down level, but in a good way: more control of the memory layout, and nicer interaction with external libraries. |
There seems to be a problem with named arguments to @[Extern]
struct A
def initialize(x : Int32)
@p = x
end
end
A.new(x: 6)
|
@asterite, what's the status on this? Will this ever be merged? |
@BlaXpirit this needs more thought. Right now all C types live under |
These are not "lib types outside lib", these are just structs that can't be extended. I agree that the currently taken approach to I wrote a comment in the issue. |
Fixes #2422
This PR works but I want to discuss this a bit before merging this.
Right now
lib
structs, unions and funs are restricted to either primitive types (Int32, Float32, etc.), pointers, enums, and structs and unions inside lib types. You can't use a "regular" crystal class or struct.With this PR one can now mark a crystal struct (defined outside a
lib
declaration) as@[Extern]
, and then the compiler lets you use this type in the above places.lib
structs and unions automatically receive getters and setters for each field, and they get an empty constructor that sets the type to zero (memzero):In contrast, and this is what I'd like to discuss, a regular struct marked with
@[Extern]
follows crystal rules, and the compiler will complain if declaring an instance variable and not properly initializing it. However, one can use@x = uninitialized ...
to mark that variable as "unsafe" and not needing a check from the compiler.Alternatively, we could automatically generate getters and setters for every instance variable, but I feel this is less safe and nice. One disadvantage of not doing this is that for C struct/union getters the compiler will invoke
to_unsafe
to do numeric conversions, but without these getters one would have to do these conversions.But, I think that this extern structs will probably be used with types that are created by
lib
functions, and you rarely need to set their instance variables, but instead you query them (or maybe you use them but don't let users query them). And in any case you can define setters and do a manual conversion there, if needed.One can also mark a struct with
@[Extern(union: true)]
, and now the struct will behave as a C union. For example:With this new
@[Extern]
attribute writing C bindings that have lots of structs and unions is greatly simplified, as one doesn't need to wrap these in a Crystal struct. There is also no need to define ato_unsafe
because the type is already the type that's passed directly to C. I checked if this could be applied to, for example, the LLVM bindings and yes, and manyLLVM:Value.new(LibLLVM.get_some_value)
are replaced with justLibLLVM.get_some_value
. But in this case the gain is very little. I think libraries such as SDL or SFML will benefit more from this because they expose structs that can be accessed directly, not only via an opaque pointer.One missing thing mentioned in #2422 is that, in order to keep binary compatibility with the C library, one shouldn't be able to add more instance variables to such structs. We could trust the user not to do so, or simply allow instance variable declarations on the first definition of the type, disallowing adding more instance variables later. For now we can start with the first option and eventually implement the second for a more robust solution.
Thoughts?