Skip to content

improve Interface::extends and ClassEntry::extends #190

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

Merged
merged 9 commits into from
Apr 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions examples/http-client/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// See the Mulan PSL v2 for more details.

use phper::{
classes::{ClassEntity, ClassEntry},
classes::{ClassEntity, ClassEntry, StateClass},
errors::{Throwable, exception_class},
};

Expand All @@ -19,7 +19,7 @@ const EXCEPTION_CLASS_NAME: &str = "HttpClient\\HttpClientException";
pub fn make_exception_class() -> ClassEntity<()> {
let mut class = ClassEntity::new(EXCEPTION_CLASS_NAME);
// The `extends` is same as the PHP class `extends`.
class.extends(exception_class);
class.extends(StateClass::from_name("Exception"));
class
}

Expand Down
4 changes: 2 additions & 2 deletions examples/http-server/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// See the Mulan PSL v2 for more details.

use phper::{
classes::{ClassEntity, ClassEntry},
classes::{ClassEntity, ClassEntry, StateClass},
errors::{Throwable, exception_class},
};
use std::error::Error;
Expand Down Expand Up @@ -43,6 +43,6 @@ impl From<HttpServerError> for phper::Error {
pub fn make_exception_class() -> ClassEntity<()> {
let mut class = ClassEntity::new(EXCEPTION_CLASS_NAME);
// As an Exception class, inheriting from the base Exception class is important.
class.extends(exception_class);
class.extends(StateClass::from_name("Exception"));
class
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ Now let's begin to finish the logic.

```rust
use phper::{
classes::{ClassEntry, ClassEntity},
classes::{ClassEntry, ClassEntity, StateClass},
errors::{exception_class, Throwable},
};

Expand All @@ -101,7 +101,7 @@ Now let's begin to finish the logic.
pub fn make_exception_class() -> ClassEntity<()> {
let mut class = ClassEntity::new(EXCEPTION_CLASS_NAME);
// The `extends` is same as the PHP class `extends`.
class.extends(exception_class);
class.extends(StateClass::from_name("Exception"));
class
}

Expand Down Expand Up @@ -155,7 +155,7 @@ Now let's begin to finish the logic.
# pub fn make_exception_class() -> ClassEntity<()> {
# let mut class = ClassEntity::new(EXCEPTION_CLASS_NAME);
# // The `extends` is same as the PHP class `extends`.
# class.extends(exception_class);
# class.extends(StateClass::from_name("Exception"));
# class
# }
#
Expand Down
12 changes: 10 additions & 2 deletions phper-doc/doc/_06_module/_06_register_class/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,21 @@ class Foo {}

## Extends & implements

If you want the class `Foo` extends `Bar`, and implements interface `Stringable`:
To allow a class to extend another, you can use `ClassEntity.extends(StateClass<T>)` for classes implemented
in your module. A StateClass can either be obtained by registering your own class against the module, or
by looking up the class by name (for example, for PHP built-in classes like `Exception`).

If you want class `Foo` extends `Bar`, and implements interface `Stringable`:

```rust,no_run
use phper::classes::{ClassEntity, ClassEntry, Interface};
use phper::{modules::Module, php_get_module};

let mut module = Module::new("test", "0.1", "");

let bar = module.add_class(ClassEntity::new("Bar"));
let mut foo = ClassEntity::new("Foo");
foo.extends(|| ClassEntry::from_globals("Bar").unwrap());
foo.extends(bar);
foo.implements(Interface::from_name("Stringable"));
```

Expand Down
7 changes: 3 additions & 4 deletions phper-doc/doc/_06_module/_07_register_interface/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,11 @@ interface Foo {}
If you want the interface `Foo` extends `ArrayAccess` and `Iterator` interfaces.

```rust,no_run
use phper::classes::{InterfaceEntity, ClassEntry};
use phper::classes::{array_access_class, iterator_class};
use phper::classes::{Interface, InterfaceEntity, ClassEntry};

let mut foo = InterfaceEntity::new("Foo");
foo.extends(|| array_access_class());
foo.extends(|| iterator_class());
foo.extends(Interface::from_name("ArrayAccess"));
foo.extends(Interface::from_name("Iterator"));
```

Same as:
Expand Down
85 changes: 58 additions & 27 deletions phper/src/classes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,26 +28,14 @@ use std::{
ffi::{CString, c_char, c_void},
fmt::Debug,
marker::PhantomData,
mem::{ManuallyDrop, replace, size_of, zeroed},
mem::{ManuallyDrop, replace, size_of, transmute, zeroed},
os::raw::c_int,
ptr,
ptr::null_mut,
rc::Rc,
slice,
};

/// Predefined interface `Iterator`.
#[inline]
pub fn iterator_class<'a>() -> &'a ClassEntry {
unsafe { ClassEntry::from_ptr(zend_ce_iterator) }
}

/// Predefined interface `ArrayAccess`.
#[inline]
pub fn array_access_class<'a>() -> &'a ClassEntry {
unsafe { ClassEntry::from_ptr(zend_ce_arrayaccess) }
}

/// Wrapper of [zend_class_entry].
#[derive(Clone)]
#[repr(transparent)]
Expand Down Expand Up @@ -287,13 +275,26 @@ fn find_global_class_entry_ptr(name: impl AsRef<str>) -> *mut zend_class_entry {
/// ```
pub struct StateClass<T> {
inner: Rc<RefCell<*mut zend_class_entry>>,
name: Option<String>,
_p: PhantomData<T>,
}

impl StateClass<()> {
/// Create from name, which will be looked up from globals.
pub fn from_name(name: impl Into<String>) -> Self {
Self {
inner: Rc::new(RefCell::new(null_mut())),
name: Some(name.into()),
_p: PhantomData,
}
}
}

impl<T> StateClass<T> {
fn null() -> Self {
Self {
inner: Rc::new(RefCell::new(null_mut())),
name: None,
_p: PhantomData,
}
}
Expand All @@ -304,7 +305,11 @@ impl<T> StateClass<T> {

/// Converts to class entry.
pub fn as_class_entry(&self) -> &ClassEntry {
unsafe { ClassEntry::from_mut_ptr(*self.inner.borrow()) }
if let Some(name) = &self.name {
ClassEntry::from_globals(name).unwrap()
} else {
unsafe { ClassEntry::from_mut_ptr(*self.inner.borrow()) }
}
}

/// Create the object from class and call `__construct` with arguments.
Expand Down Expand Up @@ -333,6 +338,7 @@ impl<T> Clone for StateClass<T> {
fn clone(&self) -> Self {
Self {
inner: self.inner.clone(),
name: self.name.clone(),
_p: self._p,
}
}
Expand Down Expand Up @@ -427,7 +433,7 @@ pub struct ClassEntity<T: 'static> {
state_constructor: Rc<StateConstructor>,
method_entities: Vec<MethodEntity>,
property_entities: Vec<PropertyEntity>,
parent: Option<Box<dyn Fn() -> &'static ClassEntry>>,
parent: Option<StateClass<()>>,
interfaces: Vec<Interface>,
constants: Vec<ConstantEntity>,
bind_class: StateClass<T>,
Expand Down Expand Up @@ -550,19 +556,40 @@ impl<T: 'static> ClassEntity<T> {
/// Register class to `extends` the parent class.
///
/// *Because in the `MINIT` phase, the class starts to register, so the*
/// *closure is used to return the `ClassEntry` to delay the acquisition of*
/// *`ClassEntry` is looked up by name to delay the acquisition of*
/// *the class.*
///
/// # Examples
///
/// ```no_run
/// use phper::classes::{ClassEntity, ClassEntry};
/// use phper::{
/// classes::{ClassEntity, ClassEntry, StateClass},
/// modules::Module,
/// php_get_module,
/// };
///
/// let mut class = ClassEntity::new("MyException");
/// class.extends(|| ClassEntry::from_globals("Exception").unwrap());
/// #[php_get_module]
/// pub fn get_module() -> Module {
/// let mut module = Module::new(
/// env!("CARGO_CRATE_NAME"),
/// env!("CARGO_PKG_VERSION"),
/// env!("CARGO_PKG_AUTHORS"),
/// );
///
/// let foo = module.add_class(ClassEntity::new("Foo"));
/// let mut bar = ClassEntity::new("Bar");
/// bar.extends(foo);
/// module.add_class(bar);
///
/// let mut ex = ClassEntity::new("MyException");
/// ex.extends(StateClass::from_name("Exception"));
/// module.add_class(ex);
///
/// module
/// }
/// ```
pub fn extends(&mut self, parent: impl Fn() -> &'static ClassEntry + 'static) {
self.parent = Some(Box::new(parent));
pub fn extends<S: 'static>(&mut self, parent: StateClass<S>) {
self.parent = Some(unsafe { transmute::<StateClass<S>, StateClass<()>>(parent) });
Copy link
Preview

Copilot AI Apr 3, 2025

Choose a reason for hiding this comment

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

The use of unsafe transmute here can lead to undefined behavior if the lifetime conversion between StateClass and StateClass<()> is not guaranteed to be safe. Consider using a safer, more explicit conversion method.

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

}

/// Register class to `implements` the interface, due to the class can
Expand Down Expand Up @@ -634,7 +661,7 @@ impl<T: 'static> ClassEntity<T> {
let parent: *mut zend_class_entry = self
.parent
.as_ref()
.map(|parent| parent())
.map(|parent| parent.as_class_entry())
.map(|entry| entry.as_ptr() as *mut _)
.unwrap_or(null_mut());

Expand Down Expand Up @@ -800,20 +827,24 @@ impl InterfaceEntity {
/// Register interface to `extends` the interfaces, due to the interface can
/// extends multi interface, so this method can be called multi time.
///
/// *Because in the `MINIT` phase, the class starts to register, so the*
/// *Because in the `MINIT` phase, the class starts to register, a*
/// *closure is used to return the `ClassEntry` to delay the acquisition of*
/// *the class.*
///
/// # Examples
///
/// ```no_run
/// use phper::classes::{ClassEntry, InterfaceEntity};
/// use phper::classes::{Interface, InterfaceEntity};
///
/// let mut interface = InterfaceEntity::new("MyInterface");
/// interface.extends(|| ClassEntry::from_globals("Stringable").unwrap());
/// interface.extends(Interface::from_name("Stringable"));
/// ```
pub fn extends(&mut self, interface: impl Fn() -> &'static ClassEntry + 'static) {
self.extends.push(Box::new(interface));
pub fn extends(&mut self, interface: Interface) {
self.extends.push(Box::new(move || {
let entry: &'static ClassEntry =
unsafe { std::mem::transmute(interface.as_class_entry()) };
Comment on lines +844 to +845
Copy link
Preview

Copilot AI Apr 3, 2025

Choose a reason for hiding this comment

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

Using unsafe transmute in this context may introduce subtle lifetime issues when converting interface's ClassEntry. It is recommended to explore a safer alternative to handle the conversion explicitly.

Suggested change
let entry: &'static ClassEntry =
unsafe { std::mem::transmute(interface.as_class_entry()) };
let entry: &'static ClassEntry = interface.as_class_entry();

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

entry
}));
}

#[allow(clippy::useless_conversion)]
Expand Down
23 changes: 14 additions & 9 deletions tests/integration/src/classes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@

use phper::{
alloc::RefClone,
classes::{
ClassEntity, ClassEntry, Interface, InterfaceEntity, Visibility, array_access_class,
iterator_class,
},
classes::{ClassEntity, ClassEntry, Interface, InterfaceEntity, StateClass, Visibility},
functions::{Argument, ReturnType},
modules::Module,
types::{ArgumentTypeHint, ReturnTypeHint},
Expand All @@ -23,10 +20,11 @@ use std::{collections::HashMap, convert::Infallible};

pub fn integrate(module: &mut Module) {
integrate_a(module);
integrate_foo(module);
let foo_class = integrate_foo(module);
integrate_i_bar(module);
integrate_static_props(module);
integrate_i_constants(module);
integrate_bar_extends_foo(module, foo_class);
#[cfg(phper_major_version = "8")]
integrate_stringable(module);
}
Expand Down Expand Up @@ -78,7 +76,7 @@ struct Foo {
array: HashMap<i64, ZVal>,
}

fn integrate_foo(module: &mut Module) {
fn integrate_foo(module: &mut Module) -> StateClass<Foo> {
let mut class = ClassEntity::new_with_state_constructor("IntegrationTest\\Foo", || Foo {
position: 0,
array: Default::default(),
Expand Down Expand Up @@ -169,14 +167,14 @@ fn integrate_foo(module: &mut Module) {
.argument(Argument::new("offset").with_type_hint(ArgumentTypeHint::Mixed))
.return_type(ReturnType::new(ReturnTypeHint::Void));

module.add_class(class);
module.add_class(class)
}

fn integrate_i_bar(module: &mut Module) {
let mut interface = InterfaceEntity::new(r"IntegrationTest\IBar");

interface.extends(|| array_access_class());
interface.extends(|| iterator_class());
interface.extends(Interface::from_name("ArrayAccess"));
interface.extends(Interface::from_name("Iterator"));

interface
.add_method("doSomethings")
Expand Down Expand Up @@ -224,6 +222,13 @@ fn integrate_static_props(module: &mut Module) {
module.add_class(class);
}

fn integrate_bar_extends_foo(module: &mut Module, foo_class: StateClass<Foo>) {
let mut cls = ClassEntity::new(r"IntegrationTest\BarExtendsFoo");
cls.extends(foo_class);
cls.add_method("test", Visibility::Public, |_, _| phper::ok(()));
module.add_class(cls);
}

#[cfg(phper_major_version = "8")]
fn integrate_stringable(module: &mut Module) {
use phper::{functions::ReturnType, types::ReturnTypeHint};
Expand Down
5 changes: 5 additions & 0 deletions tests/integration/tests/php/classes.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,3 +100,8 @@ class Foo2 extends IntegrationTest\Foo {}
assert_false(IntegrationTest\IConstants::CST_FALSE);
assert_eq(100, IntegrationTest\IConstants::CST_INT);
assert_eq(10.0, IntegrationTest\IConstants::CST_FLOAT);

// Test module class extends module class
$bar = new \IntegrationTest\BarExtendsFoo; //Bar should extend Foo
$reflection = new ReflectionClass($bar);
assert_true($reflection->isSubclassOf(IntegrationTest\Foo::class));
Loading