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.

Sign up for GitHub

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

ocaml-interop integration #48

Merged
merged 28 commits into from
Feb 26, 2021
Merged

ocaml-interop integration #48

Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
7b77771
Use ocaml-interop for low-level operations
zshipko Dec 17, 2020
077e0a5
WIP
zshipko Dec 18, 2020
1678907
Add ability to rename gc handle
zshipko Dec 18, 2020
3e4d49c
Mark Value methods as unsafe
zshipko Dec 18, 2020
e52a231
Remove ocaml-sys macros
zshipko Dec 18, 2020
8d59b1e
WIP
zshipko Dec 18, 2020
05157f3
Remove Rooted
zshipko Dec 18, 2020
8b5c9e8
Create rooted values from parameters in ocaml::body
zshipko Dec 18, 2020
468f57f
Use ocaml-interop git repo
zshipko Dec 18, 2020
6b44c41
clippy
zshipko Dec 18, 2020
31c53ab
ci
zshipko Dec 19, 2020
692e411
clippy
zshipko Dec 19, 2020
2d25bc0
Fix attr
zshipko Dec 19, 2020
aadc97d
Remove call to Gc.minor to ensure allocation issues have been fixed
zshipko Dec 19, 2020
4f81aed
cleanup
zshipko Dec 19, 2020
caff018
Start adding more OCaml values in tests
zshipko Dec 19, 2020
bc69178
Updates for latest ocaml-interop
zshipko Jan 14, 2021
d3a8d68
as_i64 -> to_i64
zshipko Jan 14, 2021
b750c46
Switch to ocaml-interop release
zshipko Jan 20, 2021
ac59e68
ToValue -> IntoValue
zshipko Jan 20, 2021
548276c
Use immutable ref for runtime handle in into_value
zshipko Feb 18, 2021
4fc48dd
Versions, cleanup derive
zshipko Feb 18, 2021
56d55c0
clippy
zshipko Feb 18, 2021
d8f7af4
Calls to into_value should not be marked as unsafe
zshipko Feb 18, 2021
a538444
Mark Value::interop as unsafe and remove unnecessary transmute
zshipko Feb 18, 2021
dcf0330
Fix test
zshipko Feb 18, 2021
802a9f9
Add a note about ocaml-interop, use interop::ocaml to call OCaml func…
zshipko Feb 26, 2021
e9fb849
Fix clippy
zshipko Feb 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/clippy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ jobs:
env:
OCAML_VERSION: 4.09.1
OCAML_WHERE_PATH: /ignored
run: cargo +nightly clippy --all -- -D warnings
run: cargo +nightly clippy --features=without-ocamlopt --all -- -D warnings
9 changes: 5 additions & 4 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "ocaml"
version = "0.19.1"
version = "0.20.0"
authors = ["Zach Shipko <zachshipko@gmail.com>"]
readme = "README.md"
keywords = ["ocaml", "rust", "ffi"]
Expand All @@ -14,16 +14,17 @@ edition = "2018"
features = [ "without-ocamlopt", "derive", "link" ]

[dependencies]
ocaml-sys = {path = "./sys", version = "0.19"}
ocaml-derive = {path = "./derive", optional = true, version = "0.19"}
ocaml-interop = "0.5"
ocaml-sys = {path = "./sys", version = "0.20"}
ocaml-derive = {path = "./derive", optional = true, version = "0.20"}
cstr_core = {version = "0.2", optional = true}
ndarray = {version = "^0.14.0", optional = true}

