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

Remove untagged_unions feature #924

Closed
RalfJung opened this issue Oct 5, 2020 · 2 comments · Fixed by #925
Closed

Remove untagged_unions feature #924

RalfJung opened this issue Oct 5, 2020 · 2 comments · Fixed by #925

Comments

@RalfJung
Copy link
Member

RalfJung commented Oct 5, 2020

Once rust-lang/rust#77547 lands, I think the untagged_unions feature can be removed from stdarch. I do not know what the usual process for this is, does stdarch use bootstrap flags like everything else?

I also noticed stdarch tests use unions to transmute things. Is there any particular reason why the tests do not use transmute? Note in particular that stdarch is using unions incorrectly here; without repr(C), there is no guarantee that the union fields all start at the same offset.

@Amanieu
Copy link
Member

Amanieu commented Oct 5, 2020

It's fine for stdarch to use the bootstrap flag since it is built as a submodule of core. This will be ignored in CI which builds it as a separate crate.

Regarding the tests I don't think there was any intentional reason to use unions. I wouldn't mind a PR changing them to use transmute or repr(C) unions instead.

@Mark-Simulacrum
Copy link
Member

No, please try to avoid using bootstrap in submodules, as we need to orchestrate a dance of dropping it (requiring submodule updates and potential upstream CI breakage) when bumping the beta compiler.

If it's unavoidable, fine, but please then try to make sure that the submodule can work with the flag enabled on nightly too, though perhaps with less functionality or something like that.

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 a pull request may close this issue.

3 participants