Pull Request

变更摘要

此次变更主要简化了 ONNX 解析器中空 domain 的处理逻辑。原先当节点 domain 为空时，要求 domain_verseion_ 映射恰好只有一个条目才能推断域，否则报错。现在改为直接默认空 domain 为 "ai.onnx" 标准域，统一查找 domain_verseion_，并在 ModelParseToGraph 中同步对 opset_import 做了相同的归一化处理。

主要改动

• ConstructOriType 空 domain 处理简化: 将原先需要根据 domain_verseion_ 大小推断空域名的复杂逻辑（要求恰好一个条目），替换为直接默认为 "ai.onnx" 后在 domain_verseion_ 中查找，找不到则报错。
• ModelParseToGraph 中 opset_import 的 domain 归一化: 在遍历 opset_import 填充 domain_verseion_ 时，对空 domain 同样默认为 "ai.onnx"，确保与 ConstructOriType 行为一致。
• 单元测试更新: ConstructOriType_empty_domain_multiple_versions 测试预期从 PARAM_INVALID 改为 SUCCESS，验证空 domain 在多个版本存在时也能正确解析为 "ai.onnx::11::CustomOp"；测试名 Prechecker_construct_ori_type_failure 重命名为 Prechecker_unregistered_op_type_failure，更准确地描述测试场景。

代码审查

I've completed a thorough review of both changed files. Let me summarize my analysis:

File 1: parser/parser/onnx/onnx_parser.cc

• ConstructOriType (lines 420-431): The old logic had complex branching for empty domain (requiring domain_verseion_.size() == 1). The new code cleanly defaults empty domain to "ai.onnx" and performs a single map lookup. This is consistent with ONNX semantics (empty domain = default ONNX namespace). The ModelParseToGraph change (lines 909-913) mirrors this same normalization when populating domain_verseion_ from opset_import, so both producer and consumer of the map use the same key. No correctness issue.

• All error paths return PARAM_INVALID appropriately. The GE_CHECK_NOTNULL guard at line 413 is preserved. No null-pointer, uninitialized variable, or resource leak issues.

• The old code had a dangling-pointer bug with to_string(domain_version_size).c_str() (temporary destroyed before use), but this code was removed, not introduced.

File 2: tests/parser/ut/parser/testcase/onnx_parser_testcase/onnx_parser_unittest.cc

• ConstructOriType_empty_domain_multiple_versions (line 777): Changed from EXPECT_EQ(ret, PARAM_INVALID) to EXPECT_EQ(ret, SUCCESS) with an additional EXPECT_EQ(ori_type, "ai.onnx::11::CustomOp") — correctly reflects the new behavior: empty domain defaults to "ai.onnx" even with multiple domains registered.

• Test rename (line 1104): Prechecker_construct_ori_type_failure → Prechecker_unregistered_op_type_failure — accurately reflects that with the new code, ConstructOriType succeeds for empty domain, and the failure now comes from the unregistered op type "BadOp" during Prechecker's CheckType or AddOp step.

No issues found in either file. The change is clean, consistent, and well-tested.

Closing Summary:

• P0: 0
• P1: 0
• P2: 0
• P3: 0

Overall risk judgment: Low. This is a well-contained simplification that aligns the parser with ONNX semantics (empty domain = default ONNX namespace "ai.onnx"). The normalization is applied consistently in both ModelParseToGraph (map population) and ConstructOriType (map lookup). The tests correctly reflect the new behavior. No regressions, correctness issues, or security concerns identified.

Files reviewed:

• parser/parser/onnx/onnx_parser.cc — no issues
• tests/parser/ut/parser/testcase/onnx_parser_testcase/onnx_parser_unittest.cc — no issues

💬 仅评论