[features]
default = ["derive"]
derive = ["ocaml-derive"]
link = ["ocaml-sys/link"]
without-ocamlopt = ["ocaml-sys/without-ocamlopt"]
without-ocamlopt = ["ocaml-sys/without-ocamlopt", "ocaml-interop/without-ocamlopt"]
caml-state = ["ocaml-sys/caml-state"]
no-std = ["cstr_core/alloc"]
bigarray-ext = ["ndarray"]
Expand Down
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ pub fn incr(value: ocaml::Value) -> ocaml::Value {
// This is equivalent to:
#[no_mangle]
pub extern "C" fn incr2(value: ocaml::Value) -> ocaml::Value {
ocaml::body!((value) {
ocaml::body!(gc: (value) {
let i = value.int_val();
ocaml::Value::int( i + 1)
})
Expand Down
2 changes: 1 addition & 1 deletion derive/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "ocaml-derive"
version = "0.19.0"
version = "0.20.0"
authors = ["Zach Shipko <zachshipko@gmail.com>"]
edition = "2018"
license = "ISC"
Expand Down
48 changes: 24 additions & 24 deletions derive/src/derive.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ fn variant_attrs(attrs: &[syn::Attribute]) -> Attrs {
})
}

pub fn tovalue_derive(mut s: synstructure::Structure) -> proc_macro::TokenStream {
pub fn intovalue_derive(mut s: synstructure::Structure) -> proc_macro::TokenStream {
Copy link
Contributor

@tizoc tizoc Feb 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zshipko question, why did you go from to -> into? (I'm curious about this because I went the opposite direction in ocaml-interop, by making into_rust -> to_rust because I think that in rust the into assumes that self is consumed.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah this is something I probably want to change back. In this case the trait is actually consuming self, but I don't think that is really the best idea here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the thing is that Value is Copy right? I think I had it like that for the same reason, but since the original value is still available I decided to switch to the to_ prefix. I am not really sure about what the convention is in cases like this, but my reasoning is that (in addition to what I said on the previous sentence) since Value is more like a reference than the actual value to_ made more sense.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I remember that IntoValue consumes self to make Custom values better integrated. It seems like consuming self is actually kind of nice because it prevents the value from being accessed after being converted to OCaml.

I will need to think about the various trade-offs a little more...

Copy link
Owner Author

@zshipko zshipko Feb 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the thing is that Value is Copy right? I think I had it like that for the same reason, but since the original value is still available I decided to switch to the to_ prefix. I am not really sure about what the convention is in cases like this, but my reasoning is that (in addition to what I said on the previous sentence) since Value is more like a reference than the actual value to_ made more sense.

The main impetus behind the name change was that clippy was complaining about it being named To* and not taking a reference. But I think your explanation makes more sense.

let mut unit_tag = 0u8;
let mut non_unit_tag = 0u8;
let is_record_like = s.variants().len() == 1;
Expand All @@ -74,49 +74,47 @@ pub fn tovalue_derive(mut s: synstructure::Structure) -> proc_macro::TokenStream
panic!("ocaml cannot derive unboxed or float arrays for enums")
}
if arity == 0 {
let init = quote!(value = ocaml::Value::int(#tag as ocaml::Int));
let init = quote!(value = unsafe { ocaml::Value::int(#tag as ocaml::Int)});
variant.fold(init, |_, _| quote!())
} else if attrs.floats {
let mut idx = 0usize;
let init = quote!(
value = ocaml::Value::alloc(#arity, ocaml::Tag::DOUBLE_ARRAY);
value = unsafe { ocaml::Value::alloc(gc, #arity, ocaml::Tag::DOUBLE_ARRAY) };
);
variant.fold(init, |acc, b| {
let i = idx;
idx += 1;
quote!(#acc; ocaml::array::set_double(value, #i, *#b as f64).unwrap();)
quote!(#acc; ocaml::array::set_double(gc, value, #i, *#b as f64).unwrap();)
})
} else if attrs.unboxed {
if variant.bindings().len() > 1 {
panic!("ocaml cannot unboxed record with multiple fields")
}
variant.each(|field| quote!(#field.to_value()))
variant.each(|field| quote!(#field.into_value(gc)))
} else {
let mut idx = 0usize;
let ghost = (0..arity).map(|idx| quote!(value.store_field(#idx, ocaml::Value::unit())));
let ghost = (0..arity)
.map(|idx| quote!(unsafe { value.store_field(gc, #idx, ocaml::Value::unit()) }));
let init = quote!(
value = ocaml::Value::alloc(#arity, ocaml::Tag(#tag));
value = unsafe { ocaml::Value::alloc(gc, #arity, ocaml::Tag(#tag)) };
#(#ghost);*;
);
variant.fold(init, |acc, b| {
let i = idx;
idx += 1;
quote!(#acc value.store_field(#i, #b.to_value());)
quote!(#acc unsafe { value.store_field(gc, #i, #b)};)
})
}
});

s.gen_impl(quote! {
gen unsafe impl ocaml::ToValue for @Self {
fn to_value(self) -> ocaml::Value {
unsafe {
ocaml::frame!((value) {
match self {
#(#body),*
}
value
})
gen unsafe impl ocaml::IntoValue for @Self {
fn into_value(self, gc: &ocaml::Runtime) -> ocaml::Value {
let mut value = ocaml::Value::unit();
match self {
#(#body),*
}
value
}
}
})
Expand Down Expand Up @@ -177,10 +175,10 @@ pub fn fromvalue_derive(s: synstructure::Structure) -> proc_macro::TokenStream {
.into()
} else {
let tag = if !attrs.floats {
quote!({ value.tag() })
quote!({ unsafe { value.tag() } })
} else {
quote!({
if value.tag() != ocaml::Tag::DOUBLE_ARRAY {
if unsafe { value.tag() } != ocaml::Tag::DOUBLE_ARRAY {
panic!("ocaml ffi: trying to convert a value which is not a double array to an unboxed record")
};
0
Expand All @@ -189,11 +187,13 @@ pub fn fromvalue_derive(s: synstructure::Structure) -> proc_macro::TokenStream {
s.gen_impl(quote! {
gen unsafe impl ocaml::FromValue for @Self {
fn from_value(value: ocaml::Value) -> Self {
let is_block = value.is_block();
let tag = if !is_block { value.int_val() as u8 } else { #tag.0 };
match (is_block, tag) {
#(#body),*
_ => panic!("ocaml ffi: received unknown variant while trying to convert ocaml structure/enum to rust"),
unsafe {
let is_block = value.is_block();
let tag = if !is_block { value.int_val() as u8 } else { #tag.0 };
match (is_block, tag) {
#(#body),*
_ => panic!("ocaml ffi: received unknown variant while trying to convert ocaml structure/enum to rust"),
}
}
}
}
Expand Down
Loading