You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/06/30 20:49:36 UTC

[GitHub] [tvm] gbonik opened a new pull request, #11977: [TVMScript] Add ObjectPath class

gbonik opened a new pull request, #11977:
URL: https://github.com/apache/tvm/pull/11977

   Motivation:
   
   Same IR node object can be referenced in several different contexts inside a larger IR object. For example, a variable could be referenced in several statements within a block.
   
   This makes it impossible to use an object pointer to uniquely identify a "location" within the larger IR object for error reporting purposes. The `ObjectPath` class addresses this problem by serving as a unique "locator".
   
   Tracking issue: https://github.com/apache/tvm/issues/11912


-- 
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: commits-unsubscribe@tvm.apache.org

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

[GitHub] [tvm] junrushao1994 merged pull request #11977: [TVMScript] Add ObjectPath class

Posted by GitBox <gi...@apache.org>.

junrushao1994 merged PR #11977:
URL: https://github.com/apache/tvm/pull/11977


-- 
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: commits-unsubscribe@tvm.apache.org

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

[GitHub] [tvm] gbonik commented on a diff in pull request #11977: [TVMScript] Add ObjectPath class

Posted by GitBox <gi...@apache.org>.

gbonik commented on code in PR #11977:
URL: https://github.com/apache/tvm/pull/11977#discussion_r918253260


