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

Spell out when arrays have zero size #162

Merged
merged 2 commits into from
Jul 25, 2019

Conversation

gnzlbg
Copy link
Contributor

@gnzlbg gnzlbg commented Jul 8, 2019

This spells out explicitly when arrays have zero size. Solves part of #37.

@Lokathor
Copy link
Contributor

Lokathor commented Jul 8, 2019

This looks like the only way that it could be defined.

However, while this file is in a PR and being edited and such, I want to bring up something that ubsan said in the zulip: that lots of unsafe code math probably already relies on stride==size and that there's probably no chance to alter that fact in future versions of Rust.

@RalfJung
Copy link
Member

I want to bring up something that ubsan said in the zulip: that lots of unsafe code math probably already relies on stride==size and that there's probably no chance to alter that fact in future versions of Rust.

If nobody but me thinks it makes any sense to keep stride != size as an option, I am fine with giving up.

@petrochenkov
Copy link

Me also thinks it makes sense to keep stride != size and support "no trailing padding" via opt-in.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jul 13, 2019

@petrochenkov @Amanieu do you think it would be possible to produce a pre-RFC in internals working #[no_trailing_padding] out ? I think it would be helpful to at least try to produce a document that shows that this is not a backward-incompatible change at this point.

I'll be fine with leaving the stride != size distinction here at least until we start preparing an RFC for the UCGs.

@hanna-kruppe
Copy link

We don't need to resolve the stride/size issue to make this editorial decision for UCG. Even if Rust eventually adds a notion of stride distinct from size, whoever designs and introduces that feature already needs update all our docs to account for it, and there's far more of that than just UCG.

Plus, we aren't even doing a very good job of being "forward compatible" with it anyway (as I'd expect when trying to accomodate such a remote and hypothetical and invasive change). Struct layout would probably be affected too (consider struct Foo(u16, u8); struct Bar(Foo, u8);) but there's no mention of the stride/size distinction in the corresponding chapter. Currently it's just this weird vestigial bit in the array chapter that's neither here nor there and just complicates things.

@petrochenkov
Copy link

@gnzlbg

do you think it would be possible to produce a pre-RFC in internals working #[no_trailing_padding] out ?

That would be really great (even if unrelated to this PR) and should've been done years ago.
Back in 2017-2018 I put an entry for attempting to implement #[no_trailing_padding] into my work queue after seeing one more rust-lang/rust#17027 -like issue, but it never got enough priority unfortunately.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jul 13, 2019

@rkruppe The question I have is whether we should guarantee here that unsafe code can rely on this never happening for arrays. That would kill any work on such a feature.

Maybe that is already the case, and such a feature has no future, but AFAICT, the layout of arrays and repr(Rust) is currently unspecified enough, that doing that might be possible, in theory. In practice, this might break enough code that it might not be possible. I don't know.

I don't mind removing this here, as long as we open an issue to track this, and resolve that issue before RFCing a guarantee that unsafe code never has to deal with size != stride.

@hanna-kruppe
Copy link

In brief, my response to that worry is that any proposal in this direction already has to do something heroic about the distinction not existing today in any documentation (up to and including the reference [1]) nor in any developer's head or any existing code. If that can be managed somehow (I am skeptical but let's consider the possibility) then adjusting the UCG as well is not a significant additional roadblock. Conversely, I can't imagine that writing this down in one more place (even if it's a rather official one after the eventual RFC) makes the de facto reliance on stride==size in the ecosystem significantly worse.

[1] See https://doc.rust-lang.org/reference/type-layout.html#size-and-alignment and https://doc.rust-lang.org/reference/type-layout.html#array-layout for example

@Lokathor
Copy link
Contributor

Using a small example and staying specific to arrays:

  • array[i]
  • array.as_ptr().add(i)

Today these are "the two ways" to access an array element. Except there's a third way that someone somewhere is doubtlessly using (probably indirectly):

  • (array.as_ptr() as *const u8).add(i * size_of::<T>())

Any changes that make stride!=size won't affect the first two forms, but it will break that third form.

Which isn't to say it's impossible, but it's probably an edition change at least where code has to opt in to the size/stride distinction, and even then I don't know how you'd make 2015/2018 and 2021 compatible with each other.

Copy link
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

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

LGTM.

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.

5 participants