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 2021/09/11 07:00:50 UTC
[GitHub] [tvm] ganler commented on a change in pull request #8988: [BugFix] Unexpected crash by tricky parameters in TVM's Python API
ganler commented on a change in pull request #8988:
URL: https://github.com/apache/tvm/pull/8988#discussion_r706573961
##########
File path: include/tvm/runtime/object.h
##########
@@ -703,12 +706,16 @@ struct ObjectPtrEqual {
* \param ParentType The parent type of the objectref
* \param ObjectName The type name of the object.
*/
-#define TVM_DEFINE_OBJECT_REF_METHODS(TypeName, ParentType, ObjectName) \
- TypeName() = default; \
- explicit TypeName(::tvm::runtime::ObjectPtr<::tvm::runtime::Object> n) : ParentType(n) {} \
- TVM_DEFINE_DEFAULT_COPY_MOVE_AND_ASSIGN(TypeName); \
- const ObjectName* operator->() const { return static_cast<const ObjectName*>(data_.get()); } \
- const ObjectName* get() const { return operator->(); } \
+#define TVM_DEFINE_OBJECT_REF_METHODS(TypeName, ParentType, ObjectName) \
+ TypeName() = default; \
+ explicit TypeName(::tvm::runtime::ObjectPtr<::tvm::runtime::Object> n) : ParentType(n) {} \
+ TVM_DEFINE_DEFAULT_COPY_MOVE_AND_ASSIGN(TypeName); \
+ const ObjectName* operator->() const { \
+ auto ptr = static_cast<const ObjectName*>(data_.get()); \
+ ICHECK(nullptr != ptr) << "Calling `->` to <" #ObjectName ">(null)"; \
+ return ptr; \
+ } \
+ const ObjectName* get() const { return static_cast<const ObjectName*>(data_.get()); } \
Review comment:
> TVM nullability by default is allowed by design, and enforcing nullptr checks every time might be contradictory with the assumptions...
BTW, I don't think it violates the assumption. Since here `ptr->SomeThing` is called, crash happens if it's a nullptr, given that the macro here is `TVM_DEFINE_OBJECT_REF_METHODS` not `TVM_DEFINE_**NOTNULLABLE**_OBJECT_REF_METHODS`. The checking here can help convert the crash into a readable exception and this can already fix 95% crash resulted from unintentional parameters.
For `TVM_DEFINE_NOTNULLABLE_OBJECT_REF_METHODS` objects, it is still the same (not violating the assumption by applying redundant checking). :-)
--
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