So in PR 37429, @camlorn made the excellent change to reorder struct fields. Unfortunately – and not unexpectedly – making this change broke various bits of unsafe code, notably in Servo but perhaps elsewhere, which was inadvertently relying on the layout of structs. (@eddyb has been doing some intense investigation and may have found the source of the problem, but maybe not.)
I wanted to discuss two things:
- What is the proper way (in general) for us to roll out changes of this kind?
- What should we do about this particular change?
My feeling is that when we have a change that we believe may cause existing code to stop working – even if that code is relying on things it should not be relying on – we should try to phase it in. At minimum, we could provide an opt-in (i.e.,
-Z reorder-struct-fields) and do some sort of PSA announcing that this change will be coming to nightly in one month’s time (or whatever), so that people can do some advance testing.
With respect to this change, I think we should roll it back immediately on beta (which is uncontroversial, I think) as well as master. We should then add it back ASAP into master with an opt-in flag. We can then make an announcement and turn it on my by default in a month or so. In other words, we should try to follow this procedure. =)
Ideally, the flag would not be boolean, but would have some sort of filter (e.g., on paths) so that when a problem is found, people can do binary search to figure out which structs are at fault. This seems to me to be an important point in practice but not so much in theory.
We discussed this some on #rustc, but no clear conclusion was reached. @camlorn is preparing a PR for beta at least. I thought maybe it’d be good to have the discussion somewhat more broadly, hence this internals post.