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


##########
include/tvm/node/structural_equal.h:
##########
@@ -198,9 +285,41 @@ class SEqualReducer : public BaseValueEqual {
   /*! \return Get the internal handler. */
   Handler* operator->() const { return handler_; }
 
+  /*! \brief Check if this reducer is tracing paths to the first mismatch. */
+  bool IsPathTracingEnabled() const { return tracing_data_ != nullptr; }
+
+  /*!
+   * \brief Get the paths of the currently compared objects.
+   *
+   * Can only be called when `IsPathTracingEnabled()` is true.
+   */
+  const ObjectPathPair& GetCurrentObjectPaths() const;
+
+  /*!
+   * \brief Specify the object paths of a detected mismatch.
+   *
+   * Can only be called when `IsPathTracingEnabled()` is true.
+   */
+  void RecordMismatchPaths(const ObjectPathPair& paths) const;
+
  private:
+  bool EnumAttrsEqual(int lhs, int rhs, const void* lhs_address, const void* 
rhs_address) const;
+
+  bool ObjectAttrsEqual(const ObjectRef& lhs, const ObjectRef& rhs, bool 
map_free_vars,
+                        const ObjectPathPair* paths) const;
+
+  static void GetPathsFromAttrAddressesAndStoreMismatch(const void* 
lhs_address,
+                                                        const void* 
rhs_address,
+                                                        const PathTracingData* 
tracing_data);
+
+  template <typename T>
+  static bool CompareAttributeValues(const T& lhs, const T& rhs,
+                                     const PathTracingData* tracing_data);

Review Comment:
   I think I mostly understand your point, but I'm missing one thing: why is 
this specific to function templates? For example, the non-template helper 
function `GetPathsFromAttrAddressesAndStoreMismatch` just above is also private 
but we have to put its declaration in the header file, because it is a static 
function in our class (which we need because of C++ visibility rules).
   
   Even if we go with the `SEqualReducerHelper` approach, we still need to leak 
some details in the header file because we need to either declare it as a 
static class or as a friend 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-unsubscr...@tvm.apache.org

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

Reply via email to