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

Add Display impl for ZendObject #74

Merged

Conversation

vodik
Copy link
Collaborator

@vodik vodik commented Sep 20, 2021

Small PR that brings part of my work getting eval/compile support built.

Adds some basic support for capturing PHP exceptions, function calling, and convenience methods around converting from Zval to Rust types.

Which that groundwork in place, add a PHP equivalent implementation for Display.

src/php/types/object.rs Outdated Show resolved Hide resolved
src/php/globals.rs Show resolved Hide resolved
src/php/globals.rs Outdated Show resolved Hide resolved
src/php/types/object.rs Outdated Show resolved Hide resolved
src/php/types/object.rs Outdated Show resolved Hide resolved
@vodik vodik force-pushed the zendobject-display branch 3 times, most recently from b8362c4 to 267a885 Compare September 23, 2021 06:04
@vodik
Copy link
Collaborator Author

vodik commented Sep 23, 2021

Okay, so I think there's a lot of good stuff in here that should get merged in to get iterated on:

  • Zval::extract as a QoL improvment
  • ExecutorGlobals::take_exception
  • FromZendObject/ZendObject::extract for implementing casts from objects.

That said, with all this built, I also think the Display implementation is inappropriate given that it can panic and I think should be dropped from this PR outright.

Thoughts?

/// This is a wrapper function around `TryFrom`.
pub fn extract<T>(self) -> Result<T>
where
T: TryFrom<Self, Error = Error>,
Copy link
Owner

Choose a reason for hiding this comment

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

TryFrom<Zval> is not implemented for all types AFAIK, what about T: FromZval<'a> taking &'a self instead?

Or if you want to drop self then for<'a> T: FromZval<'a> and keep the function parameter as self. The for<'a> just makes it valid to have T as String and not &str (as it would reference the zval after dropping).

@vodik vodik force-pushed the zendobject-display branch from 267a885 to 285620c Compare October 3, 2021 01:22
@vodik vodik force-pushed the zendobject-display branch 2 times, most recently from 405ce25 to fc557e8 Compare October 3, 2021 01:31
@vodik vodik force-pushed the zendobject-display branch from fc557e8 to c5292c4 Compare October 3, 2021 01:32
@davidcole1340 davidcole1340 merged commit 0f2cabb into davidcole1340:master Oct 3, 2021
@vodik vodik deleted the zendobject-display branch October 3, 2021 02:01
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.

2 participants