junrushao1994 commented on a change in pull request #5838: URL: https://github.com/apache/incubator-tvm/pull/5838#discussion_r443085597
########## File path: src/target/target_id.cc ########## @@ -91,38 +91,74 @@ void VerifyTypeInfo(const ObjectRef& obj, const TargetIdNode::ValueTypeInfo& inf } } -TVM_DLL void TargetIdNode::ValidateSchema(const Map<String, ObjectRef>& config) const { +void TargetIdNode::ValidateSchema(const Map<String, ObjectRef>& config) const { + const String kTargetId = "id"; for (const auto& kv : config) { - auto it = key2vtype_.find(kv.first); + const String& name = kv.first; + const ObjectRef& obj = kv.second; + if (name == kTargetId) { + CHECK(obj->IsInstance<StringObj>()) + << "AttributeError: \"id\" is not a string, but its type is " << obj->GetTypeKey(); + CHECK(Downcast<String>(obj) == this->name) + << "AttributeError: \"id\" = " << obj << " is inconsistent with TargetId " << this->name; + continue; + } + auto it = key2vtype_.find(name); if (it == key2vtype_.end()) { std::ostringstream os; - os << "AttributeError: Invalid config option, cannot recognize \'" << kv.first - << "\' candidates are:"; - bool is_first = true; + os << "AttributeError: Invalid config option, cannot recognize \'" << name + << "\'. Candidates are:"; for (const auto& kv : key2vtype_) { - if (is_first) { - is_first = false; - } else { - os << ','; - } - os << ' ' << kv.first; + os << "\n " << kv.first; } LOG(FATAL) << os.str(); throw; } - const auto& obj = kv.second; const auto& info = it->second; try { VerifyTypeInfo(obj, info); } catch (const tvm::Error& e) { - LOG(FATAL) << "AttributeError: Schema validation failed for TargetId " << name + LOG(FATAL) << "AttributeError: Schema validation failed for TargetId " << this->name << ", details:\n" << e.what() << "\n" - << "The given config is:\n" + << "The config is:\n" << config; throw; } } } +inline String GetId(const Map<String, ObjectRef>& target, const char* name) { + const String kTargetId = "id"; + CHECK(target.count(kTargetId)) << "AttributeError: \"id\" does not exist in " << name << "\n" + << name << " = " << target; + const ObjectRef& obj = target[kTargetId]; + CHECK(obj->IsInstance<StringObj>()) << "AttributeError: \"id\" is not a string in " << name + << ", but its type is " << obj->GetTypeKey() << "\n" + << name << " = " << target; + return Downcast<String>(obj); +} + +void TargetValidateSchema(const Map<String, ObjectRef>& config) { + try { + const String kTargetHost = "target_host"; + Map<String, ObjectRef> target = config; + Map<String, ObjectRef> target_host; + String target_id = GetId(target, "target"); + String target_host_id; + if (config.count(kTargetHost)) { + target.erase(kTargetHost); + target_host = Downcast<Map<String, ObjectRef>>(config[kTargetHost]); Review comment: I see your point. However, `target_host` is special cased due to the following reason 1) `target_host` should not be nested: there shouldn't be a target_host inside another target_host; 2) Type `target` may need further refactor to be handled properly in the validation ---------------------------------------------------------------- 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