Add basic parallel indexing logic#36
Conversation
| [enum] | ||
| rename_variants = "ScreamingSnakeCase" |
There was a problem hiding this comment.
Enums in C are usually all in caps, so I added this to make sure cbindgen will transform them for us.
| #include <string.h> | ||
| #include "repository.h" | ||
|
|
||
| // Store the class references |
There was a problem hiding this comment.
C extensions are usually organized the same way as CRuby: each file contains everything related to a specific class/module and the file name is the same as the class/module name.
I moved everything to repository.c.
| # Represents the base declaration class for all indexed items. This class is partially defined in C to connect with | ||
| # the Rust layer | ||
| class Declaration |
There was a problem hiding this comment.
Not strictly necessary, but a good example of how we can mix C and Ruby for defining the API. In reality, we only need C to access the Rust side of things. Simple things that aren't performance critical can be defined in Ruby.
|
@vinistock For ruby/prism#3594, how are we able to verify that the change successfully fixes the issue for Windows? |
|
We can point to the GitHub repo and give it a try. |
| } | ||
| } | ||
|
|
||
| pub fn index(&mut self, file_path: String) { |
There was a problem hiding this comment.
I wonder if we should return a Result<(), std::io::Error> in case we can't read the file?
|
|
||
| void* repository; | ||
| TypedData_Get_Struct(self, void*, &repository_type, repository); | ||
| idx_index_all_c(repository, converted_file_paths, length); |
There was a problem hiding this comment.
If the indexing panics, is there a change we're leaking converted_file_paths?
There was a problem hiding this comment.
Maybe we should also raise an exception back to Ruby is the indexing failed? For example if we can't open one of the file in the given paths?
| Box::into_raw(Box::new(repository)) as *mut CRepository | ||
| /// Converts an array of C strings (const **char) to a Vec<String> | ||
| unsafe fn convert_double_pointer_to_vec(data: &mut &mut c_char, len: size_t) -> Result<Vec<String>, Utf8Error> { | ||
| unsafe { |
There was a problem hiding this comment.
Do we need the unsafe block if the function itself is already marked unsafe?
|
Closing in favour of #54 |
3 participants
Add example Rust to Ruby VM bindings to the repository, so that people can use this while working on their prototypes. The keyword here is example.
This PR demonstrates how we can share an
ArcofRepositorywith a Ruby object and how we can return a boxed value to produce a new RubyDeclarationobject. That doesn't mean we'll actually need this exactly as is. Until we have settled on the data structures, we have no way of knowing exactly what we'll need. This is useful nonetheless so that people can test their prototypes with Ruby scripts and see if they actually do what they want.The PR includes:
I didn't stress over adding a bunch of tests since this code is a temporary example and will eventually be substituted by the real implementation.
The most important part of this PR, and what I would love for people to focus on, is the documentation of the
c_interfacefile. Do you agree with everything in there? Is it clear enough for you to feel empowered to add more bindings to the Ruby VM? How can we ensure that the details are as easy to follow as possible?Unfortunate note
Windows CI will be broken until ruby/prism#3594 is merged (provided it actually fixes the issue, which I'm only between 60-70% sure).