Review Suggestion: Make rbasic_class return Option<NonNull>>.

oxidize-rb · Dec 6, 2024 · d6f7f13 · d6f7f13
1 parent 883de6c
commit d6f7f13
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 10 deletions.
diff --git a/crates/rb-sys-tests/src/stable_api_test.rs b/crates/rb-sys-tests/src/stable_api_test.rs
@@ -1,4 +1,4 @@
-use rb_sys::StableApiDefinition;
+use rb_sys::{StableApiDefinition, VALUE};
 use rb_sys_test_helpers::rstring as gen_rstring;
 
 macro_rules! parity_test {
@@ -185,10 +185,10 @@ parity_test!(
     name: test_rbasic_class_of_array,
     func: rbasic_class,
     data_factory: {
-        unsafe { rb_sys::rb_ary_new() }
+        unsafe { rb_sys::rb_ary_new() as VALUE }
     },
     expected: {
-        unsafe { rb_sys::rb_cArray }
+        unsafe { Some(std::ptr::NonNull::new_unchecked(rb_sys::rb_cArray as _)) }
     }
 );
 
@@ -198,7 +198,9 @@ parity_test!(
     data_factory: {
       ruby_eval!("[]")
     },
-    expected: ruby_eval!("Array")
+    expected: {
+      unsafe { Some(std::ptr::NonNull::new_unchecked(ruby_eval!("Array") as *mut VALUE)) }
+    }
 );
 
 parity_test!(

diff --git a/crates/rb-sys/src/stable_api/compiled.rs b/crates/rb-sys/src/stable_api/compiled.rs
@@ -1,6 +1,9 @@
 use super::StableApiDefinition;
 use crate::{ruby_value_type, VALUE};
-use std::os::raw::{c_char, c_long};
+use std::{
+    os::raw::{c_char, c_long},
+    ptr::NonNull,
+};
 
 #[allow(dead_code)]
 extern "C" {
@@ -91,8 +94,9 @@ impl StableApiDefinition for Definition {
         impl_rarray_const_ptr(obj)
     }
 
-    unsafe fn rbasic_class(&self, obj: VALUE) -> VALUE {
-        impl_rbasic_class(obj)
+    #[inline]
+    unsafe fn rbasic_class(&self, obj: VALUE) -> Option<NonNull<VALUE>> {
+        NonNull::<VALUE>::new(impl_rbasic_class(obj) as _)
     }
 
     unsafe fn frozen_p(&self, obj: VALUE) -> bool {

diff --git a/crates/rb-sys/src/stable_api/ruby_3_3.rs b/crates/rb-sys/src/stable_api/ruby_3_3.rs
@@ -3,7 +3,10 @@ use crate::{
     internal::{RArray, RString},
     value_type, VALUE,
 };
-use std::os::raw::{c_char, c_long};
+use std::{
+    os::raw::{c_char, c_long},
+    ptr::NonNull,
+};
 
 #[cfg(not(ruby_eq_3_3))]
 compile_error!("This file should only be included in Ruby 3.3 builds");
@@ -77,10 +80,10 @@ impl StableApiDefinition for Definition {
     }
 
     #[inline]
-    unsafe fn rbasic_class(&self, obj: VALUE) -> VALUE {
+    unsafe fn rbasic_class(&self, obj: VALUE) -> Option<NonNull<VALUE>> {
         let rbasic = obj as *const crate::RBasic;
 
-        (*rbasic).klass
+        NonNull::<VALUE>::new((*rbasic).klass as _)
     }
 
     #[inline]