FrozenGene commented on a change in pull request #5753:
URL: https://github.com/apache/incubator-tvm/pull/5753#discussion_r453464508
##########
File path: src/runtime/module.cc
##########
@@ -66,9 +66,19 @@ PackedFunc ModuleNode::GetFunction(const std::string& name,
bool query_imports)
PackedFunc pf = self->GetFunction(name, GetObjectPtr<Object>(this));
if (pf != nullptr) return pf;
if (query_imports) {
- for (Module& m : self->imports_) {
- pf = m->GetFunction(name, m.data_);
- if (pf != nullptr) return pf;
Review comment:
I prefer current fix. As if we add `query_import` flag, we will add it
into the basic core class `module`. However, for basic core class I would love
to keep its current structure, that is every module `GetFunction` is his own
module function. The logic bug I think is here. That is when we have
`query_import` flag, current logic here doesn't iterate every module we
imported, just `self->imports_`
----------------------------------------------------------------
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:
us...@infra.apache.org
