Skip to content

Clarify non-nullable pointer optimization in repr(C) section#60

Merged
Gankra merged 2 commits intorust-lang:masterfrom
ramosbugs:other_reprs_nullable_opt
Mar 21, 2018
Merged

Clarify non-nullable pointer optimization in repr(C) section#60
Gankra merged 2 commits intorust-lang:masterfrom
ramosbugs:other_reprs_nullable_opt

Conversation

@ramosbugs
Copy link
Copy Markdown
Contributor

Resolves #59

@Gankra
Copy link
Copy Markdown
Contributor

Gankra commented Mar 13, 2018

Looks good, just need to double-check that Box is Actually FFI-Safe, as this is quite surprising to hear

@ramosbugs
Copy link
Copy Markdown
Contributor Author

Thanks for the review!

The Nullable Pointer Optimization subsection under FFI calls out boxes since they can't hold null pointers. This makes it seem like they should be FFI-safe since the representation in memory would be the same as for a raw pointer (iiuc). I'm far from an authority on this, so please correct me if I'm wrong!

One potential distinction I see between Box<T> and the other listed types is that since a Box holds Rust-owned data (which is freed using Rust's allocator), it's not suitable to receive pointers from non-Rust code. However, it should be able to provide pointers from Rust to non-Rust code, right?

@Gankra
Copy link
Copy Markdown
Contributor

Gankra commented Mar 14, 2018

There are subtle annoying details about ABI here. Specifically, pointers are a specific kind of thing in ABIs, and a struct containing a pointer does not have the same one. This affects how it would be passed in a function call. I expect we currently do match the ABI, but I'm uncertain that we guarantee it. (why would we?)

@Gankra
Copy link
Copy Markdown
Contributor

Gankra commented Mar 14, 2018

Discussing with @eddyb, we agree we could guarantee the ABI of Box but don't have a strong motivation to do so. All we guarantee about Box is that Option<Box> has the same layout as Box.

@ramosbugs
Copy link
Copy Markdown
Contributor Author

Makes sense. I can't see any motivation for guaranteeing that ABI either. Updated the PR.

Copy link
Copy Markdown
Contributor

@Gankra Gankra left a comment

Choose a reason for hiding this comment

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

Thanks!

@Gankra Gankra merged commit 6a8f0a2 into rust-lang:master Mar 21, 2018
@ramosbugs ramosbugs deleted the other_reprs_nullable_opt branch March 21, 2018 06:22
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.

2 participants