Skip to content
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

[WIP] Trait and Macro for exporting C API for identifiers #647

Closed
wants to merge 7 commits into from
Closed

[WIP] Trait and Macro for exporting C API for identifiers #647

wants to merge 7 commits into from

Conversation

twitu
Copy link
Collaborator

@twitu twitu commented May 17, 2022

Pull Request

Macro to implement C API for identifiers

Type of change

  • [ x ] Refactor

How has this change been tested?

This is what the macro generates. Requesting feedback before applying this across other identifiers.

    impl InterconvertPyString for Venue {
        #[export_name = "Venue_from_pystr"]
        unsafe extern "C" fn from_pystr(ptr: *mut ffi::PyObject) -> Self {
            Venue {
                value: Box::new(pystr_to_string(ptr)),
            }
        }
        #[export_name = "Venue_to_pystr"]
        unsafe extern "C" fn to_pystr(&self) -> *mut ffi::PyObject {
            string_to_pystr(self.value.as_str())
        }
    }

Not tested currently

@codecov
Copy link

codecov bot commented May 17, 2022

Codecov Report

Merging #647 (b7841b7) into develop (b332e67) will increase coverage by 0.00%.
The diff coverage is n/a.

❗ Current head b7841b7 differs from pull request most recent head e6d6912. Consider uploading reports for the commit e6d6912 to get more accurate results

@@           Coverage Diff            @@
##           develop     #647   +/-   ##
========================================
  Coverage    90.99%   90.99%           
========================================
  Files          298      298           
  Lines        25190    25198    +8     
========================================
+ Hits         22922    22930    +8     
  Misses        2268     2268           
Impacted Files Coverage Δ
nautilus_trader/model/data/tick.pyx 98.31% <0.00%> (+0.12%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b332e67...e6d6912. Read the comment docs.

@twitu
Copy link
Collaborator Author

twitu commented May 18, 2022

I've also added a DeRef trait which allows Venue to be referenced as if if were it's inner string content itself. It's a useful idiom when operating on the Rust side of things.

@cjdsellers cjdsellers changed the title Trait and Macro for exporting C API for identifiers [WIP] Trait and Macro for exporting C API for identifiers May 18, 2022
@twitu
Copy link
Collaborator Author

twitu commented May 19, 2022

A couple of factors combine to make this impossible.

  1. cbindgen does not generate bindings for functions defined inside traits.
  2. this means that we need unique names for each ffi functions for e.g. uuid4_to_pystr
  3. Currently it is not possible to generate identifier names from passed tokens inside a macro

This pretty much kills any chance of generating extern functions generically from macros.

SO answer
rust-lang/rust#12249
rust-lang/rust#29599

@twitu twitu closed this May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant