Skip to content

#[repr(transparent)] for Box<T>? #52976

Closed

Description

@RReverser

#43036 added #[repr(transparent)] with one of the goals being safe FFI interaction for newtypes, and added that attribute to bunch of built-in types such as NonZero, Waker, Unique etc.

One use-case that could be really helpful for FFI though is adding it to Box<T>. I haven't found any previous discussions on pros and cons of doing so, so apologies if this is way off and there are obvious reasons why it can't be such, but as far as I can tell, this is not a breaking change neither from API nor from memory representation perspective.

Looking from definition, Box is just a wrapper around Unique which is already #[repr(transparent)] and wraps NonNull which is also #[repr(transparent)] which, in turn, wraps raw pointer which is FFI-safe.

Adding this attribute to Box would make it transparent wrapper all the way down around a raw pointer, and so would allow to do FFI interactions by simply leaking Box as return type in allocator functions and accepting it as-is in deallocator function without manual into_raw and from_raw conversions.

Aside from ergonomics improvement (allocation / deallocation is pretty common operation in cdylibs), this would also allow code to be more self-documenting as just from looking at rustdoc you would be able to tell which pointers are owned and will be allocated or consumed by Rust side.

Counter-argument might be that one can implement custom transparent CBox on top of NonNull, but this involves reimplementing many APIs and guarantees Box already provides e.g. Unique is not exposed at the moment, and then you also have various standard traits that "just work" with Box, so if it would be possible to avoid reimplementing all of that with no obvious downsides, it might be still useful to shared library authors.

