-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Relax] Normalize use of void-type variable to inline R.tuple() #16658
[Relax] Normalize use of void-type variable to inline R.tuple() #16658
Conversation
502f828
to
edddfc3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be good for avoiding the tricky situation that arises with #16641, mainly where it can be a problem to omit the binding of a unit-type variable if the var is used later. I think it's pretty harmless to simply inline all unit values.
This is a follow-up commit to apache#16641. While parsing of relax expressions without a variable binding could be implemented at that point (e.g. `R.assert_op(condition)` instead of `dummy_var = R.assert_op(condition)`), the corresponding printing changes could not. This was because a variable that satisfies `relax::HasVoidStructInfo(var)` could still be used later in the function, and removing its binding would result in use of an undefined variable. This commit normalizes use of void-type variables to an in-line `R.tuple()`. This simplifies the relax function, and also allows the binding of void-type variables to be hidden.
edddfc3
to
844f06c
Compare
Agreed, and it matches our existing behavior for things like |
…he#16658) * [Relax] Normalize use of void-type variable to inline R.tuple() This is a follow-up commit to apache#16641. While parsing of relax expressions without a variable binding could be implemented at that point (e.g. `R.assert_op(condition)` instead of `dummy_var = R.assert_op(condition)`), the corresponding printing changes could not. This was because a variable that satisfies `relax::HasVoidStructInfo(var)` could still be used later in the function, and removing its binding would result in use of an undefined variable. This commit normalizes use of void-type variables to an in-line `R.tuple()`. This simplifies the relax function, and also allows the binding of void-type variables to be hidden. * Fix breakage in unit tests
This is a follow-up commit to #16641. While parsing of relax expressions without a variable binding could be implemented at that point (e.g.
R.assert_op(condition)
instead ofdummy_var = R.assert_op(condition)
), the corresponding printing changes could not. This was because a variable that satisfiesrelax::HasVoidStructInfo(var)
could still be used later in the function, and removing its binding would result in use of an undefined variable.This commit normalizes use of void-type variables to an in-line
R.tuple()
. This simplifies the relax function, and also allows the binding of void-type variables to be hidden.