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



##########
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:
       What does this do? It seems like `suf.len() - 4` will only ever be 0 or 
-1.  Should this be something like `id.len() - suf.len()`?

##########
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:
       👍  I never noticed this but presumably you are removing this because 
this is not actually safe?

##########
File path: rust/tvm-rt/src/object/object_ptr.rs
##########
@@ -257,9 +250,7 @@ impl<T: IsObject> ObjectPtr<T> {
 
         if is_derived {
             // NB: self gets dropped here causng a dec ref which we need to 
migtigate with an inc ref before it is dropped.

Review comment:
       I think this comment might be out of date now?




----------------------------------------------------------------
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:
[email protected]


Reply via email to