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

Don't use direct field access in Simd functions #339

Merged
merged 3 commits into from
Apr 23, 2023

Conversation

Sp00ph
Copy link
Member

@Sp00ph Sp00ph commented Apr 22, 2023

Copy link
Contributor

@workingjubilee workingjubilee left a comment

Choose a reason for hiding this comment

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

Please document why we have to do it this way, so no one tries to be "helpful" and open a PR "fixing" this.

@Sp00ph
Copy link
Member Author

Sp00ph commented Apr 22, 2023

Where do you think would be the best place to put such a disclaimer? Should I just put it on the Simd struct directly, since it's not specific to the changed functions? Or maybe both on the struct and on the changed functions?

@workingjubilee
Copy link
Contributor

Let's do this belt-and-suspenders style, yeah. Double up. It can be terse but it should be in both places.

@Sp00ph
Copy link
Member Author

Sp00ph commented Apr 22, 2023

I expanded the comment a bit to also include that you can't directly construct the type either, as I don't think it's obvious that that counts as a field access (and also had a typo in the previous commit message).

@calebzulawski
Copy link
Member

I think you can remove the transmute feature now

@Sp00ph
Copy link
Member Author

Sp00ph commented Apr 22, 2023

This is the second time in a row now that the x86_64-apple-darwin test failed because it wasn't able to reach https://static.rust-lang.org/dist/channel-rust-nightly.toml.sha256. Looks like something's wrong with that machine.

@workingjubilee workingjubilee merged commit f916add into rust-lang:master Apr 23, 2023
75 checks passed
@jhorstmann
Copy link

Sounds like this could already be the fix to what I just reported in rust-lang/rust#110722

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.

None yet

4 participants