##########
include/tvm/node/object_path.h:
##########
@@ -0,0 +1,281 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/node/object_path.h
+ * ObjectPath class that represents a path from a root object to one of its descendants
+ * via attribute access, array indexing etc.
+ */
+
+#ifndef TVM_NODE_OBJECT_PATH_H_
+#define TVM_NODE_OBJECT_PATH_H_
+
+#include <tvm/runtime/container/string.h>
+#include <tvm/runtime/object.h>
+
+#include <string>
+
+namespace tvm {
+
+using runtime::Object;
+using runtime::ObjectPtr;
+using runtime::ObjectRef;
+
+class ObjectPath;
+
+/*!
+ * \brief Path to an object from some root object.
+ *
+ * Motivation:
+ *
+ * Same IR node object can be referenced in several different contexts inside a larger IR object.
+ * For example, a variable could be referenced in several statements within a block.
+ *
+ * This makes it impossible to use an object pointer to uniquely identify a "location" within
+ * the larger IR object for error reporting purposes. The ObjectPath class addresses this problem
+ * by serving as a unique "locator".
+ */
+class ObjectPathNode : public Object {
+ public:
+  /*! \brief Get the parent path */
+  ObjectPath GetParent() const;
+  /*!
+   * \brief Get the length of the path.
+   *
+   * For example, the path returned by `ObjectPath::Root()` has length 1.
+   */
+  size_t Length() const;
+
+  /*!
+   * \brief Get a path prefix of the given length.
+   *
+   * Provided `length` must not exceed the `Length()` of this path.
+   */
+  ObjectPath GetPrefix(size_t length) const;
+
+  /*!
+   * \brief Check if this path is a prefix of another path.
+   *
+   * The prefix is not strict, i.e. a path is considered a prefix of itself.
+   */
+  bool IsPrefixOf(const ObjectPath& other) const;
+
+  /*! \brief Check if two paths are equal. */
+  bool PathsEqual(const ObjectPath& other) const;
+
+  /*! \brief Extend this path with access to an object attribute. */
+  ObjectPath Attr(const char* attr_key);
+
+  /*! \brief Extend this path with access to an object attribute. */
+  ObjectPath Attr(String attr_key);
+
+  /*! \brief Extend this path with access to an array element. */
+  ObjectPath ArrayIndex(size_t index);
+
+  /*! \brief Extend this path with access to a missing array element. */
+  ObjectPath MissingArrayElement(size_t index);
+
+  /*! \brief Extend this path with access to a map value. */
+  ObjectPath MapValue(ObjectRef key);
+
+  /*! \brief Extend this path with access to a missing map entry. */
+  ObjectPath MissingMapEntry();
+
+  static constexpr const char* _type_key = "ObjectPath";
+  TVM_DECLARE_BASE_OBJECT_INFO(ObjectPathNode, Object);
+
+ protected:
+  explicit ObjectPathNode(ObjectPathNode* parent);
+
+  friend class ObjectPath;
+  friend std::string GetObjectPathRepr(const ObjectPathNode* node);
+
+  const ObjectPathNode* ParentNode() const;
+
+  /*! Compares just the last node of the path, without comparing the whole path. */
+  virtual bool LastNodeEqual(const ObjectPathNode* other) const = 0;
+
+  virtual std::string LastNodeString() const = 0;
+
+ private:
+  ObjectRef parent_;
+  size_t length_;
+};
+
+class ObjectPath : public ObjectRef {
+ public:
+  /*! \brief Create a path that represents the root object itself. */
+  static ObjectPath Root();
+
+  TVM_DEFINE_MUTABLE_OBJECT_REF_METHODS(ObjectPath, ObjectRef, ObjectPathNode);

Review Comment:
   Fixed.



-- 
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: commits-unsubscribe@tvm.apache.org

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

[GitHub] [tvm] junrushao1994 commented on a diff in pull request #11977: [TVMScript] Add ObjectPath class

Posted by GitBox <gi...@apache.org>.

junrushao1994 commented on code in PR #11977:
URL: https://github.com/apache/tvm/pull/11977#discussion_r920395236


##########
include/tvm/node/object_path.h:
##########
@@ -0,0 +1,281 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/node/object_path.h
+ * ObjectPath class that represents a path from a root object to one of its descendants
+ * via attribute access, array indexing etc.
+ */
+
+#ifndef TVM_NODE_OBJECT_PATH_H_
+#define TVM_NODE_OBJECT_PATH_H_
+
+#include <tvm/runtime/container/string.h>
+#include <tvm/runtime/object.h>
+
+#include <string>
+
+namespace tvm {
+
+using runtime::Object;
+using runtime::ObjectPtr;
+using runtime::ObjectRef;
+
+class ObjectPath;
+
+/*!
+ * \brief Path to an object from some root object.
+ *
+ * Motivation:
+ *
+ * Same IR node object can be referenced in several different contexts inside a larger IR object.
+ * For example, a variable could be referenced in several statements within a block.
+ *
+ * This makes it impossible to use an object pointer to uniquely identify a "location" within
+ * the larger IR object for error reporting purposes. The ObjectPath class addresses this problem
+ * by serving as a unique "locator".
+ */
+class ObjectPathNode : public Object {
+ public:
+  /*! \brief Get the parent path */
+  ObjectPath GetParent() const;
+  /*!
+   * \brief Get the length of the path.
+   *
+   * For example, the path returned by `ObjectPath::Root()` has length 1.
+   */
+  size_t Length() const;
+
+  /*!
+   * \brief Get a path prefix of the given length.
+   *
+   * Provided `length` must not exceed the `Length()` of this path.
+   */
+  ObjectPath GetPrefix(size_t length) const;
+
+  /*!
+   * \brief Check if this path is a prefix of another path.
+   *
+   * The prefix is not strict, i.e. a path is considered a prefix of itself.
+   */
+  bool IsPrefixOf(const ObjectPath& other) const;
+
+  /*! \brief Check if two paths are equal. */
+  bool PathsEqual(const ObjectPath& other) const;
+
+  /*! \brief Extend this path with access to an object attribute. */
+  ObjectPath Attr(const char* attr_key);
+
+  /*! \brief Extend this path with access to an object attribute. */
+  ObjectPath Attr(String attr_key);
+
+  /*! \brief Extend this path with access to an array element. */
+  ObjectPath ArrayIndex(size_t index);
+
+  /*! \brief Extend this path with access to a missing array element. */
+  ObjectPath MissingArrayElement(size_t index);
+
+  /*! \brief Extend this path with access to a map value. */
+  ObjectPath MapValue(ObjectRef key);
+
+  /*! \brief Extend this path with access to a missing map entry. */
+  ObjectPath MissingMapEntry();
+
+  static constexpr const char* _type_key = "ObjectPath";
+  TVM_DECLARE_BASE_OBJECT_INFO(ObjectPathNode, Object);
+
+ protected:
+  explicit ObjectPathNode(ObjectPathNode* parent);
+
+  friend class ObjectPath;
+  friend std::string GetObjectPathRepr(const ObjectPathNode* node);
+
+  const ObjectPathNode* ParentNode() const;
+
+  /*! Compares just the last node of the path, without comparing the whole path. */
+  virtual bool LastNodeEqual(const ObjectPathNode* other) const = 0;
+
+  virtual std::string LastNodeString() const = 0;
+
+ private:
+  ObjectRef parent_;
+  size_t length_;
+};
+
+class ObjectPath : public ObjectRef {
+ public:
+  /*! \brief Create a path that represents the root object itself. */
+  static ObjectPath Root();
+
+  TVM_DEFINE_MUTABLE_OBJECT_REF_METHODS(ObjectPath, ObjectRef, ObjectPathNode);
+};
+
+//-------------------------------------------------------------------------
+//-----   Concrete object path nodes   ------------------------------------
+//-------------------------------------------------------------------------
+
+// ----- Root -----
+
+class RootPathNode final : public ObjectPathNode {
+ public:
+  explicit RootPathNode();
+
+  static constexpr const char* _type_key = "RootPath";
+  TVM_DECLARE_FINAL_OBJECT_INFO(RootPathNode, ObjectPathNode);
+
+ protected:
+  bool LastNodeEqual(const ObjectPathNode* other) const final;
+  std::string LastNodeString() const final;
+};
+
+class RootPath : public ObjectPath {
+ public:
+  TVM_DEFINE_OBJECT_REF_METHODS(RootPath, ObjectPath, RootPathNode);
+};
+
+// ----- Attribute access -----
+
+class AttributeAccessPathNode final : public ObjectPathNode {
+ public:
+  /*! \brief Name of the attribute being accessed. Must be a static string. */
+  String attr_key;
+
+  explicit AttributeAccessPathNode(ObjectPathNode* parent, String attr_key);

Review Comment:
   Yeah C++ visibility is definitely too weird. I hate it personally but have to trial-and-error for a couple of times with some friend declaration like [this](https://github.com/apache/tvm/blob/main/src/tir/schedule/primitive/compute_inline.cc#L790-L791) lol
   
   > Also, I don't really see how this goes with "non-nullability" thing. For example, if I had some attribute that is a non-nullable ObjectRef, wouldn't it lack the default constructor?
   
   This is painpoint of the current "non-nullability" issue that I talked with @tqchen previously. Currently to work around this issue, we have to use some default initialization that makes some non-nullable objects in a "temporarily-null" state ([example](https://github.com/apache/tvm/blob/7d9a07ccc70eef951bcfff0333c2f82cdc6a3b12/include/tvm/meta_schedule/apply_history_best.h#L50))



-- 
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: commits-unsubscribe@tvm.apache.org

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

[GitHub] [tvm] junrushao1994 commented on pull request #11977: [TVMScript] Add ObjectPath class

Posted by GitBox <gi...@apache.org>.

junrushao1994 commented on PR #11977:
URL: https://github.com/apache/tvm/pull/11977#issuecomment-1171718077

   Also CC: @tqchen 


-- 
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: commits-unsubscribe@tvm.apache.org

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

[GitHub] [tvm] gbonik commented on a diff in pull request #11977: [TVMScript] Add ObjectPath class

Posted by GitBox <gi...@apache.org>.

gbonik commented on code in PR #11977:
URL: https://github.com/apache/tvm/pull/11977#discussion_r918236015


##########
src/node/object_path.cc:
##########
@@ -0,0 +1,322 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/node/object_path.h>
+#include <tvm/node/repr_printer.h>
+#include <tvm/runtime/memory.h>
+#include <tvm/runtime/registry.h>
+
+#include <algorithm>
+#include <cstring>
+
+using namespace tvm::runtime;
+
+namespace tvm {
+
+// ============== ObjectPathNode ==============
+
+ObjectPathNode::ObjectPathNode(ObjectPathNode* parent)
+    : parent_(GetRef<ObjectRef>(parent)), length_(parent == nullptr ? 1 : parent->length_ + 1) {}
+
+// --- GetParent ---
+
+ObjectPath ObjectPathNode::GetParent() const { return Downcast<ObjectPath>(parent_); }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathGetParent").set_body_typed([](const ObjectPath& path) {
+  return path->GetParent();
+});

Review Comment:
   Unfortunately this gives me a compile error:
   
   ```
   include/tvm/runtime/registry.h:201:30: error: cannot declare parameter ‘target’ to be of abstract type ‘tvm::ObjectPathNode’
     201 |     auto fwrap = [f](const T target, Args... params) -> R {
         |                      ~~~~~~~~^~~~~~
   
   ```
   
   I think it may work only if the method was on the `ObjectPath` class.



-- 
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: commits-unsubscribe@tvm.apache.org

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

[GitHub] [tvm] junrushao1994 commented on a diff in pull request #11977: [TVMScript] Add ObjectPath class

Posted by GitBox <gi...@apache.org>.

junrushao1994 commented on code in PR #11977:
URL: https://github.com/apache/tvm/pull/11977#discussion_r920395236


##########
include/tvm/node/object_path.h:
##########
@@ -0,0 +1,281 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/node/object_path.h
+ * ObjectPath class that represents a path from a root object to one of its descendants
+ * via attribute access, array indexing etc.
+ */
+
+#ifndef TVM_NODE_OBJECT_PATH_H_
+#define TVM_NODE_OBJECT_PATH_H_
+
+#include <tvm/runtime/container/string.h>
+#include <tvm/runtime/object.h>
+
+#include <string>
+
+namespace tvm {
+
+using runtime::Object;
+using runtime::ObjectPtr;
+using runtime::ObjectRef;
+
+class ObjectPath;
+
+/*!
+ * \brief Path to an object from some root object.
+ *
+ * Motivation:
+ *
+ * Same IR node object can be referenced in several different contexts inside a larger IR object.
+ * For example, a variable could be referenced in several statements within a block.
+ *
+ * This makes it impossible to use an object pointer to uniquely identify a "location" within
+ * the larger IR object for error reporting purposes. The ObjectPath class addresses this problem
+ * by serving as a unique "locator".
+ */
+class ObjectPathNode : public Object {
+ public:
+  /*! \brief Get the parent path */
+  ObjectPath GetParent() const;
+  /*!
+   * \brief Get the length of the path.
+   *
+   * For example, the path returned by `ObjectPath::Root()` has length 1.
+   */
+  size_t Length() const;
+
+  /*!
+   * \brief Get a path prefix of the given length.
+   *
+   * Provided `length` must not exceed the `Length()` of this path.
+   */
+  ObjectPath GetPrefix(size_t length) const;
+
+  /*!
+   * \brief Check if this path is a prefix of another path.
+   *
+   * The prefix is not strict, i.e. a path is considered a prefix of itself.
+   */
+  bool IsPrefixOf(const ObjectPath& other) const;
+
+  /*! \brief Check if two paths are equal. */
+  bool PathsEqual(const ObjectPath& other) const;
+
+  /*! \brief Extend this path with access to an object attribute. */
+  ObjectPath Attr(const char* attr_key);
+
+  /*! \brief Extend this path with access to an object attribute. */
+  ObjectPath Attr(String attr_key);
+
+  /*! \brief Extend this path with access to an array element. */
+  ObjectPath ArrayIndex(size_t index);
+
+  /*! \brief Extend this path with access to a missing array element. */
+  ObjectPath MissingArrayElement(size_t index);
+
+  /*! \brief Extend this path with access to a map value. */
+  ObjectPath MapValue(ObjectRef key);
+
+  /*! \brief Extend this path with access to a missing map entry. */
+  ObjectPath MissingMapEntry();
+
+  static constexpr const char* _type_key = "ObjectPath";
+  TVM_DECLARE_BASE_OBJECT_INFO(ObjectPathNode, Object);
+
+ protected:
+  explicit ObjectPathNode(ObjectPathNode* parent);
+
+  friend class ObjectPath;
+  friend std::string GetObjectPathRepr(const ObjectPathNode* node);
+
+  const ObjectPathNode* ParentNode() const;
+
+  /*! Compares just the last node of the path, without comparing the whole path. */
+  virtual bool LastNodeEqual(const ObjectPathNode* other) const = 0;
+
+  virtual std::string LastNodeString() const = 0;
+
+ private:
+  ObjectRef parent_;
+  size_t length_;
+};
+
+class ObjectPath : public ObjectRef {
+ public:
+  /*! \brief Create a path that represents the root object itself. */
+  static ObjectPath Root();
+
+  TVM_DEFINE_MUTABLE_OBJECT_REF_METHODS(ObjectPath, ObjectRef, ObjectPathNode);
+};
+
+//-------------------------------------------------------------------------
+//-----   Concrete object path nodes   ------------------------------------
+//-------------------------------------------------------------------------
+
+// ----- Root -----
+
+class RootPathNode final : public ObjectPathNode {
+ public:
+  explicit RootPathNode();
+
+  static constexpr const char* _type_key = "RootPath";
+  TVM_DECLARE_FINAL_OBJECT_INFO(RootPathNode, ObjectPathNode);
+
+ protected:
+  bool LastNodeEqual(const ObjectPathNode* other) const final;
+  std::string LastNodeString() const final;
+};
+
+class RootPath : public ObjectPath {
+ public:
+  TVM_DEFINE_OBJECT_REF_METHODS(RootPath, ObjectPath, RootPathNode);
+};
+
+// ----- Attribute access -----
+
+class AttributeAccessPathNode final : public ObjectPathNode {
+ public:
+  /*! \brief Name of the attribute being accessed. Must be a static string. */
+  String attr_key;
+
+  explicit AttributeAccessPathNode(ObjectPathNode* parent, String attr_key);

Review Comment:
   Yeah C++ visibility is definitely too weird. I hate it personally but have to trial-and-error for a couple of times with some friend declaration like [this](https://github.com/apache/tvm/blob/main/src/tir/schedule/primitive/compute_inline.cc#L790-L791) lol
   
   > Also, I don't really see how this goes with "non-nullability" thing. For example, if I had some attribute that is a non-nullable ObjectRef, wouldn't it lack the default constructor?
   
   This is painpoint of the current "non-nullability" that I talked with @tqchen previously. Currently to work around this issue, we have to use default initialization to make non-nullable objects in a "temporarily-null" state ([example](https://github.com/apache/tvm/blob/7d9a07ccc70eef951bcfff0333c2f82cdc6a3b12/include/tvm/meta_schedule/apply_history_best.h#L50))



-- 
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: commits-unsubscribe@tvm.apache.org

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

[GitHub] [tvm] junrushao1994 commented on a diff in pull request #11977: [TVMScript] Add ObjectPath class

Posted by GitBox <gi...@apache.org>.

junrushao1994 commented on code in PR #11977:
URL: https://github.com/apache/tvm/pull/11977#discussion_r920350872


##########
src/node/object_path.cc:
##########
@@ -0,0 +1,322 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/node/object_path.h>
+#include <tvm/node/repr_printer.h>
+#include <tvm/runtime/memory.h>
+#include <tvm/runtime/registry.h>
+
+#include <algorithm>
+#include <cstring>
+
+using namespace tvm::runtime;
+
+namespace tvm {
+
+// ============== ObjectPathNode ==============
+
+ObjectPathNode::ObjectPathNode(ObjectPathNode* parent)
+    : parent_(GetRef<ObjectRef>(parent)), length_(parent == nullptr ? 1 : parent->length_ + 1) {}
+
+// --- GetParent ---
+
+ObjectPath ObjectPathNode::GetParent() const { return Downcast<ObjectPath>(parent_); }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathGetParent").set_body_typed([](const ObjectPath& path) {
+  return path->GetParent();
+});

Review Comment:
   I just tried and this one works on my machine ™
   
   ```C++
   TVM_REGISTER_GLOBAL("node.ObjectPathGetParent")
       .set_body_method<ObjectPath>(&ObjectPathNode::GetParent);
   ```



-- 
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: commits-unsubscribe@tvm.apache.org

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

[GitHub] [tvm] gbonik commented on a diff in pull request #11977: [TVMScript] Add ObjectPath class

Posted by GitBox <gi...@apache.org>.

gbonik commented on code in PR #11977:
URL: https://github.com/apache/tvm/pull/11977#discussion_r918272273


##########
include/tvm/node/object_path.h:
##########
@@ -0,0 +1,281 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/node/object_path.h
+ * ObjectPath class that represents a path from a root object to one of its descendants
+ * via attribute access, array indexing etc.
+ */
+
+#ifndef TVM_NODE_OBJECT_PATH_H_
+#define TVM_NODE_OBJECT_PATH_H_
+
+#include <tvm/runtime/container/string.h>
+#include <tvm/runtime/object.h>
+
+#include <string>
+
+namespace tvm {
+
+using runtime::Object;
+using runtime::ObjectPtr;
+using runtime::ObjectRef;
+
+class ObjectPath;
+
+/*!
+ * \brief Path to an object from some root object.
+ *
+ * Motivation:
+ *
+ * Same IR node object can be referenced in several different contexts inside a larger IR object.
+ * For example, a variable could be referenced in several statements within a block.
+ *
+ * This makes it impossible to use an object pointer to uniquely identify a "location" within
+ * the larger IR object for error reporting purposes. The ObjectPath class addresses this problem
+ * by serving as a unique "locator".
+ */
+class ObjectPathNode : public Object {
+ public:
+  /*! \brief Get the parent path */
+  ObjectPath GetParent() const;
+  /*!
+   * \brief Get the length of the path.
+   *
+   * For example, the path returned by `ObjectPath::Root()` has length 1.
+   */
+  size_t Length() const;
+
+  /*!
+   * \brief Get a path prefix of the given length.
+   *
+   * Provided `length` must not exceed the `Length()` of this path.
+   */
+  ObjectPath GetPrefix(size_t length) const;
+
+  /*!
+   * \brief Check if this path is a prefix of another path.
+   *
+   * The prefix is not strict, i.e. a path is considered a prefix of itself.
+   */
+  bool IsPrefixOf(const ObjectPath& other) const;
+
+  /*! \brief Check if two paths are equal. */
+  bool PathsEqual(const ObjectPath& other) const;
+
+  /*! \brief Extend this path with access to an object attribute. */
+  ObjectPath Attr(const char* attr_key);
+
+  /*! \brief Extend this path with access to an object attribute. */
+  ObjectPath Attr(String attr_key);
+
+  /*! \brief Extend this path with access to an array element. */
+  ObjectPath ArrayIndex(size_t index);
+
+  /*! \brief Extend this path with access to a missing array element. */
+  ObjectPath MissingArrayElement(size_t index);
+
+  /*! \brief Extend this path with access to a map value. */
+  ObjectPath MapValue(ObjectRef key);
+
+  /*! \brief Extend this path with access to a missing map entry. */
+  ObjectPath MissingMapEntry();
+
+  static constexpr const char* _type_key = "ObjectPath";
+  TVM_DECLARE_BASE_OBJECT_INFO(ObjectPathNode, Object);
+
+ protected:
+  explicit ObjectPathNode(ObjectPathNode* parent);
+
+  friend class ObjectPath;
+  friend std::string GetObjectPathRepr(const ObjectPathNode* node);
+
+  const ObjectPathNode* ParentNode() const;
+
+  /*! Compares just the last node of the path, without comparing the whole path. */
+  virtual bool LastNodeEqual(const ObjectPathNode* other) const = 0;
+
+  virtual std::string LastNodeString() const = 0;
+
+ private:
+  ObjectRef parent_;
+  size_t length_;
+};
+
+class ObjectPath : public ObjectRef {
+ public:
+  /*! \brief Create a path that represents the root object itself. */
+  static ObjectPath Root();
+
+  TVM_DEFINE_MUTABLE_OBJECT_REF_METHODS(ObjectPath, ObjectRef, ObjectPathNode);
+};
+
+//-------------------------------------------------------------------------
+//-----   Concrete object path nodes   ------------------------------------
+//-------------------------------------------------------------------------
+
+// ----- Root -----
+
+class RootPathNode final : public ObjectPathNode {
+ public:
+  explicit RootPathNode();
+
+  static constexpr const char* _type_key = "RootPath";
+  TVM_DECLARE_FINAL_OBJECT_INFO(RootPathNode, ObjectPathNode);
+
+ protected:
+  bool LastNodeEqual(const ObjectPathNode* other) const final;
+  std::string LastNodeString() const final;
+};
+
+class RootPath : public ObjectPath {
+ public:
+  TVM_DEFINE_OBJECT_REF_METHODS(RootPath, ObjectPath, RootPathNode);
+};
+
+// ----- Attribute access -----
+
+class AttributeAccessPathNode final : public ObjectPathNode {
+ public:
+  /*! \brief Name of the attribute being accessed. Must be a static string. */
+  String attr_key;
+
+  explicit AttributeAccessPathNode(ObjectPathNode* parent, String attr_key);

Review Comment:
   This constructor is only meant to be called from `ObjectPath::Attr()` via `make_object`. I would make it private, but I can't, because it is called from the internals of `make_object()` (C++ visibility rules are funny :) )
   
   I guess I could remove this constructor and rely on the default one, and then set the fields after creation. But that would make it a bit more cumbersome, since I'd need to duplicate the logic of the base class (ObjectPathNode) constructor.
   
   Also, I don't really see how this goes with "non-nullability" thing. For example, if I had some attribute that is a non-nullable ObjectRef, wouldn't it lack the default constructor?



-- 
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: commits-unsubscribe@tvm.apache.org

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

[GitHub] [tvm] gbonik commented on a diff in pull request #11977: [TVMScript] Add ObjectPath class

Posted by GitBox <gi...@apache.org>.

gbonik commented on code in PR #11977:
URL: https://github.com/apache/tvm/pull/11977#discussion_r920365203


##########
src/node/object_path.cc:
##########
@@ -0,0 +1,322 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/node/object_path.h>
+#include <tvm/node/repr_printer.h>
+#include <tvm/runtime/memory.h>
+#include <tvm/runtime/registry.h>
+
+#include <algorithm>
+#include <cstring>
+
+using namespace tvm::runtime;
+
+namespace tvm {
+
+// ============== ObjectPathNode ==============
+
+ObjectPathNode::ObjectPathNode(ObjectPathNode* parent)
+    : parent_(GetRef<ObjectRef>(parent)), length_(parent == nullptr ? 1 : parent->length_ + 1) {}
+
+// --- GetParent ---
+
+ObjectPath ObjectPathNode::GetParent() const { return Downcast<ObjectPath>(parent_); }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathGetParent").set_body_typed([](const ObjectPath& path) {
+  return path->GetParent();
+});

Review Comment:
   I think I forgot the explicit template parameter. It works now, thanks!



-- 
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: commits-unsubscribe@tvm.apache.org

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

[GitHub] [tvm] gbonik commented on a diff in pull request #11977: [TVMScript] Add ObjectPath class

Posted by GitBox <gi...@apache.org>.

gbonik commented on code in PR #11977:
URL: https://github.com/apache/tvm/pull/11977#discussion_r918260734


##########
src/node/object_path.cc:
##########
@@ -0,0 +1,322 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/node/object_path.h>
+#include <tvm/node/repr_printer.h>
+#include <tvm/runtime/memory.h>
+#include <tvm/runtime/registry.h>
+
+#include <algorithm>
+#include <cstring>
+
+using namespace tvm::runtime;
+
+namespace tvm {
+
+// ============== ObjectPathNode ==============
+
+ObjectPathNode::ObjectPathNode(ObjectPathNode* parent)
+    : parent_(GetRef<ObjectRef>(parent)), length_(parent == nullptr ? 1 : parent->length_ + 1) {}
+
+// --- GetParent ---
+
+ObjectPath ObjectPathNode::GetParent() const { return Downcast<ObjectPath>(parent_); }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathGetParent").set_body_typed([](const ObjectPath& path) {
+  return path->GetParent();
+});
+
+// --- Length ---
+
+size_t ObjectPathNode::Length() const { return length_; }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathLength").set_body_typed([](const ObjectPath& path) {
+  return static_cast<int64_t>(path->Length());
+});
+
+// --- GetPrefix ---
+
+ObjectPath ObjectPathNode::GetPrefix(size_t length) const {
+  if (length > Length()) {
+    throw std::out_of_range("Attempted to get a prefix longer than the path itself");
+  }
+
+  const ObjectPathNode* node = this;
+  size_t suffix_len = Length() - length;
+  for (size_t i = 0; i < suffix_len; ++i) {
+    node = node->ParentNode();
+  }
+
+  return GetRef<ObjectPath>(node);
+}
+
+TVM_REGISTER_GLOBAL("node.ObjectPathGetPrefix")
+    .set_body_typed([](const ObjectPath& path, int64_t length) {
+      if (length < 0) {
+        throw std::out_of_range("Prefix length can't be negative");
+      }
+      return path->GetPrefix(static_cast<size_t>(length));
+    });
+
+// --- IsPrefixOf ---
+
+bool ObjectPathNode::IsPrefixOf(const ObjectPath& other) const {
+  if (!other.defined()) {
+    return false;
+  }
+
+  size_t this_len = Length();
+  if (this_len > other->Length()) {
+    return false;
+  }
+  return this->PathsEqual(other->GetPrefix(this_len));
+}
+
+TVM_REGISTER_GLOBAL("node.ObjectPathIsPrefixOf")
+    .set_body_typed([](const ObjectPath& a, const ObjectPath& b) { return a->IsPrefixOf(b); });
+
+// --- Attr ---
+
+ObjectPath ObjectPathNode::Attr(const char* attr_key) {
+  if (attr_key != nullptr) {
+    return ObjectPath(make_object<AttributeAccessPathNode>(this, attr_key));
+  } else {
+    return ObjectPath(make_object<UnknownAttributeAccessPathNode>(this));
+  }
+}
+
+ObjectPath ObjectPathNode::Attr(String attr_key) {
+  if (attr_key.defined()) {
+    return ObjectPath(make_object<AttributeAccessPathNode>(this, attr_key));
+  } else {
+    return ObjectPath(make_object<UnknownAttributeAccessPathNode>(this));
+  }
+}

Review Comment:
   Done



-- 
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: commits-unsubscribe@tvm.apache.org

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

[GitHub] [tvm] junrushao1994 commented on a diff in pull request #11977: [TVMScript] Add ObjectPath class

Posted by GitBox <gi...@apache.org>.

junrushao1994 commented on code in PR #11977:
URL: https://github.com/apache/tvm/pull/11977#discussion_r912440070


##########
src/node/object_path.cc:
##########
@@ -0,0 +1,322 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/node/object_path.h>
+#include <tvm/node/repr_printer.h>
+#include <tvm/runtime/memory.h>
+#include <tvm/runtime/registry.h>
+
+#include <algorithm>
+#include <cstring>
+
+using namespace tvm::runtime;
+
+namespace tvm {
+
+// ============== ObjectPathNode ==============
+
+ObjectPathNode::ObjectPathNode(ObjectPathNode* parent)
+    : parent_(GetRef<ObjectRef>(parent)), length_(parent == nullptr ? 1 : parent->length_ + 1) {}
+
+// --- GetParent ---
+
+ObjectPath ObjectPathNode::GetParent() const { return Downcast<ObjectPath>(parent_); }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathGetParent").set_body_typed([](const ObjectPath& path) {
+  return path->GetParent();
+});
+
+// --- Length ---
+
+size_t ObjectPathNode::Length() const { return length_; }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathLength").set_body_typed([](const ObjectPath& path) {
+  return static_cast<int64_t>(path->Length());
+});
+
+// --- GetPrefix ---
+
+ObjectPath ObjectPathNode::GetPrefix(size_t length) const {
+  if (length > Length()) {
+    throw std::out_of_range("Attempted to get a prefix longer than the path itself");
+  }

Review Comment:
   In TVM, we use CHECK (for user-facing errors) and ICHECK (for internal errors) series more often. For example:
   
   ```suggestion
     CHECK_LE(length, Length()) << "IndexError: Attempted to get a prefix longer than the path itself";
   ```
   
   The prefix `IndexError` will be converted automatically to python's native IndexError: https://github.com/apache/tvm/blob/6642c6e8b05534d3c08f8fa2969c31d553aa8476/python/tvm/error.py#L62



##########
src/node/object_path.cc:
##########
@@ -0,0 +1,322 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/node/object_path.h>
+#include <tvm/node/repr_printer.h>
+#include <tvm/runtime/memory.h>
+#include <tvm/runtime/registry.h>
+
+#include <algorithm>
+#include <cstring>
+
+using namespace tvm::runtime;
+
+namespace tvm {
+
+// ============== ObjectPathNode ==============
+
+ObjectPathNode::ObjectPathNode(ObjectPathNode* parent)
+    : parent_(GetRef<ObjectRef>(parent)), length_(parent == nullptr ? 1 : parent->length_ + 1) {}
+
+// --- GetParent ---
+
+ObjectPath ObjectPathNode::GetParent() const { return Downcast<ObjectPath>(parent_); }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathGetParent").set_body_typed([](const ObjectPath& path) {
+  return path->GetParent();
+});
+
+// --- Length ---
+
+size_t ObjectPathNode::Length() const { return length_; }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathLength").set_body_typed([](const ObjectPath& path) {
+  return static_cast<int64_t>(path->Length());
+});
+
+// --- GetPrefix ---
+
+ObjectPath ObjectPathNode::GetPrefix(size_t length) const {
+  if (length > Length()) {
+    throw std::out_of_range("Attempted to get a prefix longer than the path itself");
+  }
+
+  const ObjectPathNode* node = this;
+  size_t suffix_len = Length() - length;
+  for (size_t i = 0; i < suffix_len; ++i) {
+    node = node->ParentNode();
+  }
+
+  return GetRef<ObjectPath>(node);
+}
+
+TVM_REGISTER_GLOBAL("node.ObjectPathGetPrefix")
+    .set_body_typed([](const ObjectPath& path, int64_t length) {
+      if (length < 0) {
+        throw std::out_of_range("Prefix length can't be negative");
+      }
+      return path->GetPrefix(static_cast<size_t>(length));

Review Comment:
   Just I would love to discuss two things here, mostly about styles:
   
   First, besides legacy in the codebase, in new APIs, we strive to make sure the C++ and Python API are consistent. Therefore, usually we do not prefer putting logic in registration to prevent potential discrepancy.
   
   The second is about the usage of `size_t`. It's more like personal preference so I dont have strong opinion: usually I would prefer `int64_t`/`int32_t` to `size_t` if there is no specific need (e.g. large uint, compatibility with STL), because unsigned types could cause some arithmetic issues and `size_t` could be platform-dependent (IIRC).
   
   Therefore, how about we prefer using `int` or `int64_t` throughout `ObjectPath` work if there is no specific need? I would imagine it simplify the implementation and eliminate API discrepancy.



##########
src/node/object_path.cc:
##########
@@ -0,0 +1,322 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/node/object_path.h>
+#include <tvm/node/repr_printer.h>
+#include <tvm/runtime/memory.h>
+#include <tvm/runtime/registry.h>
+
+#include <algorithm>
+#include <cstring>
+
+using namespace tvm::runtime;
+
+namespace tvm {
+
+// ============== ObjectPathNode ==============
+
+ObjectPathNode::ObjectPathNode(ObjectPathNode* parent)
+    : parent_(GetRef<ObjectRef>(parent)), length_(parent == nullptr ? 1 : parent->length_ + 1) {}
+
+// --- GetParent ---
+
+ObjectPath ObjectPathNode::GetParent() const { return Downcast<ObjectPath>(parent_); }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathGetParent").set_body_typed([](const ObjectPath& path) {
+  return path->GetParent();
+});
+
+// --- Length ---
+
+size_t ObjectPathNode::Length() const { return length_; }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathLength").set_body_typed([](const ObjectPath& path) {
+  return static_cast<int64_t>(path->Length());
+});
+
+// --- GetPrefix ---
+
+ObjectPath ObjectPathNode::GetPrefix(size_t length) const {
+  if (length > Length()) {
+    throw std::out_of_range("Attempted to get a prefix longer than the path itself");
+  }
+
+  const ObjectPathNode* node = this;
+  size_t suffix_len = Length() - length;
+  for (size_t i = 0; i < suffix_len; ++i) {
+    node = node->ParentNode();
+  }
+
+  return GetRef<ObjectPath>(node);
+}
+
+TVM_REGISTER_GLOBAL("node.ObjectPathGetPrefix")
+    .set_body_typed([](const ObjectPath& path, int64_t length) {
+      if (length < 0) {
+        throw std::out_of_range("Prefix length can't be negative");
+      }
+      return path->GetPrefix(static_cast<size_t>(length));
+    });
+
+// --- IsPrefixOf ---
+
+bool ObjectPathNode::IsPrefixOf(const ObjectPath& other) const {
+  if (!other.defined()) {
+    return false;
+  }

Review Comment:
   Just curious: when will `other` be null? If there is such cases, in the newer codebase, we would prefer wrap `other` using `Optional<ObjectPath>` to be void-safe, i.e.
   
   ```suggestion
   bool ObjectPathNode::IsPrefixOf(const Optional<ObjectPath>& other) const {
     if (!other.defined()) {
       return false;
     }
   ```
   



##########
include/tvm/node/object_path.h:
##########
@@ -0,0 +1,281 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/node/object_path.h
+ * ObjectPath class that represents a path from a root object to one of its descendants
+ * via attribute access, array indexing etc.
+ */
+
+#ifndef TVM_NODE_OBJECT_PATH_H_
+#define TVM_NODE_OBJECT_PATH_H_
+
+#include <tvm/runtime/container/string.h>
+#include <tvm/runtime/object.h>
+
+#include <string>
+
+namespace tvm {
+
+using runtime::Object;
+using runtime::ObjectPtr;
+using runtime::ObjectRef;
+
+class ObjectPath;
+
+/*!
+ * \brief Path to an object from some root object.
+ *
+ * Motivation:
+ *
+ * Same IR node object can be referenced in several different contexts inside a larger IR object.
+ * For example, a variable could be referenced in several statements within a block.
+ *
+ * This makes it impossible to use an object pointer to uniquely identify a "location" within
+ * the larger IR object for error reporting purposes. The ObjectPath class addresses this problem
+ * by serving as a unique "locator".
+ */
+class ObjectPathNode : public Object {
+ public:
+  /*! \brief Get the parent path */
+  ObjectPath GetParent() const;
+  /*!
+   * \brief Get the length of the path.
+   *
+   * For example, the path returned by `ObjectPath::Root()` has length 1.
+   */
+  size_t Length() const;
+
+  /*!
+   * \brief Get a path prefix of the given length.
+   *
+   * Provided `length` must not exceed the `Length()` of this path.
+   */
+  ObjectPath GetPrefix(size_t length) const;
+
+  /*!
+   * \brief Check if this path is a prefix of another path.
+   *
+   * The prefix is not strict, i.e. a path is considered a prefix of itself.
+   */
+  bool IsPrefixOf(const ObjectPath& other) const;
+
+  /*! \brief Check if two paths are equal. */
+  bool PathsEqual(const ObjectPath& other) const;
+
+  /*! \brief Extend this path with access to an object attribute. */
+  ObjectPath Attr(const char* attr_key);
+
+  /*! \brief Extend this path with access to an object attribute. */
+  ObjectPath Attr(String attr_key);
+
+  /*! \brief Extend this path with access to an array element. */
+  ObjectPath ArrayIndex(size_t index);
+
+  /*! \brief Extend this path with access to a missing array element. */
+  ObjectPath MissingArrayElement(size_t index);
+
+  /*! \brief Extend this path with access to a map value. */
+  ObjectPath MapValue(ObjectRef key);
+
+  /*! \brief Extend this path with access to a missing map entry. */
+  ObjectPath MissingMapEntry();
+
+  static constexpr const char* _type_key = "ObjectPath";
+  TVM_DECLARE_BASE_OBJECT_INFO(ObjectPathNode, Object);
+
+ protected:
+  explicit ObjectPathNode(ObjectPathNode* parent);
+
+  friend class ObjectPath;
+  friend std::string GetObjectPathRepr(const ObjectPathNode* node);
+
+  const ObjectPathNode* ParentNode() const;
+
+  /*! Compares just the last node of the path, without comparing the whole path. */
+  virtual bool LastNodeEqual(const ObjectPathNode* other) const = 0;
+
+  virtual std::string LastNodeString() const = 0;
+
+ private:
+  ObjectRef parent_;
+  size_t length_;
+};
+
+class ObjectPath : public ObjectRef {
+ public:
+  /*! \brief Create a path that represents the root object itself. */
+  static ObjectPath Root();
+
+  TVM_DEFINE_MUTABLE_OBJECT_REF_METHODS(ObjectPath, ObjectRef, ObjectPathNode);
+};
+
+//-------------------------------------------------------------------------
+//-----   Concrete object path nodes   ------------------------------------
+//-------------------------------------------------------------------------
+
+// ----- Root -----
+
+class RootPathNode final : public ObjectPathNode {
+ public:
+  explicit RootPathNode();
+
+  static constexpr const char* _type_key = "RootPath";
+  TVM_DECLARE_FINAL_OBJECT_INFO(RootPathNode, ObjectPathNode);
+
+ protected:
+  bool LastNodeEqual(const ObjectPathNode* other) const final;
+  std::string LastNodeString() const final;
+};
+
+class RootPath : public ObjectPath {
+ public:
+  TVM_DEFINE_OBJECT_REF_METHODS(RootPath, ObjectPath, RootPathNode);
+};
+
+// ----- Attribute access -----
+
+class AttributeAccessPathNode final : public ObjectPathNode {
+ public:
+  /*! \brief Name of the attribute being accessed. Must be a static string. */
+  String attr_key;
+
+  explicit AttributeAccessPathNode(ObjectPathNode* parent, String attr_key);

Review Comment:
   A style issue: in TVM we prefer having constructor in `ObjectRef`s. In our case, it's like:
   
   ```C++
   AttributeAccessPath::AttributeAccessPath(ObjectPathNode* parent, String attr_key) {
     ObjectPtr<AttributeAccessPathNode> n = make_object<AttributeAccessPath>();
     n->parent = parent;
     n->attr_key = std::move(attr_key);
     return AttributeAccessPath(n);
   }
   ```



##########
include/tvm/node/object_path.h:
##########
@@ -0,0 +1,281 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/node/object_path.h
+ * ObjectPath class that represents a path from a root object to one of its descendants
+ * via attribute access, array indexing etc.
+ */
+
+#ifndef TVM_NODE_OBJECT_PATH_H_
+#define TVM_NODE_OBJECT_PATH_H_
+
+#include <tvm/runtime/container/string.h>
+#include <tvm/runtime/object.h>
+
+#include <string>
+
+namespace tvm {
+
+using runtime::Object;
+using runtime::ObjectPtr;
+using runtime::ObjectRef;
+
+class ObjectPath;
+
+/*!
+ * \brief Path to an object from some root object.
+ *
+ * Motivation:
+ *
+ * Same IR node object can be referenced in several different contexts inside a larger IR object.
+ * For example, a variable could be referenced in several statements within a block.
+ *
+ * This makes it impossible to use an object pointer to uniquely identify a "location" within
+ * the larger IR object for error reporting purposes. The ObjectPath class addresses this problem
+ * by serving as a unique "locator".
+ */
+class ObjectPathNode : public Object {
+ public:
+  /*! \brief Get the parent path */
+  ObjectPath GetParent() const;
+  /*!
+   * \brief Get the length of the path.
+   *
+   * For example, the path returned by `ObjectPath::Root()` has length 1.
+   */
+  size_t Length() const;
+
+  /*!
+   * \brief Get a path prefix of the given length.
+   *
+   * Provided `length` must not exceed the `Length()` of this path.
+   */
+  ObjectPath GetPrefix(size_t length) const;
+
+  /*!
+   * \brief Check if this path is a prefix of another path.
+   *
+   * The prefix is not strict, i.e. a path is considered a prefix of itself.
+   */
+  bool IsPrefixOf(const ObjectPath& other) const;
+
+  /*! \brief Check if two paths are equal. */
+  bool PathsEqual(const ObjectPath& other) const;
+
+  /*! \brief Extend this path with access to an object attribute. */
+  ObjectPath Attr(const char* attr_key);
+
+  /*! \brief Extend this path with access to an object attribute. */
+  ObjectPath Attr(String attr_key);
+
+  /*! \brief Extend this path with access to an array element. */
+  ObjectPath ArrayIndex(size_t index);
+
+  /*! \brief Extend this path with access to a missing array element. */
+  ObjectPath MissingArrayElement(size_t index);
+
+  /*! \brief Extend this path with access to a map value. */
+  ObjectPath MapValue(ObjectRef key);
+
+  /*! \brief Extend this path with access to a missing map entry. */
+  ObjectPath MissingMapEntry();
+
+  static constexpr const char* _type_key = "ObjectPath";
+  TVM_DECLARE_BASE_OBJECT_INFO(ObjectPathNode, Object);
+
+ protected:
+  explicit ObjectPathNode(ObjectPathNode* parent);
+
+  friend class ObjectPath;
+  friend std::string GetObjectPathRepr(const ObjectPathNode* node);
+
+  const ObjectPathNode* ParentNode() const;
+
+  /*! Compares just the last node of the path, without comparing the whole path. */
+  virtual bool LastNodeEqual(const ObjectPathNode* other) const = 0;
+
+  virtual std::string LastNodeString() const = 0;
+
+ private:
+  ObjectRef parent_;

Review Comment:
   Let's make it explicit that the `parent_` would be nullptr
   
   ```suggestion
     Optional<ObjectRef> parent_;
   ```



##########
include/tvm/node/object_path.h:
##########
@@ -0,0 +1,281 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/node/object_path.h
+ * ObjectPath class that represents a path from a root object to one of its descendants
+ * via attribute access, array indexing etc.
+ */
+
+#ifndef TVM_NODE_OBJECT_PATH_H_
+#define TVM_NODE_OBJECT_PATH_H_
+
+#include <tvm/runtime/container/string.h>
+#include <tvm/runtime/object.h>
+
+#include <string>
+
+namespace tvm {
+
+using runtime::Object;
+using runtime::ObjectPtr;
+using runtime::ObjectRef;
+
+class ObjectPath;
+
+/*!
+ * \brief Path to an object from some root object.
+ *
+ * Motivation:
+ *
+ * Same IR node object can be referenced in several different contexts inside a larger IR object.
+ * For example, a variable could be referenced in several statements within a block.
+ *
+ * This makes it impossible to use an object pointer to uniquely identify a "location" within
+ * the larger IR object for error reporting purposes. The ObjectPath class addresses this problem
+ * by serving as a unique "locator".
+ */
+class ObjectPathNode : public Object {
+ public:
+  /*! \brief Get the parent path */
+  ObjectPath GetParent() const;
+  /*!
+   * \brief Get the length of the path.
+   *
+   * For example, the path returned by `ObjectPath::Root()` has length 1.
+   */
+  size_t Length() const;
+
+  /*!
+   * \brief Get a path prefix of the given length.
+   *
+   * Provided `length` must not exceed the `Length()` of this path.
+   */
+  ObjectPath GetPrefix(size_t length) const;
+
+  /*!
+   * \brief Check if this path is a prefix of another path.
+   *
+   * The prefix is not strict, i.e. a path is considered a prefix of itself.
+   */
+  bool IsPrefixOf(const ObjectPath& other) const;
+
+  /*! \brief Check if two paths are equal. */
+  bool PathsEqual(const ObjectPath& other) const;
+
+  /*! \brief Extend this path with access to an object attribute. */
+  ObjectPath Attr(const char* attr_key);
+
+  /*! \brief Extend this path with access to an object attribute. */
+  ObjectPath Attr(String attr_key);
+
+  /*! \brief Extend this path with access to an array element. */
+  ObjectPath ArrayIndex(size_t index);
+
+  /*! \brief Extend this path with access to a missing array element. */
+  ObjectPath MissingArrayElement(size_t index);
+
+  /*! \brief Extend this path with access to a map value. */
+  ObjectPath MapValue(ObjectRef key);
+
+  /*! \brief Extend this path with access to a missing map entry. */
+  ObjectPath MissingMapEntry();
+
+  static constexpr const char* _type_key = "ObjectPath";
+  TVM_DECLARE_BASE_OBJECT_INFO(ObjectPathNode, Object);
+
+ protected:
+  explicit ObjectPathNode(ObjectPathNode* parent);
+
+  friend class ObjectPath;
+  friend std::string GetObjectPathRepr(const ObjectPathNode* node);
+
+  const ObjectPathNode* ParentNode() const;
+
+  /*! Compares just the last node of the path, without comparing the whole path. */
+  virtual bool LastNodeEqual(const ObjectPathNode* other) const = 0;
+
+  virtual std::string LastNodeString() const = 0;
+
+ private:
+  ObjectRef parent_;
+  size_t length_;
+};
+
+class ObjectPath : public ObjectRef {
+ public:
+  /*! \brief Create a path that represents the root object itself. */
+  static ObjectPath Root();
+
+  TVM_DEFINE_MUTABLE_OBJECT_REF_METHODS(ObjectPath, ObjectRef, ObjectPathNode);

Review Comment:
   QQ: when will we want to get a mutable pointer to `ObjectPathNode`? If there isn't a case, shall we instead use `TVM_DEFINE_OBJECT_REF_METHODS`?
   
   Also, we increasingly intend to declare objects as "not-nullable" and have an `Optional<>` wrapper to ensure null-safety. In this case, does it make sense if we declare it using `TVM_DEFINE_NOTNULLABLE_OBJECT_REF_METHODS`?



##########
src/node/object_path.cc:
##########
@@ -0,0 +1,322 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/node/object_path.h>
+#include <tvm/node/repr_printer.h>
+#include <tvm/runtime/memory.h>
+#include <tvm/runtime/registry.h>
+
+#include <algorithm>
+#include <cstring>
+
+using namespace tvm::runtime;
+
+namespace tvm {
+
+// ============== ObjectPathNode ==============
+
+ObjectPathNode::ObjectPathNode(ObjectPathNode* parent)
+    : parent_(GetRef<ObjectRef>(parent)), length_(parent == nullptr ? 1 : parent->length_ + 1) {}
+
+// --- GetParent ---
+
+ObjectPath ObjectPathNode::GetParent() const { return Downcast<ObjectPath>(parent_); }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathGetParent").set_body_typed([](const ObjectPath& path) {
+  return path->GetParent();
+});

Review Comment:
   it's possible to simplify the boilerplate here. [Example](https://github.com/apache/tvm/blob/6642c6e8b05534d3c08f8fa2969c31d553aa8476/src/tir/schedule/schedule.cc#L169-L170).
   
   
   ```suggestion
   TVM_REGISTER_GLOBAL("node.ObjectPathGetParent").set_body_method<ObjectPath>(&ObjectPathNode::GetParent);
   ```



##########
src/node/object_path.cc:
##########
@@ -0,0 +1,322 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/node/object_path.h>
+#include <tvm/node/repr_printer.h>
+#include <tvm/runtime/memory.h>
+#include <tvm/runtime/registry.h>
+
+#include <algorithm>
+#include <cstring>
+
+using namespace tvm::runtime;
+
+namespace tvm {
+
+// ============== ObjectPathNode ==============
+
+ObjectPathNode::ObjectPathNode(ObjectPathNode* parent)
+    : parent_(GetRef<ObjectRef>(parent)), length_(parent == nullptr ? 1 : parent->length_ + 1) {}
+
+// --- GetParent ---
+
+ObjectPath ObjectPathNode::GetParent() const { return Downcast<ObjectPath>(parent_); }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathGetParent").set_body_typed([](const ObjectPath& path) {
+  return path->GetParent();
+});
+
+// --- Length ---
+
+size_t ObjectPathNode::Length() const { return length_; }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathLength").set_body_typed([](const ObjectPath& path) {
+  return static_cast<int64_t>(path->Length());
+});
+
+// --- GetPrefix ---
+
+ObjectPath ObjectPathNode::GetPrefix(size_t length) const {
+  if (length > Length()) {
+    throw std::out_of_range("Attempted to get a prefix longer than the path itself");
+  }
+
+  const ObjectPathNode* node = this;
+  size_t suffix_len = Length() - length;
+  for (size_t i = 0; i < suffix_len; ++i) {
+    node = node->ParentNode();
+  }
+
+  return GetRef<ObjectPath>(node);
+}
+
+TVM_REGISTER_GLOBAL("node.ObjectPathGetPrefix")
+    .set_body_typed([](const ObjectPath& path, int64_t length) {
+      if (length < 0) {
+        throw std::out_of_range("Prefix length can't be negative");
+      }
+      return path->GetPrefix(static_cast<size_t>(length));
+    });
+
+// --- IsPrefixOf ---
+
+bool ObjectPathNode::IsPrefixOf(const ObjectPath& other) const {
+  if (!other.defined()) {
+    return false;
+  }
+
+  size_t this_len = Length();
+  if (this_len > other->Length()) {
+    return false;
+  }
+  return this->PathsEqual(other->GetPrefix(this_len));
+}
+
+TVM_REGISTER_GLOBAL("node.ObjectPathIsPrefixOf")
+    .set_body_typed([](const ObjectPath& a, const ObjectPath& b) { return a->IsPrefixOf(b); });
+
+// --- Attr ---
+
+ObjectPath ObjectPathNode::Attr(const char* attr_key) {
+  if (attr_key != nullptr) {
+    return ObjectPath(make_object<AttributeAccessPathNode>(this, attr_key));
+  } else {
+    return ObjectPath(make_object<UnknownAttributeAccessPathNode>(this));
+  }
+}
+
+ObjectPath ObjectPathNode::Attr(String attr_key) {
+  if (attr_key.defined()) {
+    return ObjectPath(make_object<AttributeAccessPathNode>(this, attr_key));
+  } else {
+    return ObjectPath(make_object<UnknownAttributeAccessPathNode>(this));
+  }
+}

Review Comment:
   let's make sure nullable values are handled explicitly
   
   ```suggestion
   ObjectPath ObjectPathNode::Attr(Optional<String> attr_key) {
     if (attr_key.defined()) {
       return ObjectPath(make_object<AttributeAccessPathNode>(this, attr_key.value()));
     } else {
       return ObjectPath(make_object<UnknownAttributeAccessPathNode>(this));
     }
   }
   ```



-- 
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: commits-unsubscribe@tvm.apache.org

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

[GitHub] [tvm] gbonik commented on a diff in pull request #11977: [TVMScript] Add ObjectPath class

Posted by GitBox <gi...@apache.org>.

gbonik commented on code in PR #11977:
URL: https://github.com/apache/tvm/pull/11977#discussion_r918241949


##########
src/node/object_path.cc:
##########
@@ -0,0 +1,322 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/node/object_path.h>
+#include <tvm/node/repr_printer.h>
+#include <tvm/runtime/memory.h>
+#include <tvm/runtime/registry.h>
+
+#include <algorithm>
+#include <cstring>
+
+using namespace tvm::runtime;
+
+namespace tvm {
+
+// ============== ObjectPathNode ==============
+
+ObjectPathNode::ObjectPathNode(ObjectPathNode* parent)
+    : parent_(GetRef<ObjectRef>(parent)), length_(parent == nullptr ? 1 : parent->length_ + 1) {}
+
+// --- GetParent ---
+
+ObjectPath ObjectPathNode::GetParent() const { return Downcast<ObjectPath>(parent_); }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathGetParent").set_body_typed([](const ObjectPath& path) {
+  return path->GetParent();
+});
+
+// --- Length ---
+
+size_t ObjectPathNode::Length() const { return length_; }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathLength").set_body_typed([](const ObjectPath& path) {
+  return static_cast<int64_t>(path->Length());
+});
+
+// --- GetPrefix ---
+
+ObjectPath ObjectPathNode::GetPrefix(size_t length) const {
+  if (length > Length()) {
+    throw std::out_of_range("Attempted to get a prefix longer than the path itself");
+  }
+
+  const ObjectPathNode* node = this;
+  size_t suffix_len = Length() - length;
+  for (size_t i = 0; i < suffix_len; ++i) {
+    node = node->ParentNode();
+  }
+
+  return GetRef<ObjectPath>(node);
+}
+
+TVM_REGISTER_GLOBAL("node.ObjectPathGetPrefix")
+    .set_body_typed([](const ObjectPath& path, int64_t length) {
+      if (length < 0) {
+        throw std::out_of_range("Prefix length can't be negative");
+      }
+      return path->GetPrefix(static_cast<size_t>(length));

Review Comment:
   Sounds good, done



-- 
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: commits-unsubscribe@tvm.apache.org

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

[GitHub] [tvm] gbonik commented on a diff in pull request #11977: [TVMScript] Add ObjectPath class

Posted by GitBox <gi...@apache.org>.

gbonik commented on code in PR #11977:
URL: https://github.com/apache/tvm/pull/11977#discussion_r918239877


##########
src/node/object_path.cc:
##########
@@ -0,0 +1,322 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/node/object_path.h>
+#include <tvm/node/repr_printer.h>
+#include <tvm/runtime/memory.h>
+#include <tvm/runtime/registry.h>
+
+#include <algorithm>
+#include <cstring>
+
+using namespace tvm::runtime;
+
+namespace tvm {
+
+// ============== ObjectPathNode ==============
+
+ObjectPathNode::ObjectPathNode(ObjectPathNode* parent)
+    : parent_(GetRef<ObjectRef>(parent)), length_(parent == nullptr ? 1 : parent->length_ + 1) {}
+
+// --- GetParent ---
+
+ObjectPath ObjectPathNode::GetParent() const { return Downcast<ObjectPath>(parent_); }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathGetParent").set_body_typed([](const ObjectPath& path) {
+  return path->GetParent();
+});
+
+// --- Length ---
+
+size_t ObjectPathNode::Length() const { return length_; }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathLength").set_body_typed([](const ObjectPath& path) {
+  return static_cast<int64_t>(path->Length());
+});
+
+// --- GetPrefix ---
+
+ObjectPath ObjectPathNode::GetPrefix(size_t length) const {
+  if (length > Length()) {
+    throw std::out_of_range("Attempted to get a prefix longer than the path itself");
+  }

Review Comment:
   Done. Didn't realize CHECK macros can throw different exceptions, thanks!



-- 
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: commits-unsubscribe@tvm.apache.org

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

[GitHub] [tvm] gbonik commented on a diff in pull request #11977: [TVMScript] Add ObjectPath class

Posted by GitBox <gi...@apache.org>.

gbonik commented on code in PR #11977:
URL: https://github.com/apache/tvm/pull/11977#discussion_r918257461


##########
src/node/object_path.cc:
##########
@@ -0,0 +1,322 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/node/object_path.h>
+#include <tvm/node/repr_printer.h>
+#include <tvm/runtime/memory.h>
+#include <tvm/runtime/registry.h>
+
+#include <algorithm>
+#include <cstring>
+
+using namespace tvm::runtime;
+
+namespace tvm {
+
+// ============== ObjectPathNode ==============
+
+ObjectPathNode::ObjectPathNode(ObjectPathNode* parent)
+    : parent_(GetRef<ObjectRef>(parent)), length_(parent == nullptr ? 1 : parent->length_ + 1) {}
+
+// --- GetParent ---
+
+ObjectPath ObjectPathNode::GetParent() const { return Downcast<ObjectPath>(parent_); }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathGetParent").set_body_typed([](const ObjectPath& path) {
+  return path->GetParent();
+});
+
+// --- Length ---
+
+size_t ObjectPathNode::Length() const { return length_; }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathLength").set_body_typed([](const ObjectPath& path) {
+  return static_cast<int64_t>(path->Length());
+});
+
+// --- GetPrefix ---
+
+ObjectPath ObjectPathNode::GetPrefix(size_t length) const {
+  if (length > Length()) {
+    throw std::out_of_range("Attempted to get a prefix longer than the path itself");
+  }
+
+  const ObjectPathNode* node = this;
+  size_t suffix_len = Length() - length;
+  for (size_t i = 0; i < suffix_len; ++i) {
+    node = node->ParentNode();
+  }
+
+  return GetRef<ObjectPath>(node);
+}
+
+TVM_REGISTER_GLOBAL("node.ObjectPathGetPrefix")
+    .set_body_typed([](const ObjectPath& path, int64_t length) {
+      if (length < 0) {
+        throw std::out_of_range("Prefix length can't be negative");
+      }
+      return path->GetPrefix(static_cast<size_t>(length));
+    });
+
+// --- IsPrefixOf ---
+
+bool ObjectPathNode::IsPrefixOf(const ObjectPath& other) const {
+  if (!other.defined()) {
+    return false;
+  }

Review Comment:
   Removed null handling



-- 
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: commits-unsubscribe@tvm.apache.org

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

[GitHub] [tvm] junrushao1994 commented on pull request #11977: [TVMScript] Add ObjectPath class

Posted by GitBox <gi...@apache.org>.

junrushao1994 commented on PR #11977:
URL: https://github.com/apache/tvm/pull/11977#issuecomment-1183494608

   BTW @gbonik you may just click "resolve conversation" if my comment is addressed :-) No need to explicitly respond haha


