slyubomirsky commented on code in PR #16641:
URL: https://github.com/apache/tvm/pull/16641#discussion_r1504846413
##########
python/tvm/script/parser/relax/parser.py:
##########
@@ -274,7 +274,21 @@ def post_visit_local_function(self: Parser, node:
doc.Expr) -> None:
@dispatch.register(token="relax", type_name="Expr")
def visit_expr_stmt(self: Parser, node: doc.Expr) -> None:
value = self.eval_expr(node.value)
- if value is not None:
+ if isinstance(value, relax.Expr):
+ var = R.emit(value)
+ IRBuilder.name("_", var)
+ is_void_value = (
+ isinstance(var.struct_info, relax.TupleStructInfo) and
len(var.struct_info.fields) == 0
+ )
+
+ if not is_void_value:
+ self.report_error(
+ node,
+ f"Non-void relax expressions must be bound to a variable, "
+ f"but expression of type {var.struct_info} was used as a
statement.",
+ )
Review Comment:
Those are good points. For the first case, we could have a warning (as C and
other languages do with the right settings) for ignoring a return value. The
second one is an interesting issue. I think it suggests that expecting an exact
textual match for the parser roundtripping is too strict of a criterion for
this situation, since a "statement" can always be written as `_ = ...` and it
would be a choice as to whether to write it that way or use the friendlier
syntax. It would make it harder to write automatic tests, true. For a
systematic solution, maybe we could formalize the idea of a desugaring step for
testing purposes?
--
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]
