Change Throwable macro attribute, and Val drop_value method.

jmjoy · jmjoy · commit 479ae412e481 · 2021-05-28T23:16:11.000+08:00
diff --git a/examples/http-client/src/client.rs b/examples/http-client/src/client.rs
@@ -1,5 +1,7 @@
 use crate::{
-    errors::HttpClientError, replace_and_get, replace_and_set, request::REQUEST_BUILDER_CLASS_NAME,
+    errors::HttpClientError,
+    request::REQUEST_BUILDER_CLASS_NAME,
+    utils::{replace_and_get, replace_and_set},
 };
 use phper::{
     classes::{ClassEntry, DynamicClass, Visibility},
diff --git a/examples/http-client/src/errors.rs b/examples/http-client/src/errors.rs
@@ -5,9 +5,11 @@ use phper::{
 
 const EXCEPTION_CLASS_NAME: &'static str = "HttpClient\\HttpClientException";
 
-#[derive(thiserror::Error, Debug)]
+#[derive(Debug, thiserror::Error, phper::Throwable)]
+#[throwable_class(EXCEPTION_CLASS_NAME)]
 pub enum HttpClientError {
     #[error(transparent)]
+    #[throwable(transparent)]
     Phper(#[from] phper::Error),
 
     #[error(transparent)]
@@ -20,15 +22,6 @@ pub enum HttpClientError {
     ResponseHadRead,
 }
 
-impl Throwable for HttpClientError {
-    fn class_entry(&self) -> &StatelessClassEntry {
-        match self {
-            HttpClientError::Phper(e) => e.class_entry(),
-            _ => ClassEntry::from_globals(EXCEPTION_CLASS_NAME).unwrap(),
-        }
-    }
-}
-
 pub fn make_exception_class() -> DynamicClass<()> {
     let mut exception_class = DynamicClass::new(EXCEPTION_CLASS_NAME);
     exception_class.extends("Exception");
diff --git a/examples/http-client/src/lib.rs b/examples/http-client/src/lib.rs
@@ -5,12 +5,12 @@ use crate::{
     response::make_response_class,
 };
 use phper::{modules::Module, php_get_module};
-use std::mem::replace;
 
 pub mod client;
 pub mod errors;
 pub mod request;
 pub mod response;
+pub mod utils;
 
 #[php_get_module]
 pub fn get_module() -> Module {
@@ -28,12 +28,3 @@ pub fn get_module() -> Module {
 
     module
 }
-
-fn replace_and_set<T>(t: &mut T, init: T, f: impl FnOnce(T) -> T) {
-    let x = f(replace(t, init));
-    let _ = replace(t, x);
-}
-
-fn replace_and_get<T, R>(t: &mut T, init: T, f: impl FnOnce(T) -> R) -> R {
-    f(replace(t, init))
-}
diff --git a/examples/http-client/src/request.rs b/examples/http-client/src/request.rs
@@ -1,4 +1,4 @@
-use crate::{errors::HttpClientError, replace_and_get, response::RESPONSE_CLASS_NAME};
+use crate::{errors::HttpClientError, response::RESPONSE_CLASS_NAME, utils::replace_and_get};
 use phper::{
     classes::{ClassEntry, DynamicClass, Visibility},
     objects::Object,
diff --git a/examples/http-client/src/response.rs b/examples/http-client/src/response.rs
@@ -1,4 +1,4 @@
-use crate::{errors::HttpClientError, replace_and_get};
+use crate::{errors::HttpClientError, utils::replace_and_get};
 use indexmap::map::IndexMap;
 use phper::{
     classes::{DynamicClass, Visibility},
diff --git a/examples/http-client/src/utils.rs b/examples/http-client/src/utils.rs
@@ -0,0 +1,10 @@
+use std::mem::replace;
+
+pub fn replace_and_set<T>(t: &mut T, init: T, f: impl FnOnce(T) -> T) {
+    let x = f(replace(t, init));
+    let _ = replace(t, x);
+}
+
+pub fn replace_and_get<T, R>(t: &mut T, init: T, f: impl FnOnce(T) -> R) -> R {
+    f(replace(t, init))
+}
diff --git a/phper-macros/src/derives.rs b/phper-macros/src/derives.rs
@@ -1,7 +1,7 @@
 use proc_macro::TokenStream;
 use proc_macro2::TokenStream as TokenStream2;
 use quote::quote;
-use syn::{Attribute, Data, DeriveInput, Fields, Meta};
+use syn::{Attribute, Data, DeriveInput, Expr, Fields, Meta, MetaNameValue};
 
 pub(crate) fn derive_throwable(input: DeriveInput) -> syn::Result<TokenStream> {
     let crate_ident = parse_throwable_crate_ident(&input);
@@ -20,27 +20,9 @@ fn parse_throwable_crate_ident(input: &DeriveInput) -> TokenStream2 {
 }
 
 fn parse_throwable_attrs(input: &DeriveInput) -> syn::Result<TokenStream2> {
-    let attr = attributes_find_ident(&input.attrs, "throwable");
-    attr.map(|attr| {
-        attr.parse_args::<Meta>().and_then(|meta| match meta {
-            Meta::NameValue(name_value) => {
-                if !name_value.path.is_ident("class") {
-                    Err(syn::Error::new_spanned(
-                        &attr,
-                        "now only support #[throwable(error = ?)] for enum",
-                    ))
-                } else {
-                    let lit = name_value.lit;
-                    Ok(quote! { #lit })
-                }
-            }
-            _ => Err(syn::Error::new_spanned(
-                &attr,
-                "now only support #[throwable(error = ?)] for enum",
-            )),
-        })
-    })
-    .unwrap_or_else(|| Ok(quote! { "Exception" }))
+    let attr = attributes_find_ident(&input.attrs, "throwable_class");
+    attr.map(|attr| attr.parse_args::<Expr>().map(|expr| quote! { #expr }))
+        .unwrap_or_else(|| Ok(quote! { "Exception" }))
 }
 
 fn parse_throwable_input(
diff --git a/phper-macros/src/lib.rs b/phper-macros/src/lib.rs
@@ -91,7 +91,7 @@ pub fn php_get_module(attr: TokenStream, input: TokenStream) -> TokenStream {
 /// ```
 ///
 /// TODO Support attribute `throwable` with `code` and `message`, integration tests.
-#[proc_macro_derive(Throwable, attributes(throwable, throwable_crate))]
+#[proc_macro_derive(Throwable, attributes(throwable, throwable_class, throwable_crate))]
 pub fn derive_throwable(input: TokenStream) -> TokenStream {
     let input = parse_macro_input!(input as DeriveInput);
     derives::derive_throwable(input).unwrap_or_else(|e| e.into_compile_error().into())
diff --git a/phper-sys/php_wrapper.c b/phper-sys/php_wrapper.c
@@ -160,3 +160,7 @@ bool phper_zend_hash_str_exists(const HashTable *ht, const char *str, size_t len
 bool phper_zend_hash_index_exists(const HashTable *ht, zend_ulong h) {
     return zend_hash_index_exists(ht, h) != 0;
 }
+
+void phper_zval_dtor(zval *zval_ptr) {
+    return zval_dtor(zval_ptr);
+}
diff --git a/phper-sys/php_wrapper.h b/phper-sys/php_wrapper.h
@@ -56,4 +56,6 @@ bool phper_call_user_function(HashTable *function_table, zval *object, zval *fun
 bool phper_zend_hash_str_exists(const HashTable *ht, const char *str, size_t len);
 bool phper_zend_hash_index_exists(const HashTable *ht, zend_ulong h);
 
+void phper_zval_dtor(zval *zval_ptr);
+
 #endif //PHPER_PHP_WRAPPER_H
diff --git a/phper/src/errors.rs b/phper/src/errors.rs
@@ -35,7 +35,7 @@ pub type Result<T> = std::result::Result<T, self::Error>;
 ///
 /// As a php exception, will throw `ErrorException` when the item not implement [Throwable].
 #[derive(thiserror::Error, crate::Throwable, Debug)]
-#[throwable(class = "ErrorException")]
+#[throwable_class("ErrorException")]
 #[throwable_crate]
 pub enum Error {
     #[error(transparent)]
@@ -85,15 +85,15 @@ impl Error {
 
 #[derive(Debug, thiserror::Error, crate::Throwable, Constructor)]
 #[error("type error: {message}")]
-#[throwable(class = "TypeError")]
+#[throwable_class("TypeError")]
 #[throwable_crate]
 pub struct TypeError {
     message: String,
 }
 
 #[derive(Debug, thiserror::Error, crate::Throwable, Constructor)]
 #[error("Class '{class_name}' not found")]
-#[throwable(class = "Error")]
+#[throwable_class("Error")]
 #[throwable_crate]
 pub struct ClassNotFoundError {
     class_name: String,
@@ -104,40 +104,31 @@ pub struct ClassNotFoundError {
     "Actual State type in generic type parameter isn't the state type registered in the class, \
 please confirm the real state type, or use StatelessClassEntry"
 )]
-#[throwable(class = "Error")]
+#[throwable_class("Error")]
 #[throwable_crate]
 pub struct StateTypeError;
 
-#[derive(thiserror::Error, Debug, Constructor)]
+#[derive(Debug, thiserror::Error, crate::Throwable, Constructor)]
 #[error("{function_name}(): expects at least {expect_count} parameter(s), {given_count} given")]
+#[throwable_class(if PHP_VERSION_ID >= 70100 { "ArgumentCountError" } else { "TypeError" })]
+#[throwable_crate]
 pub struct ArgumentCountError {
     function_name: String,
     expect_count: usize,
     given_count: usize,
 }
 
-impl Throwable for ArgumentCountError {
-    fn class_entry(&self) -> &StatelessClassEntry {
-        let class_name = if PHP_VERSION_ID >= 70100 {
-            "ArgumentCountError"
-        } else {
-            "TypeError"
-        };
-        ClassEntry::from_globals(class_name).unwrap()
-    }
-}
-
 #[derive(Debug, thiserror::Error, crate::Throwable, Constructor)]
 #[error("Invalid call to {fn_name}")]
-#[throwable(class = "BadFunctionCallException")]
+#[throwable_class("BadFunctionCallException")]
 #[throwable_crate]
 pub struct CallFunctionError {
     fn_name: String,
 }
 
 #[derive(Debug, thiserror::Error, crate::Throwable, Constructor)]
 #[error("Invalid call to {class_name}::{method_name}")]
-#[throwable(class = "BadMethodCallException")]
+#[throwable_class("BadMethodCallException")]
 #[throwable_crate]
 pub struct CallMethodError {
     class_name: String,
diff --git a/phper/src/values.rs b/phper/src/values.rs
@@ -264,15 +264,7 @@ impl Val {
     }
 
     unsafe fn drop_value(&mut self) {
-        // TODO Use zval_dtor.
-        let t = self.get_type();
-        if t.is_string() {
-            ZendString::free(self.inner.value.str as *mut ZendString);
-        } else if t.is_array() {
-            Array::free(self.inner.value.arr as *mut Array);
-        } else if t.is_object() {
-            Object::free(self.inner.value.obj as *mut StatelessObject);
-        }
+        phper_zval_dtor(self.as_mut_ptr());
     }
 }
 
@@ -295,6 +287,8 @@ impl Drop for Val {
 
 /// The trait for setting the value of [Val], mainly as the return value of
 /// functions and methods, and initializer of [Val].
+///
+/// TODO Better name, distinguish between non-referenced and referenced cases.
 pub trait SetVal {
     unsafe fn set_val(self, val: &mut Val);
 }

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-use crate::{errors::HttpClientError, replace_and_get, response::RESPONSE_CLASS_NAME};`
	`1`	`+use crate::{errors::HttpClientError, response::RESPONSE_CLASS_NAME, utils::replace_and_get};`
`2`	`2`	`use phper::{`
`3`	`3`	`classes::{ClassEntry, DynamicClass, Visibility},`
`4`	`4`	`objects::Object,`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-use crate::{errors::HttpClientError, replace_and_get};`
	`1`	`+use crate::{errors::HttpClientError, utils::replace_and_get};`
`2`	`2`	`use indexmap::map::IndexMap;`
`3`	`3`	`use phper::{`
`4`	`4`	`classes::{DynamicClass, Visibility},`
Original file line number	Diff line number	Diff line change
`@@ -160,3 +160,7 @@ bool phper_zend_hash_str_exists(const HashTable ht, const char str, size_t len`
`160`	`160`	`bool phper_zend_hash_index_exists(const HashTable *ht, zend_ulong h) {`
`161`	`161`	`return zend_hash_index_exists(ht, h) != 0;`
`162`	`162`	`}`
	`163`	`+`
	`164`	`+void phper_zval_dtor(zval *zval_ptr) {`
	`165`	`+ return zval_dtor(zval_ptr);`
	`166`	`+}`