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

Provide the ability to zero padding bytes and return &[u8] #494

Open
5 tasks
joshlf opened this issue Oct 12, 2023 · 3 comments
Open
5 tasks

Provide the ability to zero padding bytes and return &[u8] #494

joshlf opened this issue Oct 12, 2023 · 3 comments
Labels
compatibility-nonbreaking Changes that are (likely to be) non-breaking

Comments

@joshlf
Copy link
Member

joshlf commented Oct 12, 2023

Progress

  • Update this issue description per this comment
  • Update KnownLayout to require that fields implement KnownLayout too
  • Do one of the following:
    • Decide that the freeze intrinsic (RFC 3605) will land and stabilize soon enough that we can rely on it instead; relax KnownLayout to not be recursive (and watch out for #1162)
    • Use the recursive KnownLayout requirement to implement this design

Details

Issues like this one demonstrate that it is sometimes useful to access the bytes of a type which cannot implement AsBytes. In these cases, it should be sound to:

  • Recursively zero any inter-field padding bytes
  • Provide access to the bytes of the object as &[u8]

We would need to teach KnownLayout to be able to zero padding, e.g.:

pub unsafe trait KnownLayout {
    fn zero_padding_and_get_bytes(&mut self) -> &[u8];
}

The only requirement for a type supporting this operation is that we know where its padding bytes are. The public API for this type could be in KnownLayout.

As of this writing, KnownLayout does not require that a type's fields also be KnownLayout. We are planning to add that requirement in order to support this design.

Open questions

  • What if we want to copy from a &T (which we can't modify) into a buffer while initializing any padding bytes in the destination like musli-zerocopy does? See this discussion.
  • fn zero_padding_and_get_bytes(&mut self) -> &[u8] doesn't allow the type system to "remember" that a value has had its padding zeroed. Perhaps we could introduce a witness type and introduce a method like:
    • fn zero_padding(&mut self) -> &mut AsBytesWitness<Self>
    • AsBytesWitness would provide field projection into any field, but would only provide direct mutable access to fields which are AsBytes (as writing to these fields cannot introduce uninitialized bytes)

Related & prior art

@joshlf joshlf added the compatibility-nonbreaking Changes that are (likely to be) non-breaking label Oct 24, 2023
@joshlf
Copy link
Member Author

joshlf commented May 3, 2024

EDIT: We've decided to require KnownLayout to be recursive to be forwards-compatible with this design. We can always relax that requirement in the future if we decide not to implement this design.


I just realized that KnownLayout may not have the necessary context. In particular, we support deriving KnownLayout on all Sized types, even those that are not repr(C), and even those whose field types are not KnownLayout. The "not repr(C)" thing we might be able to work around by querying for field offsets, but without the ability to recursively descend into each field type, we have no way of knowing where padding is inside of individual fields.

@jswrenn
Copy link
Collaborator

jswrenn commented May 10, 2024

I suspect default-repr enums are going to render this strategy a non-starter, since we cannot reason about the size of their discriminants. Given that, do we still want to proceed with #1223?

@joshlf
Copy link
Member Author

joshlf commented May 30, 2024

I suspect default-repr enums are going to render this strategy a non-starter, since we cannot reason about the size of their discriminants. Given that, do we still want to proceed with #1223?

Discussed offline. Given this concern, and given the likelihood of freeze stabilizing in the medium-term, we're going to hold off on this issue for now. We may still do it - I'm not closing it - but we're not going to make any attempt to be forwards-compatible with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility-nonbreaking Changes that are (likely to be) non-breaking
Projects
None yet
Development

No branches or pull requests

2 participants