-- 
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: commits-unsubscribe@tvm.apache.org

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

[GitHub] [tvm] junrushao1994 commented on a diff in pull request #11977: [TVMScript] Add ObjectPath class

Posted by GitBox <gi...@apache.org>.

junrushao1994 commented on code in PR #11977:
URL: https://github.com/apache/tvm/pull/11977#discussion_r920375014


##########
src/node/object_path.cc:
##########
@@ -0,0 +1,322 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <tvm/node/object_path.h>
+#include <tvm/node/repr_printer.h>
+#include <tvm/runtime/memory.h>
+#include <tvm/runtime/registry.h>
+
+#include <algorithm>
+#include <cstring>
+
+using namespace tvm::runtime;
+
+namespace tvm {
+
+// ============== ObjectPathNode ==============
+
+ObjectPathNode::ObjectPathNode(ObjectPathNode* parent)
+    : parent_(GetRef<ObjectRef>(parent)), length_(parent == nullptr ? 1 : parent->length_ + 1) {}
+
+// --- GetParent ---
+
+ObjectPath ObjectPathNode::GetParent() const { return Downcast<ObjectPath>(parent_); }
+
+TVM_REGISTER_GLOBAL("node.ObjectPathGetParent").set_body_typed([](const ObjectPath& path) {
+  return path->GetParent();
+});

Review Comment:
   i'm reading the error message you provided - it seems that it's not dispatching to a desirable `set_body_method` method. Yours is [this one](https://github.com/apache/tvm/blob/1cd42e8/include/tvm/runtime/registry.h#L184-L207), while mine is [this](https://github.com/apache/tvm/blob/1cd42e8/include/tvm/runtime/registry.h#L216-L249). Perhaps you forgot to explicitly specify the template instantiation `.set_body_method<ObjectPath>` here?
   



-- 
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: commits-unsubscribe@tvm.apache.org

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

[GitHub] [tvm] junrushao1994 commented on a diff in pull request #11977: [TVMScript] Add ObjectPath class

Posted by GitBox <gi...@apache.org>.

junrushao1994 commented on code in PR #11977:
URL: https://github.com/apache/tvm/pull/11977#discussion_r920395236


##########
include/tvm/node/object_path.h:
##########
@@ -0,0 +1,281 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/node/object_path.h
+ * ObjectPath class that represents a path from a root object to one of its descendants
+ * via attribute access, array indexing etc.
+ */
+
+#ifndef TVM_NODE_OBJECT_PATH_H_
+#define TVM_NODE_OBJECT_PATH_H_
+
+#include <tvm/runtime/container/string.h>
+#include <tvm/runtime/object.h>
+
+#include <string>
+
+namespace tvm {
+
+using runtime::Object;
+using runtime::ObjectPtr;
+using runtime::ObjectRef;
+
+class ObjectPath;
+
+/*!
+ * \brief Path to an object from some root object.
+ *
+ * Motivation:
+ *
+ * Same IR node object can be referenced in several different contexts inside a larger IR object.
+ * For example, a variable could be referenced in several statements within a block.
+ *
+ * This makes it impossible to use an object pointer to uniquely identify a "location" within
+ * the larger IR object for error reporting purposes. The ObjectPath class addresses this problem
+ * by serving as a unique "locator".
+ */
+class ObjectPathNode : public Object {
+ public:
+  /*! \brief Get the parent path */
+  ObjectPath GetParent() const;
+  /*!
+   * \brief Get the length of the path.
+   *
+   * For example, the path returned by `ObjectPath::Root()` has length 1.
+   */
+  size_t Length() const;
+
+  /*!
+   * \brief Get a path prefix of the given length.
+   *
+   * Provided `length` must not exceed the `Length()` of this path.
+   */
+  ObjectPath GetPrefix(size_t length) const;
+
+  /*!
+   * \brief Check if this path is a prefix of another path.
+   *
+   * The prefix is not strict, i.e. a path is considered a prefix of itself.
+   */
+  bool IsPrefixOf(const ObjectPath& other) const;
+
+  /*! \brief Check if two paths are equal. */
+  bool PathsEqual(const ObjectPath& other) const;
+
+  /*! \brief Extend this path with access to an object attribute. */
+  ObjectPath Attr(const char* attr_key);
+
+  /*! \brief Extend this path with access to an object attribute. */
+  ObjectPath Attr(String attr_key);
+
+  /*! \brief Extend this path with access to an array element. */
+  ObjectPath ArrayIndex(size_t index);
+
+  /*! \brief Extend this path with access to a missing array element. */
+  ObjectPath MissingArrayElement(size_t index);
+
+  /*! \brief Extend this path with access to a map value. */
+  ObjectPath MapValue(ObjectRef key);
+
+  /*! \brief Extend this path with access to a missing map entry. */
+  ObjectPath MissingMapEntry();
+
+  static constexpr const char* _type_key = "ObjectPath";
+  TVM_DECLARE_BASE_OBJECT_INFO(ObjectPathNode, Object);
+
+ protected:
+  explicit ObjectPathNode(ObjectPathNode* parent);
+
+  friend class ObjectPath;
+  friend std::string GetObjectPathRepr(const ObjectPathNode* node);
+
+  const ObjectPathNode* ParentNode() const;
+
+  /*! Compares just the last node of the path, without comparing the whole path. */
+  virtual bool LastNodeEqual(const ObjectPathNode* other) const = 0;
+
+  virtual std::string LastNodeString() const = 0;
+
+ private:
+  ObjectRef parent_;
+  size_t length_;
+};
+
+class ObjectPath : public ObjectRef {
+ public:
+  /*! \brief Create a path that represents the root object itself. */
+  static ObjectPath Root();
+
+  TVM_DEFINE_MUTABLE_OBJECT_REF_METHODS(ObjectPath, ObjectRef, ObjectPathNode);
+};
+
+//-------------------------------------------------------------------------
+//-----   Concrete object path nodes   ------------------------------------
+//-------------------------------------------------------------------------
+
+// ----- Root -----
+
+class RootPathNode final : public ObjectPathNode {
+ public:
+  explicit RootPathNode();
+
+  static constexpr const char* _type_key = "RootPath";
+  TVM_DECLARE_FINAL_OBJECT_INFO(RootPathNode, ObjectPathNode);
+
+ protected:
+  bool LastNodeEqual(const ObjectPathNode* other) const final;
+  std::string LastNodeString() const final;
+};
+
+class RootPath : public ObjectPath {
+ public:
+  TVM_DEFINE_OBJECT_REF_METHODS(RootPath, ObjectPath, RootPathNode);
+};
+
+// ----- Attribute access -----
+
+class AttributeAccessPathNode final : public ObjectPathNode {
+ public:
+  /*! \brief Name of the attribute being accessed. Must be a static string. */
+  String attr_key;
+
+  explicit AttributeAccessPathNode(ObjectPathNode* parent, String attr_key);

Review Comment:
   Yeah C++ visibility is definitely too weird. I hate it personally but have to trial-and-error for a couple of times with some friend declaration like [this](https://github.com/apache/tvm/blob/main/src/tir/schedule/primitive/compute_inline.cc#L790-L791) lol
   
   > Also, I don't really see how this goes with "non-nullability" thing. For example, if I had some attribute that is a non-nullable ObjectRef, wouldn't it lack the default constructor?
   
   This is painpoint of the current "non-nullability" that I talked with @tqchen previously. Currently to work around this issue, we have to use some default initialization that makes some non-nullable objects in a "temporarily-null" state ([example](https://github.com/apache/tvm/blob/7d9a07ccc70eef951bcfff0333c2f82cdc6a3b12/include/tvm/meta_schedule/apply_history_best.h#L50))



-- 
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: commits-unsubscribe@tvm.apache.org

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

[GitHub] [tvm] gbonik commented on a diff in pull request #11977: [TVMScript] Add ObjectPath class

Posted by GitBox <gi...@apache.org>.

gbonik commented on code in PR #11977:
URL: https://github.com/apache/tvm/pull/11977#discussion_r918245728


##########
include/tvm/node/object_path.h:
##########
@@ -0,0 +1,281 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file tvm/node/object_path.h
+ * ObjectPath class that represents a path from a root object to one of its descendants
+ * via attribute access, array indexing etc.
+ */
+
+#ifndef TVM_NODE_OBJECT_PATH_H_
+#define TVM_NODE_OBJECT_PATH_H_
+
+#include <tvm/runtime/container/string.h>
+#include <tvm/runtime/object.h>
+
+#include <string>
+
+namespace tvm {
+
+using runtime::Object;
+using runtime::ObjectPtr;
+using runtime::ObjectRef;
+
+class ObjectPath;
+
+/*!
+ * \brief Path to an object from some root object.
+ *
+ * Motivation:
+ *
+ * Same IR node object can be referenced in several different contexts inside a larger IR object.
+ * For example, a variable could be referenced in several statements within a block.
+ *
+ * This makes it impossible to use an object pointer to uniquely identify a "location" within
+ * the larger IR object for error reporting purposes. The ObjectPath class addresses this problem
+ * by serving as a unique "locator".
+ */
+class ObjectPathNode : public Object {
+ public:
+  /*! \brief Get the parent path */
+  ObjectPath GetParent() const;
+  /*!
+   * \brief Get the length of the path.
+   *
+   * For example, the path returned by `ObjectPath::Root()` has length 1.
+   */
+  size_t Length() const;
+
+  /*!
+   * \brief Get a path prefix of the given length.
+   *
+   * Provided `length` must not exceed the `Length()` of this path.
+   */
+  ObjectPath GetPrefix(size_t length) const;
+
+  /*!
+   * \brief Check if this path is a prefix of another path.
+   *
+   * The prefix is not strict, i.e. a path is considered a prefix of itself.
+   */
+  bool IsPrefixOf(const ObjectPath& other) const;
+
+  /*! \brief Check if two paths are equal. */
+  bool PathsEqual(const ObjectPath& other) const;
+
+  /*! \brief Extend this path with access to an object attribute. */
+  ObjectPath Attr(const char* attr_key);
+
+  /*! \brief Extend this path with access to an object attribute. */
+  ObjectPath Attr(String attr_key);
+
+  /*! \brief Extend this path with access to an array element. */
+  ObjectPath ArrayIndex(size_t index);
+
+  /*! \brief Extend this path with access to a missing array element. */
+  ObjectPath MissingArrayElement(size_t index);
+
+  /*! \brief Extend this path with access to a map value. */
+  ObjectPath MapValue(ObjectRef key);
+
+  /*! \brief Extend this path with access to a missing map entry. */
+  ObjectPath MissingMapEntry();
+
+  static constexpr const char* _type_key = "ObjectPath";
+  TVM_DECLARE_BASE_OBJECT_INFO(ObjectPathNode, Object);
+
+ protected:
+  explicit ObjectPathNode(ObjectPathNode* parent);
+
+  friend class ObjectPath;
+  friend std::string GetObjectPathRepr(const ObjectPathNode* node);
+
+  const ObjectPathNode* ParentNode() const;
+
+  /*! Compares just the last node of the path, without comparing the whole path. */
+  virtual bool LastNodeEqual(const ObjectPathNode* other) const = 0;
+
+  virtual std::string LastNodeString() const = 0;
+
+ private:
+  ObjectRef parent_;

Review Comment:
   Done



-- 
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: commits-unsubscribe@tvm.apache.org

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