search

ffi / ffi-compiler

Apache License 2.0
32 stars 10 forks source link

Improve "Fake FFI" to support more FFI features #8

Closed davispuh closed 10 years ago

davispuh commented 10 years ago

Mainly added support for callbacks and also added few other methods. Still for full support would need a lot more improvement.

davispuh commented 10 years ago

Is this project abandoned?

davispuh commented 10 years ago

@luislavena, @frahugo what's happening with this? ffi-compiler is great and works fine in most cases, but lacks a few features. I've 3 PRs here, but no one have looked at them yet for 3 months and that's quite a long time. A lot of gems depend on ffi-compiler and I like it very much as Makefile replacement when compiling libraries for use with FFI.

luislavena commented 10 years ago

Hello @davispuh I'm not a maintainer or committer of this project.

Wayne Meissner was, but he is no longer active on GitHub.

Perhaps @enebo and @headius can take a look to this?

davispuh commented 10 years ago

I don't know who are maintainers.

This is all what's listed as contributors in GitHub screenshot

luislavena commented 10 years ago

See the list of commits:

https://github.com/ffi/ffi-compiler/commits/master

None of the contributors there have a big number of commits compared to the total of the repository.

I assume because Wayne removed his account is that he is no longer listed there.

enebo commented 10 years ago

Sorry, I do have rights for to commit for this project. In this particular project my domain knowledge is near zero but I can comment on the Ruby in the commit.

I am willing to commit it based on it appearing to be something you understand but I would like to see Struct not extend Hash. Generally, extending core types leads to cross-impl weirdness. I also wonder if struct really should have all the methods of Hash added to it? Does that seem like something you can reasonably change?

davispuh commented 10 years ago

Actually it's FFI::Struct (from ffi/struct.rb#101) and not Ruby's as that indeed would be wrong. Adding hash was just the most quickest way. Basically it's needed only for [] and []= methods. So anyway I removed hash and added these methods. I also updated few other things such as added more types from FFI.

enebo commented 10 years ago

Yeah I knew it was not a Ruby struct since that would not be possible to extend from Hash. I see you added [] and []= but I don't see an implementation of those methods?

davispuh commented 10 years ago

It's not needed. Fake FFI is used only to replace FFI when generating C header file from Ruby FFI definitions and all required information are already taken from layout (for example), this is needed only so we don't get error when some source file calls it.

I just added quick implementation, it shouldn't make any difference, but we might avoid some side-effects in case some module tries to read previously assigned value.

enebo commented 10 years ago

Cool thanks for updating that. If you think it should not store values then you could also maybe add a comment specifying it is intended to be empty (I guess the Fake implies it though).

davispuh commented 10 years ago

It just doesn't really matter what it does as long as it's callable and like I said actual implementation might be better than always returning nil.

BTW there's also PR #11