tqchen commented on code in PR #36:
URL: https://github.com/apache/tvm-ffi/pull/36#discussion_r2399645754


##########
include/tvm/ffi/reflection/registry.h:
##########
@@ -25,23 +25,74 @@
 
 #include <tvm/ffi/any.h>
 #include <tvm/ffi/c_api.h>
+#include <tvm/ffi/container/map.h>
+#include <tvm/ffi/container/variant.h>
 #include <tvm/ffi/function.h>
+#include <tvm/ffi/optional.h>
+#include <tvm/ffi/string.h>
 #include <tvm/ffi/type_traits.h>
 
+#include <optional>
+#include <sstream>
 #include <string>
 #include <utility>
+#include <vector>
 
 namespace tvm {
 namespace ffi {
 /*! \brief Reflection namespace */
 namespace reflection {
-
 /*!
  * \brief Trait that can be used to set field info
  * \sa DefaultValue, AttachFieldFlag
  */
 struct FieldInfoTrait {};
 
+/*! \brief User-supplied metadata attached to a field or a method */
+class Metadata : public FieldInfoTrait {
+ public:
+  /*!
+   * \brief Constructor
+   * \param dict The initial dictionary
+   */
+  explicit Metadata(std::initializer_list<std::pair<String, Any>> dict) : 
dict_(dict) {}
+  /*!
+   * \brief Merge this Metadata into the given metadata dictionary
+   * \param info The field info
+   * \param data The metadata to be merged into
+   */
+  TVM_FFI_INLINE void Apply(TVMFFIFieldInfo* info, Metadata* data) {

Review Comment:
   I think we likelyw ant to refctor into `TVMFFIFieldInfo` into a 
reflection::details::FieldInfoBuilder that subclasses the TVMFFIFieldInfo with 
extra metadata, keep Apply taking reflection::details::FieldInfoBuilder. So we 
don't have the second Metadata params, which was artifact of `TVMFFIFieldInfo` 
not able directly contain that info



##########
include/tvm/ffi/reflection/registry.h:
##########
@@ -25,23 +25,74 @@
 
 #include <tvm/ffi/any.h>
 #include <tvm/ffi/c_api.h>
+#include <tvm/ffi/container/map.h>
+#include <tvm/ffi/container/variant.h>
 #include <tvm/ffi/function.h>
+#include <tvm/ffi/optional.h>
+#include <tvm/ffi/string.h>
 #include <tvm/ffi/type_traits.h>
 
+#include <optional>
+#include <sstream>
 #include <string>
 #include <utility>
+#include <vector>
 
 namespace tvm {
 namespace ffi {
 /*! \brief Reflection namespace */
 namespace reflection {
-
 /*!
  * \brief Trait that can be used to set field info
  * \sa DefaultValue, AttachFieldFlag
  */
 struct FieldInfoTrait {};
 
+/*! \brief User-supplied metadata attached to a field or a method */
+class Metadata : public FieldInfoTrait {
+ public:
+  /*!
+   * \brief Constructor
+   * \param dict The initial dictionary
+   */
+  explicit Metadata(std::initializer_list<std::pair<String, Any>> dict) : 
dict_(dict) {}
+  /*!
+   * \brief Merge this Metadata into the given metadata dictionary
+   * \param info The field info
+   * \param data The metadata to be merged into
+   */
+  TVM_FFI_INLINE void Apply(TVMFFIFieldInfo* info, Metadata* data) {
+    this->dict_.insert(this->dict_.end(), data->dict_.begin(), 
data->dict_.end());
+  }
+  /*! \brief Convert the metadata to JSON string */
+  std::string ToJSON() const {
+    std::ostringstream os;
+    os << "{";
+    bool first = true;
+    for (const auto& [key, value] : dict_) {
+      if (!first) {
+        os << ",";
+      }
+      os << "\"" << key << "\":";
+      if (std::optional<int> v = value.as<int>()) {
+        os << *v;
+      } else if (std::optional<bool> v = value.as<bool>()) {
+        os << (*v ? "true" : "false");
+      } else if (const details::StringObj* v = value.as<details::StringObj>()) 
{
+        os << EscapeString(String(*v)).c_str();

Review Comment:
   not sure if this can cause issue given  EscapeString(String(*v)) returns a 
temp variable (and c_str() returns a non-owning view), better to first cache 
that temp variable. 



##########
include/tvm/ffi/reflection/registry.h:
##########
@@ -469,7 +532,9 @@ class ObjectDef : public ReflectionDefBase {
     Function method = GetMethod(std::string(type_key_) + "." + name, 
std::forward<Func>(func));
     info.method = AnyView(method).CopyToTVMFFIAny();
     // apply method info traits
-    ((ApplyMethodInfoTrait(&info, std::forward<Extra>(extra)), ...));
+    ((ApplyMethodInfoTrait(&info, &data, std::forward<Extra>(extra)), ...));
+    std::string all_metadata = data.ToJSON();

Review Comment:
    reflection::details::FieldInfoBuilder.GetMetadata()



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to