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/01/22 22:39:27 UTC
[GitHub] [tvm] tkonolige opened a new pull request #7330: [FIX] Improve error messages when array/map types do not match in function calls
tkonolige opened a new pull request #7330:
URL: https://github.com/apache/tvm/pull/7330
This PR fixes error messages that arise when a runtime array does not have the correct element types for a function. For example:
```
# before
Check failed: ObjectTypeChecker<TObjectRef>::Check(ptr) == false: Expect Array[Operation] but get Array
# after
Check failed: ObjectTypeChecker<TObjectRef>::Check(ptr) == false: Expect Array[Operation] but get Array[index 0: Tensor]
```
Note that because runtime `Array`s are not homogenous, specific elements can fail the type checking. I've added which element fails to the errors message along with its type. This can also happen for `Map`s, but I could not figure out a good way to print which element mismatches (because the elements are unordered and I worry printing the key may take too much space).
This is a replacement for #7309 that returns the mismatched type when a runtime object cannot be converted.
@jroesch @tqchen @junrushao1994 @rkimball
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tkonolige commented on pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
tkonolige commented on pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#issuecomment-771862712
@tqchen @junrushao1994 CI is green for this PR now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#issuecomment-766414983
The improvement is super helpful! 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tkonolige commented on a change in pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#discussion_r563867171
##########
File path: src/ir/expr.cc
##########
@@ -49,7 +49,7 @@ PrimExpr PrimExpr::FromObject_(ObjectRef ref) {
if (auto* ptr = ref.as<runtime::StringObj>()) {
return tir::StringImm(GetRef<runtime::String>(ptr));
}
- ICHECK(ObjectTypeChecker<PrimExpr>::Check(ref.get()))
+ ICHECK(!static_cast<bool>(ObjectTypeChecker<PrimExpr>::Mismatch(ref.get())))
<< "Expect type " << ObjectTypeChecker<PrimExpr>::TypeName() << " but get "
<< ref->GetTypeKey();
Review comment:
I'm using `ICHECK(!checked_type.defined())` in other places, so I'll use that 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tkonolige commented on a change in pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#discussion_r563996736
##########
File path: include/tvm/node/container.h
##########
@@ -1449,6 +1449,19 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
+ static Optional<String> CheckAndGetMismatch(const Object* ptr) {
+ if (ptr == nullptr) return NullOpt;
+ if (!ptr->IsInstance<ArrayNode>()) return String(ptr->GetTypeKey());
+ const ArrayNode* n = static_cast<const ArrayNode*>(ptr);
+ for (size_t i = 0; i < n->size(); i++) {
+ const ObjectRef& p = (*n)[i];
+ Optional<String> check_subtype = ObjectTypeChecker<T>::CheckAndGetMismatch(p.get());
+ if (check_subtype.defined()) {
+ return String("Array[index " + std::to_string(i) + ": " + check_subtype.value() + "]");
Review comment:
`Expected Array[Array[OtherType]] but got Array[index 0: Array[index 1: SomeType]]`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7330: [FIX] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#discussion_r563307683
##########
File path: include/tvm/runtime/packed_func.h
##########
@@ -1360,14 +1376,16 @@ inline bool TVMPODValue_::IsObjectRef() const {
}
// NOTE: we don't pass NDArray and runtime::Module as RValue ref.
if (type_code_ == kTVMObjectRValueRefArg) {
- return ObjectTypeChecker<TObjectRef>::Check(*static_cast<Object**>(value_.v_handle));
+ return !static_cast<bool>(
Review comment:
IsObjectRef can be used in code-path that expects multiple argument variants.
```
if (arg.IsObjectRef<T0>()) {
} else {
another type of arg.
}
```
As a result, it is deseriiable to make the else path also fast(and not constructing error messages). Given that the recursive checking logic is not complicated. I think we could have separate impl for both CheckAndGetMismatchMessage and Check.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a change in pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#discussion_r563335299
##########
File path: include/tvm/node/container.h
##########
@@ -1449,31 +1449,41 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<ArrayNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
+ if (!ptr->IsInstance<ArrayNode>()) return Optional<String>(ptr->GetTypeKey());
const ArrayNode* n = static_cast<const ArrayNode*>(ptr);
- for (const ObjectRef& p : *n) {
- if (!ObjectTypeChecker<T>::Check(p.get())) {
- return false;
+ for (size_t i = 0; i < n->size(); i++) {
+ const ObjectRef& p = (*n)[i];
+ auto check_subtype = ObjectTypeChecker<T>::Mismatch(p.get());
+ if (static_cast<bool>(check_subtype)) {
+ return Optional<String>("Array[index " + std::to_string(i) + ": " + check_subtype.value() +
+ "]");
}
}
- return true;
+ return Optional<String>();
}
static std::string TypeName() { return "Array[" + ObjectTypeChecker<T>::TypeName() + "]"; }
};
template <typename K, typename V>
struct ObjectTypeChecker<Map<K, V>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<MapNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
+ if (!ptr->IsInstance<MapNode>()) return Optional<String>(ptr->GetTypeKey());
const MapNode* n = static_cast<const MapNode*>(ptr);
for (const auto& kv : *n) {
- if (!ObjectTypeChecker<K>::Check(kv.first.get())) return false;
- if (!ObjectTypeChecker<V>::Check(kv.second.get())) return false;
+ Optional<String> key_type = ObjectTypeChecker<K>::Mismatch(kv.first.get());
+ Optional<String> value_type = ObjectTypeChecker<K>::Mismatch(kv.first.get());
+ if (static_cast<bool>(key_type) || static_cast<bool>(value_type)) {
Review comment:
yeah I think we should use `.defined()` more, which is more straightforward because it is a word :-)
##########
File path: include/tvm/node/container.h
##########
@@ -1449,31 +1449,41 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<ArrayNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
+ if (!ptr->IsInstance<ArrayNode>()) return Optional<String>(ptr->GetTypeKey());
const ArrayNode* n = static_cast<const ArrayNode*>(ptr);
- for (const ObjectRef& p : *n) {
- if (!ObjectTypeChecker<T>::Check(p.get())) {
- return false;
+ for (size_t i = 0; i < n->size(); i++) {
+ const ObjectRef& p = (*n)[i];
+ auto check_subtype = ObjectTypeChecker<T>::Mismatch(p.get());
+ if (static_cast<bool>(check_subtype)) {
Review comment:
```suggestion
if (check_subtype.defined()) {
```
##########
File path: include/tvm/node/container.h
##########
@@ -1449,31 +1449,41 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<ArrayNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
+ if (!ptr->IsInstance<ArrayNode>()) return Optional<String>(ptr->GetTypeKey());
const ArrayNode* n = static_cast<const ArrayNode*>(ptr);
- for (const ObjectRef& p : *n) {
- if (!ObjectTypeChecker<T>::Check(p.get())) {
- return false;
+ for (size_t i = 0; i < n->size(); i++) {
+ const ObjectRef& p = (*n)[i];
+ auto check_subtype = ObjectTypeChecker<T>::Mismatch(p.get());
Review comment:
Be more explicit about the return type. My (personal) rule of thumb is that if it is not c++ iterator, and the type does not appear on the same line, then I would force myself to write it out
```suggestion
Optional<String> check_subtype = ObjectTypeChecker<T>::Mismatch(p.get());
```
##########
File path: src/ir/expr.cc
##########
@@ -49,7 +49,7 @@ PrimExpr PrimExpr::FromObject_(ObjectRef ref) {
if (auto* ptr = ref.as<runtime::StringObj>()) {
return tir::StringImm(GetRef<runtime::String>(ptr));
}
- ICHECK(ObjectTypeChecker<PrimExpr>::Check(ref.get()))
+ ICHECK(!static_cast<bool>(ObjectTypeChecker<PrimExpr>::Mismatch(ref.get())))
<< "Expect type " << ObjectTypeChecker<PrimExpr>::TypeName() << " but get "
<< ref->GetTypeKey();
Review comment:
ICHECK/CHECK will print out this line, but I don't think it is informative though. We can do this instead. (I wonder if we have an equivalent of `ICHECK` in `LOG(FATAL)`)
```C++
if (Optional<String> err = ObjectTypeChecker<PrimExpr>::Mismatch(ref.get())) {
LOG(FATAL) << "Expect type " << ObjectTypeChecker<PrimExpr>::TypeName()
<< " but get " << ref->GetTypeKey() << ". Full error message: " << err.value();
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tkonolige commented on a change in pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#discussion_r563867171
##########
File path: src/ir/expr.cc
##########
@@ -49,7 +49,7 @@ PrimExpr PrimExpr::FromObject_(ObjectRef ref) {
if (auto* ptr = ref.as<runtime::StringObj>()) {
return tir::StringImm(GetRef<runtime::String>(ptr));
}
- ICHECK(ObjectTypeChecker<PrimExpr>::Check(ref.get()))
+ ICHECK(!static_cast<bool>(ObjectTypeChecker<PrimExpr>::Mismatch(ref.get())))
<< "Expect type " << ObjectTypeChecker<PrimExpr>::TypeName() << " but get "
<< ref->GetTypeKey();
Review comment:
I'm using `ICHECK(!checked_type.defined())` in other places, so I'll use that here.
##########
File path: include/tvm/node/container.h
##########
@@ -1449,6 +1449,19 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
+ static Optional<String> CheckAndGetMismatch(const Object* ptr) {
+ if (ptr == nullptr) return NullOpt;
+ if (!ptr->IsInstance<ArrayNode>()) return String(ptr->GetTypeKey());
+ const ArrayNode* n = static_cast<const ArrayNode*>(ptr);
+ for (size_t i = 0; i < n->size(); i++) {
+ const ObjectRef& p = (*n)[i];
+ Optional<String> check_subtype = ObjectTypeChecker<T>::CheckAndGetMismatch(p.get());
+ if (check_subtype.defined()) {
+ return String("Array[index " + std::to_string(i) + ": " + check_subtype.value() + "]");
Review comment:
`Expected Array[Array[OtherType]] but got Array[index 0: Array[index 1: SomeType]]`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#issuecomment-767910998
I think this PR is good to merge once the CI is green
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#issuecomment-771875728
@tqchen Please take another look and approve explicitly if there is no further issue. I can merge it in only with all reviewers' approval :-)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a change in pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#discussion_r563975690
##########
File path: include/tvm/node/container.h
##########
@@ -1449,6 +1449,19 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
+ static Optional<String> CheckAndGetMismatch(const Object* ptr) {
+ if (ptr == nullptr) return NullOpt;
+ if (!ptr->IsInstance<ArrayNode>()) return String(ptr->GetTypeKey());
+ const ArrayNode* n = static_cast<const ArrayNode*>(ptr);
+ for (size_t i = 0; i < n->size(); i++) {
+ const ObjectRef& p = (*n)[i];
+ Optional<String> check_subtype = ObjectTypeChecker<T>::CheckAndGetMismatch(p.get());
+ if (check_subtype.defined()) {
+ return String("Array[index " + std::to_string(i) + ": " + check_subtype.value() + "]");
Review comment:
Hey, I just wonder, if there is a variable `x` typed as nested `Array<Array<SomeType>>`, and some checking fails on like `x[0][1]`, in this case, what the error message could be?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#issuecomment-766414983
The improvement is super helpful! 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7330: [FIX] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#discussion_r563307342
##########
File path: include/tvm/node/container.h
##########
@@ -1449,31 +1449,41 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<ArrayNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
+ if (!ptr->IsInstance<ArrayNode>()) return Optional<String>(ptr->GetTypeKey());
const ArrayNode* n = static_cast<const ArrayNode*>(ptr);
- for (const ObjectRef& p : *n) {
- if (!ObjectTypeChecker<T>::Check(p.get())) {
- return false;
+ for (size_t i = 0; i < n->size(); i++) {
+ const ObjectRef& p = (*n)[i];
+ auto check_subtype = ObjectTypeChecker<T>::Mismatch(p.get());
+ if (static_cast<bool>(check_subtype)) {
+ return Optional<String>("Array[index " + std::to_string(i) + ": " + check_subtype.value() +
+ "]");
}
}
- return true;
+ return Optional<String>();
}
static std::string TypeName() { return "Array[" + ObjectTypeChecker<T>::TypeName() + "]"; }
};
template <typename K, typename V>
struct ObjectTypeChecker<Map<K, V>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<MapNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
+ if (!ptr->IsInstance<MapNode>()) return Optional<String>(ptr->GetTypeKey());
const MapNode* n = static_cast<const MapNode*>(ptr);
for (const auto& kv : *n) {
- if (!ObjectTypeChecker<K>::Check(kv.first.get())) return false;
- if (!ObjectTypeChecker<V>::Check(kv.second.get())) return false;
+ Optional<String> key_type = ObjectTypeChecker<K>::Mismatch(kv.first.get());
+ Optional<String> value_type = ObjectTypeChecker<K>::Mismatch(kv.first.get());
+ if (static_cast<bool>(key_type) || static_cast<bool>(value_type)) {
Review comment:
Both works
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tkonolige commented on pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
tkonolige commented on pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#issuecomment-771862712
@tqchen @junrushao1994 CI is green for this PR now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7330: [FIX] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#discussion_r563307683
##########
File path: include/tvm/runtime/packed_func.h
##########
@@ -1360,14 +1376,16 @@ inline bool TVMPODValue_::IsObjectRef() const {
}
// NOTE: we don't pass NDArray and runtime::Module as RValue ref.
if (type_code_ == kTVMObjectRValueRefArg) {
- return ObjectTypeChecker<TObjectRef>::Check(*static_cast<Object**>(value_.v_handle));
+ return !static_cast<bool>(
Review comment:
IsObjectRef can be used in code-path that expects multiple argument variants.
```
if (arg.IsObjectRef<T0>()) {
} else {
another type of arg.
}
```
As a result, it is deseriiable to make the else path also fast(and not constructing error messages). Given that the recursive checking logic is not complicated. I think we could have separate impl for both CheckAndGetMismatchMessage and Check.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#issuecomment-771875728
@tqchen Please take another look and approve explicitly if there is no further issue. I can merge it in only with all reviewers' approval :-)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 merged pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
junrushao1994 merged pull request #7330:
URL: https://github.com/apache/tvm/pull/7330
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a change in pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#discussion_r563334497
##########
File path: include/tvm/node/container.h
##########
@@ -1449,31 +1449,41 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<ArrayNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
+ if (!ptr->IsInstance<ArrayNode>()) return Optional<String>(ptr->GetTypeKey());
Review comment:
```suggestion
if (!ptr->IsInstance<ArrayNode>()) return String(ptr->GetTypeKey());
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tkonolige commented on pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
tkonolige commented on pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#issuecomment-766953756
I've addressed all the review comments. I totally missed `Optional::defined`. Its much better than a static_cast. I rename Mismatch to CheckAndGetMismatch and I added the original Checks back in.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a change in pull request #7330: [FIX] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#discussion_r563241667
##########
File path: include/tvm/node/container.h
##########
@@ -1449,31 +1449,41 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<ArrayNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
+ if (!ptr->IsInstance<ArrayNode>()) return Optional<String>(ptr->GetTypeKey());
const ArrayNode* n = static_cast<const ArrayNode*>(ptr);
- for (const ObjectRef& p : *n) {
- if (!ObjectTypeChecker<T>::Check(p.get())) {
- return false;
+ for (size_t i = 0; i < n->size(); i++) {
+ const ObjectRef& p = (*n)[i];
+ auto check_subtype = ObjectTypeChecker<T>::Mismatch(p.get());
+ if (static_cast<bool>(check_subtype)) {
+ return Optional<String>("Array[index " + std::to_string(i) + ": " + check_subtype.value() +
+ "]");
}
}
- return true;
+ return Optional<String>();
}
static std::string TypeName() { return "Array[" + ObjectTypeChecker<T>::TypeName() + "]"; }
};
template <typename K, typename V>
struct ObjectTypeChecker<Map<K, V>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<MapNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
+ if (!ptr->IsInstance<MapNode>()) return Optional<String>(ptr->GetTypeKey());
const MapNode* n = static_cast<const MapNode*>(ptr);
for (const auto& kv : *n) {
- if (!ObjectTypeChecker<K>::Check(kv.first.get())) return false;
- if (!ObjectTypeChecker<V>::Check(kv.second.get())) return false;
+ Optional<String> key_type = ObjectTypeChecker<K>::Mismatch(kv.first.get());
+ Optional<String> value_type = ObjectTypeChecker<K>::Mismatch(kv.first.get());
+ if (static_cast<bool>(key_type) || static_cast<bool>(value_type)) {
Review comment:
Perhaps we should prefer key_type.defined()
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a change in pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#discussion_r564002777
##########
File path: include/tvm/node/container.h
##########
@@ -1449,6 +1449,19 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
+ static Optional<String> CheckAndGetMismatch(const Object* ptr) {
+ if (ptr == nullptr) return NullOpt;
+ if (!ptr->IsInstance<ArrayNode>()) return String(ptr->GetTypeKey());
+ const ArrayNode* n = static_cast<const ArrayNode*>(ptr);
+ for (size_t i = 0; i < n->size(); i++) {
+ const ObjectRef& p = (*n)[i];
+ Optional<String> check_subtype = ObjectTypeChecker<T>::CheckAndGetMismatch(p.get());
+ if (check_subtype.defined()) {
+ return String("Array[index " + std::to_string(i) + ": " + check_subtype.value() + "]");
Review comment:
Nice!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 merged pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
junrushao1994 merged pull request #7330:
URL: https://github.com/apache/tvm/pull/7330
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tkonolige commented on pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
tkonolige commented on pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#issuecomment-766953756
I've addressed all the review comments. I totally missed `Optional::defined`. Its much better than a static_cast. I rename Mismatch to CheckAndGetMismatch and I added the original Checks back in.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7330: [FIX] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#discussion_r563229798
##########
File path: include/tvm/node/container.h
##########
@@ -1449,31 +1449,41 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<ArrayNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
+ if (!ptr->IsInstance<ArrayNode>()) return Optional<String>(ptr->GetTypeKey());
const ArrayNode* n = static_cast<const ArrayNode*>(ptr);
- for (const ObjectRef& p : *n) {
- if (!ObjectTypeChecker<T>::Check(p.get())) {
- return false;
+ for (size_t i = 0; i < n->size(); i++) {
+ const ObjectRef& p = (*n)[i];
+ auto check_subtype = ObjectTypeChecker<T>::Mismatch(p.get());
+ if (static_cast<bool>(check_subtype)) {
+ return Optional<String>("Array[index " + std::to_string(i) + ": " + check_subtype.value() +
+ "]");
}
}
- return true;
+ return Optional<String>();
}
static std::string TypeName() { return "Array[" + ObjectTypeChecker<T>::TypeName() + "]"; }
};
template <typename K, typename V>
struct ObjectTypeChecker<Map<K, V>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<MapNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
+ if (!ptr->IsInstance<MapNode>()) return Optional<String>(ptr->GetTypeKey());
const MapNode* n = static_cast<const MapNode*>(ptr);
for (const auto& kv : *n) {
- if (!ObjectTypeChecker<K>::Check(kv.first.get())) return false;
- if (!ObjectTypeChecker<V>::Check(kv.second.get())) return false;
+ Optional<String> key_type = ObjectTypeChecker<K>::Mismatch(kv.first.get());
+ Optional<String> value_type = ObjectTypeChecker<K>::Mismatch(kv.first.get());
+ if (static_cast<bool>(key_type) || static_cast<bool>(value_type)) {
Review comment:
key_type != nullptr (to be explicit)
##########
File path: include/tvm/node/container.h
##########
@@ -1449,31 +1449,41 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<ArrayNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
+ if (!ptr->IsInstance<ArrayNode>()) return Optional<String>(ptr->GetTypeKey());
const ArrayNode* n = static_cast<const ArrayNode*>(ptr);
- for (const ObjectRef& p : *n) {
- if (!ObjectTypeChecker<T>::Check(p.get())) {
- return false;
+ for (size_t i = 0; i < n->size(); i++) {
+ const ObjectRef& p = (*n)[i];
+ auto check_subtype = ObjectTypeChecker<T>::Mismatch(p.get());
+ if (static_cast<bool>(check_subtype)) {
+ return Optional<String>("Array[index " + std::to_string(i) + ": " + check_subtype.value() +
+ "]");
}
}
- return true;
+ return Optional<String>();
Review comment:
NullOpt
##########
File path: include/tvm/node/container.h
##########
@@ -1449,31 +1449,41 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<ArrayNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
Review comment:
CheckMismatch
##########
File path: include/tvm/node/container.h
##########
@@ -1449,31 +1449,41 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<ArrayNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
+ if (!ptr->IsInstance<ArrayNode>()) return Optional<String>(ptr->GetTypeKey());
const ArrayNode* n = static_cast<const ArrayNode*>(ptr);
- for (const ObjectRef& p : *n) {
- if (!ObjectTypeChecker<T>::Check(p.get())) {
- return false;
+ for (size_t i = 0; i < n->size(); i++) {
+ const ObjectRef& p = (*n)[i];
+ auto check_subtype = ObjectTypeChecker<T>::Mismatch(p.get());
+ if (static_cast<bool>(check_subtype)) {
+ return Optional<String>("Array[index " + std::to_string(i) + ": " + check_subtype.value() +
+ "]");
}
}
- return true;
+ return Optional<String>();
}
static std::string TypeName() { return "Array[" + ObjectTypeChecker<T>::TypeName() + "]"; }
};
template <typename K, typename V>
struct ObjectTypeChecker<Map<K, V>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<MapNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
+ if (!ptr->IsInstance<MapNode>()) return Optional<String>(ptr->GetTypeKey());
const MapNode* n = static_cast<const MapNode*>(ptr);
for (const auto& kv : *n) {
- if (!ObjectTypeChecker<K>::Check(kv.first.get())) return false;
- if (!ObjectTypeChecker<V>::Check(kv.second.get())) return false;
+ Optional<String> key_type = ObjectTypeChecker<K>::Mismatch(kv.first.get());
+ Optional<String> value_type = ObjectTypeChecker<K>::Mismatch(kv.first.get());
+ if (static_cast<bool>(key_type) || static_cast<bool>(value_type)) {
+ std::string key_name = static_cast<bool>(key_type) ? std::string(key_type.value())
+ : ObjectTypeChecker<K>::TypeName();
+ std::string value_name = static_cast<bool>(value_type) ? std::string(value_type.value())
+ : ObjectTypeChecker<V>::TypeName();
+ return Optional<String>("Map[" + key_name + ", " + value_name + "]");
+ }
}
- return true;
+ return Optional<String>();
Review comment:
NullOpt
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a change in pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#discussion_r563334447
##########
File path: include/tvm/node/container.h
##########
@@ -1449,31 +1449,41 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<ArrayNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
Review comment:
```suggestion
if (ptr == nullptr) return NullOpt;
```
##########
File path: include/tvm/node/container.h
##########
@@ -1449,31 +1449,41 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<ArrayNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
+ if (!ptr->IsInstance<ArrayNode>()) return Optional<String>(ptr->GetTypeKey());
Review comment:
```suggestion
if (!ptr->IsInstance<ArrayNode>()) return String(ptr->GetTypeKey());
```
##########
File path: include/tvm/node/container.h
##########
@@ -1449,31 +1449,41 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<ArrayNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
+ if (!ptr->IsInstance<ArrayNode>()) return Optional<String>(ptr->GetTypeKey());
const ArrayNode* n = static_cast<const ArrayNode*>(ptr);
- for (const ObjectRef& p : *n) {
- if (!ObjectTypeChecker<T>::Check(p.get())) {
- return false;
+ for (size_t i = 0; i < n->size(); i++) {
+ const ObjectRef& p = (*n)[i];
+ auto check_subtype = ObjectTypeChecker<T>::Mismatch(p.get());
+ if (static_cast<bool>(check_subtype)) {
+ return Optional<String>("Array[index " + std::to_string(i) + ": " + check_subtype.value() +
+ "]");
}
}
- return true;
+ return Optional<String>();
}
static std::string TypeName() { return "Array[" + ObjectTypeChecker<T>::TypeName() + "]"; }
};
template <typename K, typename V>
struct ObjectTypeChecker<Map<K, V>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<MapNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
+ if (!ptr->IsInstance<MapNode>()) return Optional<String>(ptr->GetTypeKey());
const MapNode* n = static_cast<const MapNode*>(ptr);
for (const auto& kv : *n) {
- if (!ObjectTypeChecker<K>::Check(kv.first.get())) return false;
- if (!ObjectTypeChecker<V>::Check(kv.second.get())) return false;
+ Optional<String> key_type = ObjectTypeChecker<K>::Mismatch(kv.first.get());
+ Optional<String> value_type = ObjectTypeChecker<K>::Mismatch(kv.first.get());
+ if (static_cast<bool>(key_type) || static_cast<bool>(value_type)) {
Review comment:
yeah I think we should use `.defined()` more, which is more straightforward because it is a word :-)
##########
File path: include/tvm/node/container.h
##########
@@ -1449,31 +1449,41 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<ArrayNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
+ if (!ptr->IsInstance<ArrayNode>()) return Optional<String>(ptr->GetTypeKey());
const ArrayNode* n = static_cast<const ArrayNode*>(ptr);
- for (const ObjectRef& p : *n) {
- if (!ObjectTypeChecker<T>::Check(p.get())) {
- return false;
+ for (size_t i = 0; i < n->size(); i++) {
+ const ObjectRef& p = (*n)[i];
+ auto check_subtype = ObjectTypeChecker<T>::Mismatch(p.get());
+ if (static_cast<bool>(check_subtype)) {
Review comment:
```suggestion
if (check_subtype.defined()) {
```
##########
File path: include/tvm/node/container.h
##########
@@ -1449,31 +1449,41 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<ArrayNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
+ if (!ptr->IsInstance<ArrayNode>()) return Optional<String>(ptr->GetTypeKey());
const ArrayNode* n = static_cast<const ArrayNode*>(ptr);
- for (const ObjectRef& p : *n) {
- if (!ObjectTypeChecker<T>::Check(p.get())) {
- return false;
+ for (size_t i = 0; i < n->size(); i++) {
+ const ObjectRef& p = (*n)[i];
+ auto check_subtype = ObjectTypeChecker<T>::Mismatch(p.get());
Review comment:
Be more explicit about the return type. My (personal) rule of thumb is that if it is not c++ iterator, and the type does not appear on the same line, then I would force myself to write it out
```suggestion
Optional<String> check_subtype = ObjectTypeChecker<T>::Mismatch(p.get());
```
##########
File path: src/ir/expr.cc
##########
@@ -49,7 +49,7 @@ PrimExpr PrimExpr::FromObject_(ObjectRef ref) {
if (auto* ptr = ref.as<runtime::StringObj>()) {
return tir::StringImm(GetRef<runtime::String>(ptr));
}
- ICHECK(ObjectTypeChecker<PrimExpr>::Check(ref.get()))
+ ICHECK(!static_cast<bool>(ObjectTypeChecker<PrimExpr>::Mismatch(ref.get())))
<< "Expect type " << ObjectTypeChecker<PrimExpr>::TypeName() << " but get "
<< ref->GetTypeKey();
Review comment:
ICHECK/CHECK will print out this line, but I don't think it is informative though. We can do this instead. (I wonder if we have an equivalent of `ICHECK` in `LOG(FATAL)`)
```C++
if (Optional<String> err = ObjectTypeChecker<PrimExpr>::Mismatch(ref.get())) {
LOG(FATAL) << "Expect type " << ObjectTypeChecker<PrimExpr>::TypeName()
<< " but get " << ref->GetTypeKey() << ". Full error message: " << err.value();
}
```
##########
File path: include/tvm/node/container.h
##########
@@ -1449,6 +1449,19 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
+ static Optional<String> CheckAndGetMismatch(const Object* ptr) {
+ if (ptr == nullptr) return NullOpt;
+ if (!ptr->IsInstance<ArrayNode>()) return String(ptr->GetTypeKey());
+ const ArrayNode* n = static_cast<const ArrayNode*>(ptr);
+ for (size_t i = 0; i < n->size(); i++) {
+ const ObjectRef& p = (*n)[i];
+ Optional<String> check_subtype = ObjectTypeChecker<T>::CheckAndGetMismatch(p.get());
+ if (check_subtype.defined()) {
+ return String("Array[index " + std::to_string(i) + ": " + check_subtype.value() + "]");
Review comment:
Hey, I just wonder, if there is a variable `x` typed as nested `Array<Array<SomeType>>`, and some checking fails on like `x[0][1]`, in this case, what the error message could be?
##########
File path: include/tvm/node/container.h
##########
@@ -1449,6 +1449,19 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
+ static Optional<String> CheckAndGetMismatch(const Object* ptr) {
+ if (ptr == nullptr) return NullOpt;
+ if (!ptr->IsInstance<ArrayNode>()) return String(ptr->GetTypeKey());
+ const ArrayNode* n = static_cast<const ArrayNode*>(ptr);
+ for (size_t i = 0; i < n->size(); i++) {
+ const ObjectRef& p = (*n)[i];
+ Optional<String> check_subtype = ObjectTypeChecker<T>::CheckAndGetMismatch(p.get());
+ if (check_subtype.defined()) {
+ return String("Array[index " + std::to_string(i) + ": " + check_subtype.value() + "]");
Review comment:
Nice!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] tqchen commented on a change in pull request #7330: [FIX] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#discussion_r563229872
##########
File path: include/tvm/node/container.h
##########
@@ -1449,31 +1449,41 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<ArrayNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
Review comment:
How about CheckAndGetMismatchMessage
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a change in pull request #7330: [FFI] Improve error messages when array/map types do not match in function calls
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #7330:
URL: https://github.com/apache/tvm/pull/7330#discussion_r563334447
##########
File path: include/tvm/node/container.h
##########
@@ -1449,31 +1449,41 @@ namespace runtime {
// Additional overloads for PackedFunc checking.
template <typename T>
struct ObjectTypeChecker<Array<T>> {
- static bool Check(const Object* ptr) {
- if (ptr == nullptr) return true;
- if (!ptr->IsInstance<ArrayNode>()) return false;
+ static Optional<String> Mismatch(const Object* ptr) {
+ if (ptr == nullptr) return Optional<String>();
Review comment:
```suggestion
if (ptr == nullptr) return NullOpt;
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org