gemini-code-assist[bot] commented on code in PR #18857:
URL: https://github.com/apache/tvm/pull/18857#discussion_r2868290423
##########
python/tvm/script/parser/tir/parser.py:
##########
@@ -537,9 +537,23 @@ def visit_assert(self: Parser, node: doc.Assert) -> None:
node : doc.Assert
The doc AST assert node.
+
+ The assert message can be either:
+ - A plain string: ``assert cond, "message"``
+ - A tuple of (kind, [parts...]): ``assert cond, ("ValueError", ["part0",
"part1"])``
"""
cond = self.eval_expr(node.test)
msg = self.eval_expr(node.msg)
+ if isinstance(msg, tuple) and len(msg) == 2:
+ kind_str, parts = msg
+ if isinstance(kind_str, tvm.tir.StringImm):
+ kind_str = kind_str.value
+ if isinstance(parts, list | tuple):
Review Comment:
The use of `|` for type unions (e.g., `list | tuple`) was introduced in
Python 3.10. To maintain compatibility with older Python versions (e.g., 3.8,
3.9), it's better to use a tuple of types with `isinstance`.
```suggestion
if isinstance(parts, (list, tuple)):
```
##########
python/tvm/script/parser/tir/parser.py:
##########
@@ -537,9 +537,23 @@ def visit_assert(self: Parser, node: doc.Assert) -> None:
node : doc.Assert
The doc AST assert node.
+
+ The assert message can be either:
+ - A plain string: ``assert cond, "message"``
+ - A tuple of (kind, [parts...]): ``assert cond, ("ValueError", ["part0",
"part1"])``
"""
cond = self.eval_expr(node.test)
msg = self.eval_expr(node.msg)
+ if isinstance(msg, tuple) and len(msg) == 2:
+ kind_str, parts = msg
+ if isinstance(kind_str, tvm.tir.StringImm):
+ kind_str = kind_str.value
+ if isinstance(parts, list | tuple):
+ parts_str = [p.value if isinstance(p, tvm.tir.StringImm) else
str(p) for p in parts]
+ frame = T.Assert(cond, "\n".join(parts_str), kind=str(kind_str))
Review Comment:
The current implementation for handling structured assert messages in
TVMScript joins the message parts into a single string. This prevents one of
the key benefits of this PR, which is string fragment reuse at the TIR level to
reduce binary size, as mentioned in the summary. The `AssertStmt` node is
designed to hold an array of string fragments, but this implementation flattens
them into a single string.
To properly support structured messages from TVMScript, the `T.Assert` IR
builder function and its underlying FFI implementation should be updated to
accept a list of strings, which can then be used to populate the
`message_parts` array in the `AssertFrameNode`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
