[GitHub] [incubator-tvm] mwillsey commented on a change in pull request #6384: Add safe up/downcasting to the Rust object system

[GitBox <gi...@apache.org>]

mwillsey commented on a change in pull request #6384:
URL: https://github.com/apache/incubator-tvm/pull/6384#discussion_r482543102



##########
File path: rust/tvm-rt/src/object/object_ptr.rs
##########
@@ -179,14 +172,6 @@ pub struct ObjectPtr<T: IsObject> {
     pub ptr: NonNull<T>,
 }
 
-fn inc_ref<T: IsObject>(ptr: NonNull<T>) {
-    unsafe { ptr.as_ref().as_object().inc_ref() }
-}
-
-fn dec_ref<T: IsObject>(ptr: NonNull<T>) {
-    unsafe { ptr.as_ref().as_object().dec_ref() }
-}
-

Review comment:
       We still do something like this, but it's just in one place now, so I inlined it.

##########
File path: rust/tvm-macros/src/object.rs
##########
@@ -23,56 +23,67 @@ use quote::quote;
 use syn::DeriveInput;
 use syn::Ident;
 
-use crate::util::get_tvm_rt_crate;
+use crate::util::*;
 
 pub fn macro_impl(input: proc_macro::TokenStream) -> TokenStream {
     let tvm_rt_crate = get_tvm_rt_crate();
     let result = quote! { #tvm_rt_crate::function::Result };
     let error = quote! { #tvm_rt_crate::errors::Error };
     let derive_input = syn::parse_macro_input!(input as DeriveInput);
-    let payload_id = derive_input.ident;
-
-    let mut type_key = None;
-    let mut ref_name = None;
-    let base = Some(Ident::new("base", Span::call_site()));
-
-    for attr in derive_input.attrs {
-        if attr.path.is_ident("type_key") {
-            type_key = Some(attr.parse_meta().expect("foo"))
-        }
-
-        if attr.path.is_ident("ref_name") {
-            ref_name = Some(attr.parse_meta().expect("foo"))
-        }
-    }
-
-    let type_key = if let Some(syn::Meta::NameValue(name_value)) = type_key {
-        match name_value.lit {
-            syn::Lit::Str(type_key) => type_key,
-            _ => panic!("foo"),
-        }
-    } else {
-        panic!("bar");
-    };
-
-    let ref_name = if let Some(syn::Meta::NameValue(name_value)) = ref_name {
-        match name_value.lit {
-            syn::Lit::Str(ref_name) => ref_name,
-            _ => panic!("foo"),
-        }
-    } else {
-        panic!("bar");
+    let payload_id = derive_input.ident.clone();
+
+    let type_key = get_attr(&derive_input, "type_key")
+        .map(attr_to_str)
+        .expect("Failed to get type_key");
+
+    let ref_id = get_attr(&derive_input, "ref_name")
+        .map(|a| Ident::new(attr_to_str(a).value().as_str(), Span::call_site()))
+        .unwrap_or_else(|| {
+            let id = payload_id.to_string();
+            let suffixes = ["Node", "Obj"];
+            if let Some(suf) = suffixes
+                .iter()
+                .find(|&suf| id.len() > suf.len() && id.ends_with(suf))
+            {
+                Ident::new(&id[..suf.len() - 4], payload_id.span())

Review comment:
       good catch!




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org