Activity

  1. mjbshaw commented on Aug 5, 2018

    @mjbshaw
    Contributor

    How does this interact with Drop? And how are foreign functions expected to allocate (using Rust's allocator) the boxed memory?

  2. CryZe commented on Aug 5, 2018

    @CryZe
    Contributor

    They don't. I already do this in my C API even though it's not entirely defined behavior at the moment: https://github.com/LiveSplit/livesplit-core/blob/3ac0834f70a3963f1b7e4fec84b0803c51e6349e/capi/src/fuzzy_list.rs#L12-L24

  3. RReverser commented on Aug 5, 2018

    @RReverser
    ContributorAuthor

    Yeah, what @CryZe said. This is not for importing foreign functions, this is for easily and safely exposing Rust functions that allocate+return+deallocate Box via C API.

  4. glandium commented on Aug 5, 2018

    @glandium
    Contributor

    How would this interact with #50882?

  5. RReverser commented on Aug 5, 2018

    @RReverser
    ContributorAuthor

    Sadly, they wouldn't be compatible when allocator is not zero-sized. I'm surprised Box is going to store allocator by value (it seems so from the change?) even though in theory same allocator state might be reused for many allocations and must be shared between them... Also it seems quite inefficient for most libs/apps out there that use just one allocator. I guess I'm going to need to dig into that PR deeper tomorrow when I'm not asleep.

  6. sfackler commented on Aug 5, 2018

    @sfackler
    Member

    Also it seems quite inefficient for most libs/apps out there that use just one allocator

    The allocator value that would be used in most cases is Global, a zero sized type.

  7. glandium commented on Aug 5, 2018

    @glandium
    Contributor

    Nothing in Box<T, A: Alloc>(Unique<T>, A) says the allocator is stored by value. It all depends what Alloc is impl'ed for. It could be a ZST, like Global, or a reference (impl Alloc for &MyAlloc).

  8. RReverser commented on Aug 6, 2018

    @RReverser
    ContributorAuthor

    @sfackler The question is more whether this would be guaranteed for any allocators (I suspect not). In that case, repr(transparent) would indeed not work, but I suspect there are other parts of compiler that currently rely on lang item behind Box being just a pointer...

  9. CryZe commented on Aug 6, 2018

    @CryZe
    Contributor
  10. RReverser commented on Aug 6, 2018

    @RReverser
    ContributorAuthor

    @CryZe

    hantomData would / should be used instead

    Sorry, I didn't get it - instead of what?

  11. CryZe commented on Aug 6, 2018

    @CryZe
    Contributor

    PhantomData<A> instead of an embedded A, which could not be zero sized.

  12. RReverser commented on Aug 6, 2018

    @RReverser
    ContributorAuthor

    Agreed. IIUC the motivation for storing A is to give flexibility and allow several instances of the same allocator, each with its own state, but this looks like quite a niche usecase to be covered by stdlib.

  13. RReverser commented on Aug 6, 2018

    @RReverser
    ContributorAuthor

    Also, I suppose, if allocator does want to share some extra data, it can always allocate pointer with larger size and store that metadata (or a pointer to one) alongside actual data in the heap.

  14. sfackler commented on Aug 6, 2018

    @sfackler
    Member

    Why does Box need to differ from unique_ptr in this respect?

  15. RReverser commented on Aug 6, 2018

    @RReverser
    ContributorAuthor

    Do you mean C++ unique_ptr? Why would Box be similar to it? For one, Rust already has lots of differences with C++, including in memory management, and in particular availability of zero-sized types which is what @CryZe and me are suggesting to use for custom allocators.

  16. 16 remaining items

  17. gnzlbg commented on Jul 10, 2019

    @gnzlbg
    Contributor

    @RReverser

    @Lokathor pointed out, that this "implementation detail" is "documented" in the repr(transparent) RFC:

    As a matter of optimisation, eligible #[repr(Rust)] structs behave as if they were #[repr(transparent)] but as an implementation detail that can't be relied upon by users.

    Box<T> is not user code, but part of the implementation, so it could rely on this, and document that, at least for Box<T> this will always hold.

  18. RReverser commented on Jul 10, 2019

    @RReverser
    ContributorAuthor

    Fine by me - not the outcome I initially hoped for, but it does the trick and reaches same goal anyway :)

  19. SimonSapin commented on Jul 25, 2019

    @SimonSapin
    Contributor

    I understand the desire to allow A's that aren't ZSTs, but it is IMO the job of those extending Box with that to figure out a way of doing so without breaking the guarantee of Box<T> being transparent.

    The goal of using standard library containers with different allocators, including non-zero-size handles, is part of an accepted RFC that precedes the new transparency guarantee being proposed here: https://rust-lang.github.io/rfcs/1398-kinds-of-allocators.html#what-about-standard-library-containers

    Ideally we would have Box<T, A> is repr(transparent) if and only if A is zero-size, but as far as I know there is no way to express that in the language today.

  20. gnzlbg commented on Jul 25, 2019

    @gnzlbg
    Contributor

    @SimonSapin

    Ideally we would have Box<T, A> is repr(transparent) if and only if A is zero-size, but as far as I know there is no way to express that in the language today.

    This PR solves that: #62514

    Once we stabilize Box<T, A> we just need to clarify that that only holds if A is zero-sized, and that's pretty much it AFAICT.

    We plan to submit an RFC to guarantee this for all types for which this holds as part of the UCGs RFC. There is a PR in the UCG repo with proposed wording for that as well: rust-lang/unsafe-code-guidelines#164 (is blocked on some nit-picking on how to abbreviate one-aligned ZSTs but there is consensus that we want to guarantee that). So when that happens the documentation comment guaranteeing this wouldn't be technically necessary anymore, since it would be a given, but it would obviously be documentation worth having.

  21. crlf0710 commented on Aug 17, 2019

    @crlf0710
    Member

    I think #62514 is only adding comments, and there seems to be needing language level mechanism if we want to move forward.

  22. gnzlbg commented on Aug 17, 2019

    @gnzlbg
    Contributor

    I think #62514 is only adding comments,

    The comment there guarantees that box can be used in C FFI. Isn't that enough to resolve this?

  23. crlf0710 commented on Aug 17, 2019

    @crlf0710
    Member

    Isn't that enough to resolve this?

    Is it? From FFI ABI perspective, adding those comments doesn't turn ABI for Box from representing a struct to representing a pointer, which is all #[repr(transparent)] all about, does it?

  24. CryZe commented on Aug 17, 2019

    @CryZe
    Contributor

    As long as it is guaranteed and there is some test verifying that it is the case, it doesn't need a special attribute.

  25. gnzlbg commented on Aug 17, 2019

    @gnzlbg
    Contributor

    From FFI ABI perspective, adding those comments doesn't turn ABI for Box from representing a struct to representing a pointer, which is all #[repr(transparent)] all about, does it?

    Box is as special as &mut T or i32, the compiler already sets the ABI of Box to that of a single pointer. The only thing the comment does is guarantee to users that this will never change (otherwise that would be a breaking change).

  26. crlf0710 commented on Aug 17, 2019

    @crlf0710
    Member

    ok, i guess that's the magic of lang_item then. thank you for the elaboration.

  27. added a commit that references this issue on Dec 12, 2019
  28. Mark-Simulacrum commented on Dec 23, 2019

    @Mark-Simulacrum
    Member

    We now have some extensive documentation about use of Box with FFI and generally the properties we guarantee: https://doc.rust-lang.org/nightly/std/boxed/index.html#memory-layout. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-feature-requestCategory: A feature request, i.e: not implemented / a PR.T-libs-api[DEPRECATED; DO NOT USE]

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions