Skip to content
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

[wgsl-in] Implement phony assignment #1866

Merged
merged 1 commit into from
Apr 27, 2022
Merged

Conversation

teoxoy
Copy link
Member

@teoxoy teoxoy commented Apr 26, 2022

fixes #1842

@stshine
Copy link
Contributor

stshine commented Apr 27, 2022

What if the spec changed to keep up with how rust handle phony assignment?

let unnamed_110 = (2 != 1);
let unnamed_111 = (2u != 1u);
let unnamed_112 = (2.0 != 1.0);
let unnamed_113 = (vec2<i32>(2) != vec2<i32>(1));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we know how many times a particular expression is used. So we could just generate _ = xxx if it's not used, and the name isn't specified

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm now thinking a new kind of statement for phony assignment might be better (i.e. Phony(Handle<Expression>)) since using named_expressions with _ feels like a hack. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not supposed to be a named expression at all. It's just an expression that is emitted at a particular point.
I don't think this warrants an IR change, we shouldn't be introducing a new statement type.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #1869 to address this.

@kvark kvark merged commit 062b66c into gfx-rs:master Apr 27, 2022
@teoxoy teoxoy deleted the phony-assignment branch April 27, 2022 08:03
@teoxoy
Copy link
Member Author

teoxoy commented Apr 27, 2022

@stshine the first time I used search and replace to replace let _ = with _ = I forgot to only filter wgsl files and was surprised to see that the code still compiled. As far as I can see rust also supports _ =.

@stshine
Copy link
Contributor

stshine commented Apr 27, 2022

@teoxoy Oh that's something TIL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[wgsl-in] Phony assignment
3 participants