From a3932401235c824e94804b16b28ca4e7f0e1ca49 Mon Sep 17 00:00:00 2001 From: Jose Narvaez Date: Mon, 16 Dec 2024 20:39:51 +0000 Subject: [PATCH 1/6] Trying to implement rstruct_len by hand. --- .../rb-sys-build/src/bindings/stable_api.rs | 2 +- crates/rb-sys-build/src/bindings/wrapper.h | 16 +++++++ crates/rb-sys-tests/src/stable_api_test.rs | 11 +++++ crates/rb-sys/src/stable_api.rs | 31 +++++++++++++ crates/rb-sys/src/stable_api/compiled.c | 20 ++++++++ crates/rb-sys/src/stable_api/compiled.rs | 39 ++++++++++++++++ crates/rb-sys/src/stable_api/ruby_3_3.rs | 46 ++++++++++++++++++- 7 files changed, 163 insertions(+), 2 deletions(-) diff --git a/crates/rb-sys-build/src/bindings/stable_api.rs b/crates/rb-sys-build/src/bindings/stable_api.rs index 479da284..130cefac 100644 --- a/crates/rb-sys-build/src/bindings/stable_api.rs +++ b/crates/rb-sys-build/src/bindings/stable_api.rs @@ -4,7 +4,7 @@ use quote::ToTokens; use crate::RbConfig; -const OPAQUE_STRUCTS: [&str; 2] = ["RString", "RArray"]; +const OPAQUE_STRUCTS: [&str; 3] = ["RString", "RArray", "RStruct"]; const OPAQUE_STRUCTS_RUBY_3_3: [&str; 3] = [ "rb_matchext_struct", diff --git a/crates/rb-sys-build/src/bindings/wrapper.h b/crates/rb-sys-build/src/bindings/wrapper.h index 43814a9a..11fd7308 100644 --- a/crates/rb-sys-build/src/bindings/wrapper.h +++ b/crates/rb-sys-build/src/bindings/wrapper.h @@ -69,3 +69,19 @@ #ifdef HAVE_RUBY_IO_BUFFER_H #include "ruby/io/buffer.h" #endif + +struct RStruct { + struct RBasic basic; + union { + struct { + long len; + const VALUE *ptr; + } heap; + /* This is a length 1 array because: + * 1. GCC has a bug that does not optimize C flexible array members + * (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102452) + * 2. Zero length arrays are not supported by all compilers + */ + const VALUE ary[1]; + } as; +}; diff --git a/crates/rb-sys-tests/src/stable_api_test.rs b/crates/rb-sys-tests/src/stable_api_test.rs index 8c3156ca..fccb55e8 100644 --- a/crates/rb-sys-tests/src/stable_api_test.rs +++ b/crates/rb-sys-tests/src/stable_api_test.rs @@ -1,3 +1,5 @@ +use std::ffi::{CStr, CString}; + use rb_sys::{StableApiDefinition, VALUE}; use rb_sys_test_helpers::rstring as gen_rstring; @@ -655,3 +657,12 @@ parity_test!( std::time::Duration::from_millis(100) } ); + +parity_test!( + name: test_rb_rstruct_len, + func: rstruct_len, + data_factory: { + ruby_eval!("Person = Struct.new(:name, :age); Person.new('Matz', 59)") + }, + expected: 2 +); diff --git a/crates/rb-sys/src/stable_api.rs b/crates/rb-sys/src/stable_api.rs index 10b34f9c..69167152 100644 --- a/crates/rb-sys/src/stable_api.rs +++ b/crates/rb-sys/src/stable_api.rs @@ -13,6 +13,7 @@ use crate::VALUE; use std::{ + ffi::{c_int, CStr}, os::raw::{c_char, c_long}, ptr::NonNull, time::Duration, @@ -175,6 +176,36 @@ pub trait StableApiDefinition { /// Blocks the current thread until the given duration has passed. fn thread_sleep(&self, duration: Duration); + + /// Defines a struct type with the given name and members. + /// + /// # Example + /// + fn rstruct_define(&self, name: &CStr, members: &[&CStr]) -> VALUE; + + /// Accesses an indexed member of the struct. + /// + /// # Safety + /// This function is unsafe because it dereferences a raw pointer to get + /// access to underlying struct data. The caller must ensure that the + /// `VALUE` is a valid pointer to a struct. + unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE; + + /// Sets an indexed member of the struct. + /// + /// # Safety + /// This function is unsafe because it dereferences a raw pointer to get + /// access to underlying struct data. The caller must ensure that the + /// `VALUE` is a valid pointer to a struct. + unsafe fn rstruct_set(&self, st: VALUE, idx: c_int, value: VALUE); + + /// Returns the number of struct members. + /// + /// # Safety + /// This function is unsafe because it dereferences a raw pointer to get + /// access to underlying struct data. The caller must ensure that the + /// `VALUE` is a valid pointer to an RStruct. + unsafe fn rstruct_len(&self, obj: VALUE) -> c_long; } #[cfg(stable_api_enable_compiled_mod)] diff --git a/crates/rb-sys/src/stable_api/compiled.c b/crates/rb-sys/src/stable_api/compiled.c index db89dc1f..5a416e5a 100644 --- a/crates/rb-sys/src/stable_api/compiled.c +++ b/crates/rb-sys/src/stable_api/compiled.c @@ -121,3 +121,23 @@ impl_thread_sleep(struct timeval time) { rb_thread_wait_for(time); } +long +impl_rstruct_len(VALUE st) { + return RSTRUCT_LEN(st); +} + +VALUE +impl_rstruct_define(char* name, ...) { + return rb_struct_define(name, NULL); +} + +VALUE +impl_rstruct_get(VALUE st, int idx) { + return Qnil; +} + +VALUE +impl_rstruct_set(VALUE st, int idx, VALUE value) { + return Qnil; +} + diff --git a/crates/rb-sys/src/stable_api/compiled.rs b/crates/rb-sys/src/stable_api/compiled.rs index 527fabd0..6ea2dd81 100644 --- a/crates/rb-sys/src/stable_api/compiled.rs +++ b/crates/rb-sys/src/stable_api/compiled.rs @@ -1,6 +1,7 @@ use super::StableApiDefinition; use crate::{ruby_value_type, timeval, VALUE}; use std::{ + ffi::{c_int, CStr}, os::raw::{c_char, c_long}, ptr::NonNull, time::Duration, @@ -79,6 +80,18 @@ extern "C" { #[link_name = "impl_thread_sleep"] fn impl_thread_sleep(interval: timeval); + + #[link_name = "impl_rstruct_define"] + fn impl_rstruct_define(name: &CStr, members: Vec<*const c_char>) -> VALUE; + + #[link_name = "impl_rstruct_get"] + fn impl_rstruct_get(st: VALUE, idx: c_int) -> VALUE; + + #[link_name = "impl_rstruct_set"] + fn impl_rstruct_set(st: VALUE, idx: c_int, value: VALUE); + + #[link_name = "impl_rstruct_len"] + fn impl_rstruct_len(obj: VALUE) -> c_long; } pub struct Definition; @@ -112,6 +125,7 @@ impl StableApiDefinition for Definition { NonNull::::new(impl_rbasic_class(obj) as _) } + #[inline] unsafe fn frozen_p(&self, obj: VALUE) -> bool { impl_frozen_p(obj) } @@ -213,4 +227,29 @@ impl StableApiDefinition for Definition { unsafe { impl_thread_sleep(time) } } + + #[inline] + fn rstruct_define(&self, name: &CStr, members: &[&CStr]) -> VALUE { + let mut members: Vec<*const c_char> = members + .iter() + .map(|m| m.as_ptr() as *const c_char) + .collect(); + members.push(std::ptr::null()); + unsafe { impl_rstruct_define(name, members) } + } + + #[inline] + unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { + impl_rstruct_get(st, idx) + } + + #[inline] + unsafe fn rstruct_set(&self, st: VALUE, idx: c_int, value: VALUE) { + impl_rstruct_set(st, idx, value) + } + + #[inline] + unsafe fn rstruct_len(&self, st: VALUE) -> c_long { + impl_rstruct_len(st) + } } diff --git a/crates/rb-sys/src/stable_api/ruby_3_3.rs b/crates/rb-sys/src/stable_api/ruby_3_3.rs index d068fac1..fe39fa2f 100644 --- a/crates/rb-sys/src/stable_api/ruby_3_3.rs +++ b/crates/rb-sys/src/stable_api/ruby_3_3.rs @@ -1,14 +1,25 @@ use super::StableApiDefinition; use crate::{ internal::{RArray, RString}, - value_type, VALUE, + ruby_fl_type, value_type, VALUE, }; use std::{ + ffi::{c_int, CStr}, os::raw::{c_char, c_long}, ptr::NonNull, time::Duration, }; +const RSTRUCT_EMBED_LEN_MASK: i64 = (ruby_fl_type::RUBY_FL_USER7 as i64) + | (ruby_fl_type::RUBY_FL_USER6 as i64) + | (ruby_fl_type::RUBY_FL_USER5 as i64) + | (ruby_fl_type::RUBY_FL_USER4 as i64) + | (ruby_fl_type::RUBY_FL_USER3 as i64) + | (ruby_fl_type::RUBY_FL_USER2 as i64) + | (ruby_fl_type::RUBY_FL_USER1 as i64); + +const RSTRUCT_EMBED_LEN_SHIFT: i64 = (crate::ruby_fl_ushift::RUBY_FL_USHIFT as i64) + 1; + #[cfg(not(ruby_eq_3_3))] compile_error!("This file should only be included in Ruby 3.3 builds"); @@ -268,4 +279,37 @@ impl StableApiDefinition for Definition { unsafe { crate::rb_thread_wait_for(time) } } + + fn rstruct_define(&self, name: &CStr, members: &[&CStr]) -> VALUE { + let mut members: Vec<*const c_char> = members + .iter() + .map(|m| m.as_ptr() as *const c_char) + .collect(); + members.push(std::ptr::null()); + unsafe { crate::rb_struct_define(name.as_ptr(), members) } + } + + unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { + crate::rb_struct_getmember(st, idx as _) + } + + unsafe fn rstruct_set(&self, st: VALUE, idx: c_int, value: VALUE) { + crate::rb_struct_aset(st, idx as _, value); + } + + // unsafe fn rstruct_len(&self, st: VALUE) -> c_long { + // crate::rb_num2long(crate::rb_struct_size(st)) + // } + + unsafe fn rstruct_len(&self, st: VALUE) -> c_long { + let rbasic = st as *const crate::RBasic; + if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + let mut ret = ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) as c_long; + ret >>= RSTRUCT_EMBED_LEN_SHIFT; + ret + } else { + let rstruct = st as *const crate::RStruct; + (*rstruct).as_.heap.len as c_long + } + } } From dbfc3dd373c60add1b95153c051095890ffdda9c Mon Sep 17 00:00:00 2001 From: Jose Narvaez Date: Thu, 19 Dec 2024 19:12:06 +0000 Subject: [PATCH 2/6] Manual RStruct bindings and implemented rstruct_len for all versions. --- crates/rb-sys-tests/src/stable_api_test.rs | 26 ++++++++- crates/rb-sys/src/stable_api/ruby_2_6.rs | 60 +++++++++++++++++++- crates/rb-sys/src/stable_api/ruby_2_7.rs | 60 +++++++++++++++++++- crates/rb-sys/src/stable_api/ruby_3_0.rs | 60 +++++++++++++++++++- crates/rb-sys/src/stable_api/ruby_3_1.rs | 59 ++++++++++++++++++- crates/rb-sys/src/stable_api/ruby_3_2.rs | 60 +++++++++++++++++++- crates/rb-sys/src/stable_api/ruby_3_3.rs | 50 +++++++++++----- crates/rb-sys/src/stable_api/ruby_3_4.rs | 66 +++++++++++++++++++++- 8 files changed, 417 insertions(+), 24 deletions(-) diff --git a/crates/rb-sys-tests/src/stable_api_test.rs b/crates/rb-sys-tests/src/stable_api_test.rs index fccb55e8..a68379e4 100644 --- a/crates/rb-sys-tests/src/stable_api_test.rs +++ b/crates/rb-sys-tests/src/stable_api_test.rs @@ -1,5 +1,3 @@ -use std::ffi::{CStr, CString}; - use rb_sys::{StableApiDefinition, VALUE}; use rb_sys_test_helpers::rstring as gen_rstring; @@ -659,10 +657,32 @@ parity_test!( ); parity_test!( - name: test_rb_rstruct_len, + name: test_rb_rstruct_len_embedded, func: rstruct_len, data_factory: { ruby_eval!("Person = Struct.new(:name, :age); Person.new('Matz', 59)") }, expected: 2 ); + +parity_test!( + name: test_rb_rstruct_len_heap, + func: rstruct_len, + data_factory: { + ruby_eval!(" + field_names = (0..80).map { |i| \"field#{i}\" }; + LargeStruct = Struct.new(*field_names.map(&:to_sym)) do + def initialize(*) + super + # Initialize all nil fields with 0 for demonstration + members.each do |field| + self[field] = 0 if self[field].nil? + end + end + end; + + LargeStruct.new + ") + }, + expected: 81 +); diff --git a/crates/rb-sys/src/stable_api/ruby_2_6.rs b/crates/rb-sys/src/stable_api/ruby_2_6.rs index 0407f8d7..6293e428 100644 --- a/crates/rb-sys/src/stable_api/ruby_2_6.rs +++ b/crates/rb-sys/src/stable_api/ruby_2_6.rs @@ -4,14 +4,39 @@ use crate::ruby_rarray_flags::*; use crate::ruby_rstring_flags::*; use crate::{ internal::{RArray, RString}, - value_type, VALUE, + ruby_fl_type, value_type, VALUE, }; use std::{ + ffi::{c_int, CStr}, os::raw::{c_char, c_long}, ptr::NonNull, time::Duration, }; +const RSTRUCT_EMBED_LEN_MASK: VALUE = + (ruby_fl_type::RUBY_FL_USER2 as VALUE) | (ruby_fl_type::RUBY_FL_USER1 as VALUE); +const RSTRUCT_EMBED_LEN_SHIFT: VALUE = (ruby_fl_type::RUBY_FL_USHIFT as VALUE) + 1; +const RSTRUCT_EMBED_LEN_MAX: usize = 3; + +#[repr(C)] +#[derive(Clone, Copy)] +pub(crate) struct HeapStructData { + pub(crate) len: std::ffi::c_long, + pub(crate) ptr: *const VALUE, +} + +#[repr(C)] +pub(crate) union RStructUnion { + pub(crate) heap: HeapStructData, + pub(crate) ary: [VALUE; RSTRUCT_EMBED_LEN_MAX], +} + +#[repr(C)] +pub struct RStruct { + pub(crate) basic: crate::RBasic, + pub(crate) as_: RStructUnion, +} + #[cfg(not(ruby_eq_2_6))] compile_error!("This file should only be included in Ruby 2.6 builds"); @@ -276,4 +301,37 @@ impl StableApiDefinition for Definition { unsafe { crate::rb_thread_wait_for(time) } } + + #[inline] + fn rstruct_define(&self, name: &CStr, members: &[&CStr]) -> VALUE { + let mut members: Vec<*const c_char> = members + .iter() + .map(|m| m.as_ptr() as *const c_char) + .collect(); + members.push(std::ptr::null()); + unsafe { crate::rb_struct_define(name.as_ptr(), members) } + } + + #[inline] + unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { + crate::rb_struct_getmember(st, idx as _) + } + + #[inline] + unsafe fn rstruct_set(&self, st: VALUE, idx: c_int, value: VALUE) { + crate::rb_struct_aset(st, idx as _, value); + } + + #[inline] + unsafe fn rstruct_len(&self, st: VALUE) -> c_long { + let rbasic = st as *const crate::RBasic; + if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + let mut ret = ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) as c_long; + ret >>= RSTRUCT_EMBED_LEN_SHIFT; + ret + } else { + let rstruct = st as *const RStruct; + (*rstruct).as_.heap.len + } + } } diff --git a/crates/rb-sys/src/stable_api/ruby_2_7.rs b/crates/rb-sys/src/stable_api/ruby_2_7.rs index e5a1eb12..643cbfd2 100644 --- a/crates/rb-sys/src/stable_api/ruby_2_7.rs +++ b/crates/rb-sys/src/stable_api/ruby_2_7.rs @@ -1,14 +1,39 @@ use super::StableApiDefinition; use crate::{ internal::{RArray, RString}, - value_type, VALUE, + ruby_fl_type, value_type, VALUE, }; use std::{ + ffi::{c_int, CStr}, os::raw::{c_char, c_long}, ptr::NonNull, time::Duration, }; +const RSTRUCT_EMBED_LEN_MASK: VALUE = + (ruby_fl_type::RUBY_FL_USER2 as VALUE) | (ruby_fl_type::RUBY_FL_USER1 as VALUE); +const RSTRUCT_EMBED_LEN_SHIFT: VALUE = (ruby_fl_type::RUBY_FL_USHIFT as VALUE) + 1; +const RSTRUCT_EMBED_LEN_MAX: usize = 3; + +#[repr(C)] +#[derive(Clone, Copy)] +pub(crate) struct HeapStructData { + pub(crate) len: std::ffi::c_long, + pub(crate) ptr: *const VALUE, +} + +#[repr(C)] +pub(crate) union RStructUnion { + pub(crate) heap: HeapStructData, + pub(crate) ary: [VALUE; RSTRUCT_EMBED_LEN_MAX], +} + +#[repr(C)] +pub struct RStruct { + pub(crate) basic: crate::RBasic, + pub(crate) as_: RStructUnion, +} + #[cfg(not(ruby_eq_2_7))] compile_error!("This file should only be included in Ruby 2.7 builds"); @@ -276,4 +301,37 @@ impl StableApiDefinition for Definition { unsafe { crate::rb_thread_wait_for(time) } } + + #[inline] + fn rstruct_define(&self, name: &CStr, members: &[&CStr]) -> VALUE { + let mut members: Vec<*const c_char> = members + .iter() + .map(|m| m.as_ptr() as *const c_char) + .collect(); + members.push(std::ptr::null()); + unsafe { crate::rb_struct_define(name.as_ptr(), members) } + } + + #[inline] + unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { + crate::rb_struct_getmember(st, idx as _) + } + + #[inline] + unsafe fn rstruct_set(&self, st: VALUE, idx: c_int, value: VALUE) { + crate::rb_struct_aset(st, idx as _, value); + } + + #[inline] + unsafe fn rstruct_len(&self, st: VALUE) -> c_long { + let rbasic = st as *const crate::RBasic; + if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + let mut ret = ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) as c_long; + ret >>= RSTRUCT_EMBED_LEN_SHIFT; + ret + } else { + let rstruct = st as *const RStruct; + (*rstruct).as_.heap.len + } + } } diff --git a/crates/rb-sys/src/stable_api/ruby_3_0.rs b/crates/rb-sys/src/stable_api/ruby_3_0.rs index cd22ae7a..ed3a0b65 100644 --- a/crates/rb-sys/src/stable_api/ruby_3_0.rs +++ b/crates/rb-sys/src/stable_api/ruby_3_0.rs @@ -1,14 +1,39 @@ use super::StableApiDefinition; use crate::{ internal::{RArray, RString}, - value_type, VALUE, + ruby_fl_type, ruby_fl_ushift, value_type, VALUE, }; use std::{ + ffi::{c_int, CStr}, os::raw::{c_char, c_long}, ptr::NonNull, time::Duration, }; +const RSTRUCT_EMBED_LEN_MASK: VALUE = + (ruby_fl_type::RUBY_FL_USER2 as VALUE) | (ruby_fl_type::RUBY_FL_USER1 as VALUE); +const RSTRUCT_EMBED_LEN_SHIFT: VALUE = (ruby_fl_ushift::RUBY_FL_USHIFT as VALUE) + 1; +const RSTRUCT_EMBED_LEN_MAX: usize = 3; + +#[repr(C)] +#[derive(Clone, Copy)] +pub(crate) struct HeapStructData { + pub(crate) len: std::ffi::c_long, + pub(crate) ptr: *const VALUE, +} + +#[repr(C)] +pub(crate) union RStructUnion { + pub(crate) heap: HeapStructData, + pub(crate) ary: [VALUE; RSTRUCT_EMBED_LEN_MAX], +} + +#[repr(C)] +pub struct RStruct { + pub(crate) basic: crate::RBasic, + pub(crate) as_: RStructUnion, +} + #[cfg(not(ruby_eq_3_0))] compile_error!("This file should only be included in Ruby 3.0 builds"); @@ -284,4 +309,37 @@ impl StableApiDefinition for Definition { unsafe { crate::rb_thread_wait_for(time) } } + + #[inline] + fn rstruct_define(&self, name: &CStr, members: &[&CStr]) -> VALUE { + let mut members: Vec<*const c_char> = members + .iter() + .map(|m| m.as_ptr() as *const c_char) + .collect(); + members.push(std::ptr::null()); + unsafe { crate::rb_struct_define(name.as_ptr(), members) } + } + + #[inline] + unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { + crate::rb_struct_getmember(st, idx as _) + } + + #[inline] + unsafe fn rstruct_set(&self, st: VALUE, idx: c_int, value: VALUE) { + crate::rb_struct_aset(st, idx as _, value); + } + + #[inline] + unsafe fn rstruct_len(&self, st: VALUE) -> c_long { + let rbasic = st as *const crate::RBasic; + if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + let mut ret = ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) as c_long; + ret >>= RSTRUCT_EMBED_LEN_SHIFT; + ret + } else { + let rstruct = st as *const RStruct; + (*rstruct).as_.heap.len + } + } } diff --git a/crates/rb-sys/src/stable_api/ruby_3_1.rs b/crates/rb-sys/src/stable_api/ruby_3_1.rs index a604426f..17d9ff4e 100644 --- a/crates/rb-sys/src/stable_api/ruby_3_1.rs +++ b/crates/rb-sys/src/stable_api/ruby_3_1.rs @@ -1,14 +1,38 @@ use super::StableApiDefinition; use crate::{ internal::{RArray, RString}, - value_type, VALUE, + ruby_fl_type, ruby_fl_ushift, value_type, VALUE, }; use std::{ + ffi::{c_int, CStr}, os::raw::{c_char, c_long}, ptr::NonNull, time::Duration, }; +const RSTRUCT_EMBED_LEN_MASK: VALUE = + (ruby_fl_type::RUBY_FL_USER2 as VALUE) | (ruby_fl_type::RUBY_FL_USER1 as VALUE); +const RSTRUCT_EMBED_LEN_SHIFT: VALUE = (ruby_fl_ushift::RUBY_FL_USHIFT as VALUE) + 1; +const RSTRUCT_EMBED_LEN_MAX: usize = 3; + +#[repr(C)] +#[derive(Clone, Copy)] +pub(crate) struct HeapStructData { + pub(crate) len: std::ffi::c_long, + pub(crate) ptr: *const VALUE, +} + +#[repr(C)] +pub(crate) union RStructUnion { + pub(crate) heap: HeapStructData, + pub(crate) ary: [VALUE; RSTRUCT_EMBED_LEN_MAX], +} + +#[repr(C)] +pub struct RStruct { + pub(crate) basic: crate::RBasic, + pub(crate) as_: RStructUnion, +} #[cfg(not(ruby_eq_3_1))] compile_error!("This file should only be included in Ruby 3.1 builds"); @@ -277,4 +301,37 @@ impl StableApiDefinition for Definition { unsafe { crate::rb_thread_wait_for(time) } } + + #[inline] + fn rstruct_define(&self, name: &CStr, members: &[&CStr]) -> VALUE { + let mut members: Vec<*const c_char> = members + .iter() + .map(|m| m.as_ptr() as *const c_char) + .collect(); + members.push(std::ptr::null()); + unsafe { crate::rb_struct_define(name.as_ptr(), members) } + } + + #[inline] + unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { + crate::rb_struct_getmember(st, idx as _) + } + + #[inline] + unsafe fn rstruct_set(&self, st: VALUE, idx: c_int, value: VALUE) { + crate::rb_struct_aset(st, idx as _, value); + } + + #[inline] + unsafe fn rstruct_len(&self, st: VALUE) -> c_long { + let rbasic = st as *const crate::RBasic; + if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + let mut ret = ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) as c_long; + ret >>= RSTRUCT_EMBED_LEN_SHIFT; + ret + } else { + let rstruct = st as *const RStruct; + (*rstruct).as_.heap.len + } + } } diff --git a/crates/rb-sys/src/stable_api/ruby_3_2.rs b/crates/rb-sys/src/stable_api/ruby_3_2.rs index 52e0067e..c20bc24b 100644 --- a/crates/rb-sys/src/stable_api/ruby_3_2.rs +++ b/crates/rb-sys/src/stable_api/ruby_3_2.rs @@ -1,14 +1,39 @@ use super::StableApiDefinition; use crate::{ internal::{RArray, RString}, - value_type, VALUE, + ruby_fl_type, ruby_fl_ushift, value_type, VALUE, }; use std::{ + ffi::{c_int, CStr}, os::raw::{c_char, c_long}, ptr::NonNull, time::Duration, }; +const RSTRUCT_EMBED_LEN_MASK: VALUE = + (ruby_fl_type::RUBY_FL_USER2 as VALUE) | (ruby_fl_type::RUBY_FL_USER1 as VALUE); +const RSTRUCT_EMBED_LEN_SHIFT: VALUE = (ruby_fl_ushift::RUBY_FL_USHIFT as VALUE) + 1; +const RSTRUCT_EMBED_LEN_MAX: usize = 3; + +#[repr(C)] +#[derive(Clone, Copy)] +pub(crate) struct HeapStructData { + pub(crate) len: std::ffi::c_long, + pub(crate) ptr: *const VALUE, +} + +#[repr(C)] +pub(crate) union RStructUnion { + pub(crate) heap: HeapStructData, + pub(crate) ary: [VALUE; RSTRUCT_EMBED_LEN_MAX], +} + +#[repr(C)] +pub struct RStruct { + pub(crate) basic: crate::RBasic, + pub(crate) as_: RStructUnion, +} + #[cfg(not(ruby_eq_3_2))] compile_error!("This file should only be included in Ruby 3.2 builds"); @@ -275,4 +300,37 @@ impl StableApiDefinition for Definition { unsafe { crate::rb_thread_wait_for(time) } } + + #[inline] + fn rstruct_define(&self, name: &CStr, members: &[&CStr]) -> VALUE { + let mut members: Vec<*const c_char> = members + .iter() + .map(|m| m.as_ptr() as *const c_char) + .collect(); + members.push(std::ptr::null()); + unsafe { crate::rb_struct_define(name.as_ptr(), members) } + } + + #[inline] + unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { + crate::rb_struct_getmember(st, idx as _) + } + + #[inline] + unsafe fn rstruct_set(&self, st: VALUE, idx: c_int, value: VALUE) { + crate::rb_struct_aset(st, idx as _, value); + } + + #[inline] + unsafe fn rstruct_len(&self, st: VALUE) -> c_long { + let rbasic = st as *const crate::RBasic; + if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + let mut ret = ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) as c_long; + ret >>= RSTRUCT_EMBED_LEN_SHIFT; + ret + } else { + let rstruct = st as *const RStruct; + (*rstruct).as_.heap.len + } + } } diff --git a/crates/rb-sys/src/stable_api/ruby_3_3.rs b/crates/rb-sys/src/stable_api/ruby_3_3.rs index fe39fa2f..02eeb2eb 100644 --- a/crates/rb-sys/src/stable_api/ruby_3_3.rs +++ b/crates/rb-sys/src/stable_api/ruby_3_3.rs @@ -1,7 +1,7 @@ use super::StableApiDefinition; use crate::{ internal::{RArray, RString}, - ruby_fl_type, value_type, VALUE, + ruby_fl_type, ruby_fl_ushift, value_type, VALUE, }; use std::{ ffi::{c_int, CStr}, @@ -10,15 +10,35 @@ use std::{ time::Duration, }; -const RSTRUCT_EMBED_LEN_MASK: i64 = (ruby_fl_type::RUBY_FL_USER7 as i64) - | (ruby_fl_type::RUBY_FL_USER6 as i64) - | (ruby_fl_type::RUBY_FL_USER5 as i64) - | (ruby_fl_type::RUBY_FL_USER4 as i64) - | (ruby_fl_type::RUBY_FL_USER3 as i64) - | (ruby_fl_type::RUBY_FL_USER2 as i64) - | (ruby_fl_type::RUBY_FL_USER1 as i64); +const RSTRUCT_EMBED_LEN_MASK: VALUE = (ruby_fl_type::RUBY_FL_USER7 as VALUE) + | (ruby_fl_type::RUBY_FL_USER6 as VALUE) + | (ruby_fl_type::RUBY_FL_USER5 as VALUE) + | (ruby_fl_type::RUBY_FL_USER4 as VALUE) + | (ruby_fl_type::RUBY_FL_USER3 as VALUE) + | (ruby_fl_type::RUBY_FL_USER2 as VALUE) + | (ruby_fl_type::RUBY_FL_USER1 as VALUE); + +const RSTRUCT_EMBED_LEN_SHIFT: VALUE = (ruby_fl_ushift::RUBY_FL_USHIFT as VALUE) + 1; + +#[repr(C)] +#[derive(Clone, Copy)] +pub(crate) struct HeapStructData { + pub(crate) len: std::ffi::c_long, + pub(crate) ptr: *const VALUE, +} -const RSTRUCT_EMBED_LEN_SHIFT: i64 = (crate::ruby_fl_ushift::RUBY_FL_USHIFT as i64) + 1; +#[repr(C)] +pub(crate) union RStructUnion { + pub(crate) heap: HeapStructData, + // Match the C array[1] layout + pub(crate) ary: [VALUE; 1], +} + +#[repr(C)] +pub struct RStruct { + pub(crate) basic: crate::RBasic, + pub(crate) as_: RStructUnion, +} #[cfg(not(ruby_eq_3_3))] compile_error!("This file should only be included in Ruby 3.3 builds"); @@ -280,6 +300,7 @@ impl StableApiDefinition for Definition { unsafe { crate::rb_thread_wait_for(time) } } + #[inline] fn rstruct_define(&self, name: &CStr, members: &[&CStr]) -> VALUE { let mut members: Vec<*const c_char> = members .iter() @@ -289,18 +310,17 @@ impl StableApiDefinition for Definition { unsafe { crate::rb_struct_define(name.as_ptr(), members) } } + #[inline] unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { crate::rb_struct_getmember(st, idx as _) } + #[inline] unsafe fn rstruct_set(&self, st: VALUE, idx: c_int, value: VALUE) { crate::rb_struct_aset(st, idx as _, value); } - // unsafe fn rstruct_len(&self, st: VALUE) -> c_long { - // crate::rb_num2long(crate::rb_struct_size(st)) - // } - + #[inline] unsafe fn rstruct_len(&self, st: VALUE) -> c_long { let rbasic = st as *const crate::RBasic; if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { @@ -308,8 +328,8 @@ impl StableApiDefinition for Definition { ret >>= RSTRUCT_EMBED_LEN_SHIFT; ret } else { - let rstruct = st as *const crate::RStruct; - (*rstruct).as_.heap.len as c_long + let rstruct = st as *const RStruct; + (*rstruct).as_.heap.len } } } diff --git a/crates/rb-sys/src/stable_api/ruby_3_4.rs b/crates/rb-sys/src/stable_api/ruby_3_4.rs index 74a82f5b..f5464e6f 100644 --- a/crates/rb-sys/src/stable_api/ruby_3_4.rs +++ b/crates/rb-sys/src/stable_api/ruby_3_4.rs @@ -1,14 +1,45 @@ use super::StableApiDefinition; use crate::{ internal::{RArray, RString}, - value_type, VALUE, + ruby_fl_type, ruby_fl_ushift, value_type, VALUE, }; use std::{ + ffi::{c_int, CStr}, os::raw::{c_char, c_long}, ptr::NonNull, time::Duration, }; +const RSTRUCT_EMBED_LEN_MASK: VALUE = (ruby_fl_type::RUBY_FL_USER7 as VALUE) + | (ruby_fl_type::RUBY_FL_USER6 as VALUE) + | (ruby_fl_type::RUBY_FL_USER5 as VALUE) + | (ruby_fl_type::RUBY_FL_USER4 as VALUE) + | (ruby_fl_type::RUBY_FL_USER3 as VALUE) + | (ruby_fl_type::RUBY_FL_USER2 as VALUE) + | (ruby_fl_type::RUBY_FL_USER1 as VALUE); + +const RSTRUCT_EMBED_LEN_SHIFT: VALUE = (ruby_fl_ushift::RUBY_FL_USHIFT as VALUE) + 1; + +#[repr(C)] +#[derive(Clone, Copy)] +pub(crate) struct HeapStructData { + pub(crate) len: std::ffi::c_long, + pub(crate) ptr: *const VALUE, +} + +#[repr(C)] +pub(crate) union RStructUnion { + pub(crate) heap: HeapStructData, + // Match the C array[1] layout + pub(crate) ary: [VALUE; 1], +} + +#[repr(C)] +pub struct RStruct { + pub(crate) basic: crate::RBasic, + pub(crate) as_: RStructUnion, +} + #[cfg(not(ruby_eq_3_4))] compile_error!("This file should only be included in Ruby 3.3 builds"); @@ -268,4 +299,37 @@ impl StableApiDefinition for Definition { unsafe { crate::rb_thread_wait_for(time) } } + + #[inline] + fn rstruct_define(&self, name: &CStr, members: &[&CStr]) -> VALUE { + let mut members: Vec<*const c_char> = members + .iter() + .map(|m| m.as_ptr() as *const c_char) + .collect(); + members.push(std::ptr::null()); + unsafe { crate::rb_struct_define(name.as_ptr(), members) } + } + + #[inline] + unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { + crate::rb_struct_getmember(st, idx as _) + } + + #[inline] + unsafe fn rstruct_set(&self, st: VALUE, idx: c_int, value: VALUE) { + crate::rb_struct_aset(st, idx as _, value); + } + + #[inline] + unsafe fn rstruct_len(&self, st: VALUE) -> c_long { + let rbasic = st as *const crate::RBasic; + if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + let mut ret = ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) as c_long; + ret >>= RSTRUCT_EMBED_LEN_SHIFT; + ret + } else { + let rstruct = st as *const RStruct; + (*rstruct).as_.heap.len + } + } } From 267fe2fff7723b5da6d228ee195fa62b4c0a7bd8 Mon Sep 17 00:00:00 2001 From: Jose Narvaez Date: Thu, 19 Dec 2024 21:15:45 +0000 Subject: [PATCH 3/6] Manual implementation of rstruct_get with extremely sus tests. --- crates/rb-sys-tests/src/stable_api_test.rs | 57 ++++++++++++++++++++++ crates/rb-sys/src/stable_api/compiled.c | 4 +- crates/rb-sys/src/stable_api/compiled.rs | 4 +- crates/rb-sys/src/stable_api/ruby_2_6.rs | 12 ++++- crates/rb-sys/src/stable_api/ruby_2_7.rs | 12 ++++- crates/rb-sys/src/stable_api/ruby_3_0.rs | 13 ++++- crates/rb-sys/src/stable_api/ruby_3_1.rs | 12 ++++- crates/rb-sys/src/stable_api/ruby_3_2.rs | 12 ++++- crates/rb-sys/src/stable_api/ruby_3_3.rs | 12 ++++- crates/rb-sys/src/stable_api/ruby_3_4.rs | 12 ++++- 10 files changed, 139 insertions(+), 11 deletions(-) diff --git a/crates/rb-sys-tests/src/stable_api_test.rs b/crates/rb-sys-tests/src/stable_api_test.rs index a68379e4..abce5991 100644 --- a/crates/rb-sys-tests/src/stable_api_test.rs +++ b/crates/rb-sys-tests/src/stable_api_test.rs @@ -686,3 +686,60 @@ parity_test!( }, expected: 81 ); + +#[rb_sys_test_helpers::ruby_test] +fn test_rb_rstruct_get_embedded() { + use rb_sys::stable_api; + let data = { + ruby_eval!( + " + Person = Struct.new(:name, :age); + Person.new('Matz', 59); + " + ) + }; + assert_ne!(stable_api::get_default().version(), (0, 0)); + #[allow(unused)] + let rust_result = unsafe { stable_api::get_default().rstruct_get(data, 0) }; + #[allow(unused_unsafe)] + let compiled_c_result = unsafe { stable_api::get_compiled().rstruct_get(data, 0) }; + assert_eq!( + compiled_c_result, rust_result, + "compiled_c was {:?}, rust was {:?}", + compiled_c_result, rust_result + ); +} + +#[rb_sys_test_helpers::ruby_test] +fn test_rb_rstruct_get_heap() { + use rb_sys::stable_api; + let data = { + ruby_eval!( + " + field_names = (0..80).map { |i| \"field#{i}\" }; + LargeStruct = Struct.new(*field_names.map(&:to_sym)) do + def initialize(*) + super + members.each do |field| + self[field] = 0 if self[field].nil? + end + end + end; + + st = LargeStruct.new; + st[:field79] = true; + st + " + ) + }; + assert_ne!(stable_api::get_default().version(), (0, 0)); + #[allow(unused)] + let rust_result = unsafe { stable_api::get_default().rstruct_get(data, 79) }; + #[allow(unused_unsafe)] + let compiled_c_result = unsafe { stable_api::get_compiled().rstruct_get(data, 79) }; + assert_eq!( + compiled_c_result, rust_result, + "compiled_c was {:?}, rust was {:?}", + compiled_c_result, rust_result + ); +} diff --git a/crates/rb-sys/src/stable_api/compiled.c b/crates/rb-sys/src/stable_api/compiled.c index 5a416e5a..b8e29492 100644 --- a/crates/rb-sys/src/stable_api/compiled.c +++ b/crates/rb-sys/src/stable_api/compiled.c @@ -133,11 +133,11 @@ impl_rstruct_define(char* name, ...) { VALUE impl_rstruct_get(VALUE st, int idx) { - return Qnil; + return RSTRUCT_GET(st, idx); } VALUE impl_rstruct_set(VALUE st, int idx, VALUE value) { - return Qnil; + return RSTRUCT_SET(st, idx, value); } diff --git a/crates/rb-sys/src/stable_api/compiled.rs b/crates/rb-sys/src/stable_api/compiled.rs index 6ea2dd81..b634b652 100644 --- a/crates/rb-sys/src/stable_api/compiled.rs +++ b/crates/rb-sys/src/stable_api/compiled.rs @@ -82,7 +82,7 @@ extern "C" { fn impl_thread_sleep(interval: timeval); #[link_name = "impl_rstruct_define"] - fn impl_rstruct_define(name: &CStr, members: Vec<*const c_char>) -> VALUE; + fn impl_rstruct_define(name: &CStr, members: &[*const c_char]) -> VALUE; #[link_name = "impl_rstruct_get"] fn impl_rstruct_get(st: VALUE, idx: c_int) -> VALUE; @@ -235,7 +235,7 @@ impl StableApiDefinition for Definition { .map(|m| m.as_ptr() as *const c_char) .collect(); members.push(std::ptr::null()); - unsafe { impl_rstruct_define(name, members) } + unsafe { impl_rstruct_define(name, members.as_ref()) } } #[inline] diff --git a/crates/rb-sys/src/stable_api/ruby_2_6.rs b/crates/rb-sys/src/stable_api/ruby_2_6.rs index 6293e428..51c10153 100644 --- a/crates/rb-sys/src/stable_api/ruby_2_6.rs +++ b/crates/rb-sys/src/stable_api/ruby_2_6.rs @@ -314,7 +314,17 @@ impl StableApiDefinition for Definition { #[inline] unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { - crate::rb_struct_getmember(st, idx as _) + let rbasic = st as *const crate::RBasic; + let rstruct = st as *const RStruct; + let slice: &[VALUE] = if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + (*rstruct).as_.ary.as_slice() + } else { + let ptr = (*rstruct).as_.heap.ptr; + let len = (*rstruct).as_.heap.len as _; + std::slice::from_raw_parts(ptr, len) + }; + + slice[idx as usize] } #[inline] diff --git a/crates/rb-sys/src/stable_api/ruby_2_7.rs b/crates/rb-sys/src/stable_api/ruby_2_7.rs index 643cbfd2..4bec45d7 100644 --- a/crates/rb-sys/src/stable_api/ruby_2_7.rs +++ b/crates/rb-sys/src/stable_api/ruby_2_7.rs @@ -314,7 +314,17 @@ impl StableApiDefinition for Definition { #[inline] unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { - crate::rb_struct_getmember(st, idx as _) + let rbasic = st as *const crate::RBasic; + let rstruct = st as *const RStruct; + let slice: &[VALUE] = if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + (*rstruct).as_.ary.as_slice() + } else { + let ptr = (*rstruct).as_.heap.ptr; + let len = (*rstruct).as_.heap.len as _; + std::slice::from_raw_parts(ptr, len) + }; + + slice[idx as usize] } #[inline] diff --git a/crates/rb-sys/src/stable_api/ruby_3_0.rs b/crates/rb-sys/src/stable_api/ruby_3_0.rs index ed3a0b65..cadfc5a1 100644 --- a/crates/rb-sys/src/stable_api/ruby_3_0.rs +++ b/crates/rb-sys/src/stable_api/ruby_3_0.rs @@ -1,4 +1,5 @@ use super::StableApiDefinition; + use crate::{ internal::{RArray, RString}, ruby_fl_type, ruby_fl_ushift, value_type, VALUE, @@ -322,7 +323,17 @@ impl StableApiDefinition for Definition { #[inline] unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { - crate::rb_struct_getmember(st, idx as _) + let rbasic = st as *const crate::RBasic; + let rstruct = st as *const RStruct; + let slice: &[VALUE] = if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + (*rstruct).as_.ary.as_slice() + } else { + let ptr = (*rstruct).as_.heap.ptr; + let len = (*rstruct).as_.heap.len as _; + std::slice::from_raw_parts(ptr, len) + }; + + slice[idx as usize] } #[inline] diff --git a/crates/rb-sys/src/stable_api/ruby_3_1.rs b/crates/rb-sys/src/stable_api/ruby_3_1.rs index 17d9ff4e..04c8ec7b 100644 --- a/crates/rb-sys/src/stable_api/ruby_3_1.rs +++ b/crates/rb-sys/src/stable_api/ruby_3_1.rs @@ -314,7 +314,17 @@ impl StableApiDefinition for Definition { #[inline] unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { - crate::rb_struct_getmember(st, idx as _) + let rbasic = st as *const crate::RBasic; + let rstruct = st as *const RStruct; + let slice: &[VALUE] = if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + (*rstruct).as_.ary.as_slice() + } else { + let ptr = (*rstruct).as_.heap.ptr; + let len = (*rstruct).as_.heap.len as _; + std::slice::from_raw_parts(ptr, len) + }; + + slice[idx as usize] } #[inline] diff --git a/crates/rb-sys/src/stable_api/ruby_3_2.rs b/crates/rb-sys/src/stable_api/ruby_3_2.rs index c20bc24b..e58d8d7f 100644 --- a/crates/rb-sys/src/stable_api/ruby_3_2.rs +++ b/crates/rb-sys/src/stable_api/ruby_3_2.rs @@ -313,7 +313,17 @@ impl StableApiDefinition for Definition { #[inline] unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { - crate::rb_struct_getmember(st, idx as _) + let rbasic = st as *const crate::RBasic; + let rstruct = st as *const RStruct; + let slice: &[VALUE] = if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + (*rstruct).as_.ary.as_slice() + } else { + let ptr = (*rstruct).as_.heap.ptr; + let len = (*rstruct).as_.heap.len as _; + std::slice::from_raw_parts(ptr, len) + }; + + slice[idx as usize] } #[inline] diff --git a/crates/rb-sys/src/stable_api/ruby_3_3.rs b/crates/rb-sys/src/stable_api/ruby_3_3.rs index 02eeb2eb..f44a8b34 100644 --- a/crates/rb-sys/src/stable_api/ruby_3_3.rs +++ b/crates/rb-sys/src/stable_api/ruby_3_3.rs @@ -312,7 +312,17 @@ impl StableApiDefinition for Definition { #[inline] unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { - crate::rb_struct_getmember(st, idx as _) + let rbasic = st as *const crate::RBasic; + let rstruct = st as *const RStruct; + let slice: &[VALUE] = if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + (*rstruct).as_.ary.as_slice() + } else { + let ptr = (*rstruct).as_.heap.ptr; + let len = (*rstruct).as_.heap.len as _; + std::slice::from_raw_parts(ptr, len) + }; + + slice[idx as usize] } #[inline] diff --git a/crates/rb-sys/src/stable_api/ruby_3_4.rs b/crates/rb-sys/src/stable_api/ruby_3_4.rs index f5464e6f..7e33fd72 100644 --- a/crates/rb-sys/src/stable_api/ruby_3_4.rs +++ b/crates/rb-sys/src/stable_api/ruby_3_4.rs @@ -312,7 +312,17 @@ impl StableApiDefinition for Definition { #[inline] unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { - crate::rb_struct_getmember(st, idx as _) + let rbasic = st as *const crate::RBasic; + let rstruct = st as *const RStruct; + let slice: &[VALUE] = if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + (*rstruct).as_.ary.as_slice() + } else { + let ptr = (*rstruct).as_.heap.ptr; + let len = (*rstruct).as_.heap.len as _; + std::slice::from_raw_parts(ptr, len) + }; + + slice[idx as usize] } #[inline] From b530ae66f63bb2ff1da0e9cfcaa5ac225e52ff84 Mon Sep 17 00:00:00 2001 From: Jose Narvaez Date: Thu, 19 Dec 2024 21:42:14 +0000 Subject: [PATCH 4/6] Testing something in ruby 3.4 --- crates/rb-sys/src/stable_api/ruby_3_4.rs | 28 ++++++++++++++---------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/crates/rb-sys/src/stable_api/ruby_3_4.rs b/crates/rb-sys/src/stable_api/ruby_3_4.rs index 7e33fd72..5c024301 100644 --- a/crates/rb-sys/src/stable_api/ruby_3_4.rs +++ b/crates/rb-sys/src/stable_api/ruby_3_4.rs @@ -30,7 +30,6 @@ pub(crate) struct HeapStructData { #[repr(C)] pub(crate) union RStructUnion { pub(crate) heap: HeapStructData, - // Match the C array[1] layout pub(crate) ary: [VALUE; 1], } @@ -312,19 +311,24 @@ impl StableApiDefinition for Definition { #[inline] unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { - let rbasic = st as *const crate::RBasic; - let rstruct = st as *const RStruct; - let slice: &[VALUE] = if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { - (*rstruct).as_.ary.as_slice() - } else { - let ptr = (*rstruct).as_.heap.ptr; - let len = (*rstruct).as_.heap.len as _; - std::slice::from_raw_parts(ptr, len) - }; - - slice[idx as usize] + crate::rb_struct_getmember(st, idx as _) } + // #[inline] + // unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { + // let rbasic = st as *const crate::RBasic; + // let rstruct = st as *const RStruct; + // let slice: &[VALUE] = if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + // (*rstruct).as_.ary.as_slice() + // } else { + // let ptr = (*rstruct).as_.heap.ptr; + // let len = (*rstruct).as_.heap.len as _; + // std::slice::from_raw_parts(ptr, len) + // }; + // + // slice[idx as usize] + // } + #[inline] unsafe fn rstruct_set(&self, st: VALUE, idx: c_int, value: VALUE) { crate::rb_struct_aset(st, idx as _, value); From a433b3af2fb7fd2e91dd8d3ae943e011eb88c72e Mon Sep 17 00:00:00 2001 From: Jose Narvaez Date: Thu, 19 Dec 2024 21:46:45 +0000 Subject: [PATCH 5/6] Removed C based struct definition. --- .../rb-sys-build/src/bindings/stable_api.rs | 2 +- crates/rb-sys-build/src/bindings/wrapper.h | 16 ----------- crates/rb-sys/src/stable_api/ruby_3_4.rs | 27 ++++++++----------- 3 files changed, 12 insertions(+), 33 deletions(-) diff --git a/crates/rb-sys-build/src/bindings/stable_api.rs b/crates/rb-sys-build/src/bindings/stable_api.rs index 130cefac..479da284 100644 --- a/crates/rb-sys-build/src/bindings/stable_api.rs +++ b/crates/rb-sys-build/src/bindings/stable_api.rs @@ -4,7 +4,7 @@ use quote::ToTokens; use crate::RbConfig; -const OPAQUE_STRUCTS: [&str; 3] = ["RString", "RArray", "RStruct"]; +const OPAQUE_STRUCTS: [&str; 2] = ["RString", "RArray"]; const OPAQUE_STRUCTS_RUBY_3_3: [&str; 3] = [ "rb_matchext_struct", diff --git a/crates/rb-sys-build/src/bindings/wrapper.h b/crates/rb-sys-build/src/bindings/wrapper.h index 11fd7308..43814a9a 100644 --- a/crates/rb-sys-build/src/bindings/wrapper.h +++ b/crates/rb-sys-build/src/bindings/wrapper.h @@ -69,19 +69,3 @@ #ifdef HAVE_RUBY_IO_BUFFER_H #include "ruby/io/buffer.h" #endif - -struct RStruct { - struct RBasic basic; - union { - struct { - long len; - const VALUE *ptr; - } heap; - /* This is a length 1 array because: - * 1. GCC has a bug that does not optimize C flexible array members - * (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102452) - * 2. Zero length arrays are not supported by all compilers - */ - const VALUE ary[1]; - } as; -}; diff --git a/crates/rb-sys/src/stable_api/ruby_3_4.rs b/crates/rb-sys/src/stable_api/ruby_3_4.rs index 5c024301..0f0a9701 100644 --- a/crates/rb-sys/src/stable_api/ruby_3_4.rs +++ b/crates/rb-sys/src/stable_api/ruby_3_4.rs @@ -311,23 +311,18 @@ impl StableApiDefinition for Definition { #[inline] unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { - crate::rb_struct_getmember(st, idx as _) - } + let rbasic = st as *const crate::RBasic; + let rstruct = st as *const RStruct; + let slice: &[VALUE] = if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + (*rstruct).as_.ary.as_slice() + } else { + let ptr = (*rstruct).as_.heap.ptr; + let len = (*rstruct).as_.heap.len as _; + std::slice::from_raw_parts(ptr, len) + }; - // #[inline] - // unsafe fn rstruct_get(&self, st: VALUE, idx: c_int) -> VALUE { - // let rbasic = st as *const crate::RBasic; - // let rstruct = st as *const RStruct; - // let slice: &[VALUE] = if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { - // (*rstruct).as_.ary.as_slice() - // } else { - // let ptr = (*rstruct).as_.heap.ptr; - // let len = (*rstruct).as_.heap.len as _; - // std::slice::from_raw_parts(ptr, len) - // }; - // - // slice[idx as usize] - // } + slice[idx as usize] + } #[inline] unsafe fn rstruct_set(&self, st: VALUE, idx: c_int, value: VALUE) { From b05c8d72a880897cc08320c3b6624b5d55d13774 Mon Sep 17 00:00:00 2001 From: Jose Narvaez Date: Sun, 29 Dec 2024 19:51:58 +0000 Subject: [PATCH 6/6] Dubious manual impl of rstruct_set. --- crates/rb-sys-tests/src/stable_api_test.rs | 68 ++++++++++++++++++++++ crates/rb-sys/src/stable_api/ruby_2_6.rs | 12 +++- crates/rb-sys/src/stable_api/ruby_2_7.rs | 12 +++- crates/rb-sys/src/stable_api/ruby_3_0.rs | 12 +++- crates/rb-sys/src/stable_api/ruby_3_1.rs | 12 +++- crates/rb-sys/src/stable_api/ruby_3_2.rs | 12 +++- crates/rb-sys/src/stable_api/ruby_3_3.rs | 13 ++++- crates/rb-sys/src/stable_api/ruby_3_4.rs | 12 +++- 8 files changed, 146 insertions(+), 7 deletions(-) diff --git a/crates/rb-sys-tests/src/stable_api_test.rs b/crates/rb-sys-tests/src/stable_api_test.rs index abce5991..b9ac74dd 100644 --- a/crates/rb-sys-tests/src/stable_api_test.rs +++ b/crates/rb-sys-tests/src/stable_api_test.rs @@ -743,3 +743,71 @@ fn test_rb_rstruct_get_heap() { compiled_c_result, rust_result ); } + +#[rb_sys_test_helpers::ruby_test] +fn test_rb_rstruct_set_embedded() { + use rb_sys::stable_api; + let data = { + ruby_eval!( + " + Person = Struct.new(:name, :age); + Person.new('Matz', 59); + " + ) + }; + assert_ne!(stable_api::get_default().version(), (0, 0)); + #[allow(unused)] + let rust_result = unsafe { + stable_api::get_default().rstruct_set(data, 0, rb_sys::Qfalse as VALUE); + stable_api::get_default().rstruct_get(data, 0) + }; + #[allow(unused_unsafe)] + let compiled_c_result = unsafe { + stable_api::get_compiled().rstruct_set(data, 0, rb_sys::Qfalse as VALUE); + stable_api::get_compiled().rstruct_get(data, 0) + }; + assert_eq!( + compiled_c_result, rust_result, + "compiled_c was {:?}, rust was {:?}", + compiled_c_result, rust_result + ); +} + +#[rb_sys_test_helpers::ruby_test] +fn test_rb_rstruct_set_heap() { + use rb_sys::stable_api; + let data = { + ruby_eval!( + " + field_names = (0..80).map { |i| \"field#{i}\" }; + LargeStruct = Struct.new(*field_names.map(&:to_sym)) do + def initialize(*) + super + members.each do |field| + self[field] = 0 if self[field].nil? + end + end + end; + + st = LargeStruct.new; + st + " + ) + }; + assert_ne!(stable_api::get_default().version(), (0, 0)); + #[allow(unused)] + let rust_result = unsafe { + stable_api::get_default().rstruct_set(data, 79, rb_sys::Qfalse as VALUE); + stable_api::get_default().rstruct_get(data, 79) + }; + #[allow(unused_unsafe)] + let compiled_c_result = unsafe { + stable_api::get_compiled().rstruct_set(data, 79, rb_sys::Qfalse as VALUE); + stable_api::get_compiled().rstruct_get(data, 79) + }; + assert_eq!( + compiled_c_result, rust_result, + "compiled_c was {:?}, rust was {:?}", + compiled_c_result, rust_result + ); +} diff --git a/crates/rb-sys/src/stable_api/ruby_2_6.rs b/crates/rb-sys/src/stable_api/ruby_2_6.rs index 51c10153..99707278 100644 --- a/crates/rb-sys/src/stable_api/ruby_2_6.rs +++ b/crates/rb-sys/src/stable_api/ruby_2_6.rs @@ -329,7 +329,17 @@ impl StableApiDefinition for Definition { #[inline] unsafe fn rstruct_set(&self, st: VALUE, idx: c_int, value: VALUE) { - crate::rb_struct_aset(st, idx as _, value); + let rbasic = st as *const crate::RBasic; + let rstruct = st as *mut RStruct; + let slice: &mut [VALUE] = if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + (*rstruct).as_.ary.as_mut_slice() + } else { + let ptr = (*rstruct).as_.heap.ptr as *mut _; + let len = (*rstruct).as_.heap.len as _; + std::slice::from_raw_parts_mut(ptr, len) + }; + + slice[idx as usize] = value; } #[inline] diff --git a/crates/rb-sys/src/stable_api/ruby_2_7.rs b/crates/rb-sys/src/stable_api/ruby_2_7.rs index 4bec45d7..b9e1fc60 100644 --- a/crates/rb-sys/src/stable_api/ruby_2_7.rs +++ b/crates/rb-sys/src/stable_api/ruby_2_7.rs @@ -329,7 +329,17 @@ impl StableApiDefinition for Definition { #[inline] unsafe fn rstruct_set(&self, st: VALUE, idx: c_int, value: VALUE) { - crate::rb_struct_aset(st, idx as _, value); + let rbasic = st as *const crate::RBasic; + let rstruct = st as *mut RStruct; + let slice: &mut [VALUE] = if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + (*rstruct).as_.ary.as_mut_slice() + } else { + let ptr = (*rstruct).as_.heap.ptr as *mut _; + let len = (*rstruct).as_.heap.len as _; + std::slice::from_raw_parts_mut(ptr, len) + }; + + slice[idx as usize] = value; } #[inline] diff --git a/crates/rb-sys/src/stable_api/ruby_3_0.rs b/crates/rb-sys/src/stable_api/ruby_3_0.rs index cadfc5a1..e5c90bd0 100644 --- a/crates/rb-sys/src/stable_api/ruby_3_0.rs +++ b/crates/rb-sys/src/stable_api/ruby_3_0.rs @@ -338,7 +338,17 @@ impl StableApiDefinition for Definition { #[inline] unsafe fn rstruct_set(&self, st: VALUE, idx: c_int, value: VALUE) { - crate::rb_struct_aset(st, idx as _, value); + let rbasic = st as *const crate::RBasic; + let rstruct = st as *mut RStruct; + let slice: &mut [VALUE] = if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + (*rstruct).as_.ary.as_mut_slice() + } else { + let ptr = (*rstruct).as_.heap.ptr as *mut _; + let len = (*rstruct).as_.heap.len as _; + std::slice::from_raw_parts_mut(ptr, len) + }; + + slice[idx as usize] = value; } #[inline] diff --git a/crates/rb-sys/src/stable_api/ruby_3_1.rs b/crates/rb-sys/src/stable_api/ruby_3_1.rs index 04c8ec7b..c99e31dd 100644 --- a/crates/rb-sys/src/stable_api/ruby_3_1.rs +++ b/crates/rb-sys/src/stable_api/ruby_3_1.rs @@ -329,7 +329,17 @@ impl StableApiDefinition for Definition { #[inline] unsafe fn rstruct_set(&self, st: VALUE, idx: c_int, value: VALUE) { - crate::rb_struct_aset(st, idx as _, value); + let rbasic = st as *const crate::RBasic; + let rstruct = st as *mut RStruct; + let slice: &mut [VALUE] = if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + (*rstruct).as_.ary.as_mut_slice() + } else { + let ptr = (*rstruct).as_.heap.ptr as *mut _; + let len = (*rstruct).as_.heap.len as _; + std::slice::from_raw_parts_mut(ptr, len) + }; + + slice[idx as usize] = value; } #[inline] diff --git a/crates/rb-sys/src/stable_api/ruby_3_2.rs b/crates/rb-sys/src/stable_api/ruby_3_2.rs index e58d8d7f..285a3703 100644 --- a/crates/rb-sys/src/stable_api/ruby_3_2.rs +++ b/crates/rb-sys/src/stable_api/ruby_3_2.rs @@ -328,7 +328,17 @@ impl StableApiDefinition for Definition { #[inline] unsafe fn rstruct_set(&self, st: VALUE, idx: c_int, value: VALUE) { - crate::rb_struct_aset(st, idx as _, value); + let rbasic = st as *const crate::RBasic; + let rstruct = st as *mut RStruct; + let slice: &mut [VALUE] = if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + (*rstruct).as_.ary.as_mut_slice() + } else { + let ptr = (*rstruct).as_.heap.ptr as *mut _; + let len = (*rstruct).as_.heap.len as _; + std::slice::from_raw_parts_mut(ptr, len) + }; + + slice[idx as usize] = value; } #[inline] diff --git a/crates/rb-sys/src/stable_api/ruby_3_3.rs b/crates/rb-sys/src/stable_api/ruby_3_3.rs index f44a8b34..7d0872e2 100644 --- a/crates/rb-sys/src/stable_api/ruby_3_3.rs +++ b/crates/rb-sys/src/stable_api/ruby_3_3.rs @@ -325,9 +325,20 @@ impl StableApiDefinition for Definition { slice[idx as usize] } + // TODO: In CRuby this method has a write barrier. What do we do? #[inline] unsafe fn rstruct_set(&self, st: VALUE, idx: c_int, value: VALUE) { - crate::rb_struct_aset(st, idx as _, value); + let rbasic = st as *const crate::RBasic; + let rstruct = st as *mut RStruct; + let slice: &mut [VALUE] = if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + (*rstruct).as_.ary.as_mut_slice() + } else { + let ptr = (*rstruct).as_.heap.ptr as *mut _; + let len = (*rstruct).as_.heap.len as _; + std::slice::from_raw_parts_mut(ptr, len) + }; + + slice[idx as usize] = value; } #[inline] diff --git a/crates/rb-sys/src/stable_api/ruby_3_4.rs b/crates/rb-sys/src/stable_api/ruby_3_4.rs index 0f0a9701..2c4679ac 100644 --- a/crates/rb-sys/src/stable_api/ruby_3_4.rs +++ b/crates/rb-sys/src/stable_api/ruby_3_4.rs @@ -326,7 +326,17 @@ impl StableApiDefinition for Definition { #[inline] unsafe fn rstruct_set(&self, st: VALUE, idx: c_int, value: VALUE) { - crate::rb_struct_aset(st, idx as _, value); + let rbasic = st as *const crate::RBasic; + let rstruct = st as *mut RStruct; + let slice: &mut [VALUE] = if ((*rbasic).flags & RSTRUCT_EMBED_LEN_MASK as VALUE) != 0 { + (*rstruct).as_.ary.as_mut_slice() + } else { + let ptr = (*rstruct).as_.heap.ptr as *mut _; + let len = (*rstruct).as_.heap.len as _; + std::slice::from_raw_parts_mut(ptr, len) + }; + + slice[idx as usize] = value; } #[inline]