book/ffi: nullable pointer cleanup #34258

durka · 2016-06-13T17:23:49Z

Expand the "nullable pointer optimization" section with a code example. Fixes #34250.

I also noticed that many of the examples use the libc crate just for types such as c_char and c_int, which are now available through std::os::raw. I changed the ones that don't need to rely on libc. I'm glad to revert that part of the commit if it's unwanted churn.

rust-highfive · 2016-06-13T17:23:55Z

r? @steveklabnik

(rust_highfive has picked a reviewer for you, use r? to override)

eddyb · 2016-06-13T17:30:09Z

src/doc/book/ffi.md

+where `None` corresponds to `null`. So `Option<extern "C" fn(c_int) -> c_int>` is a correct way
+to represent a nullable function pointer using the C ABI (corresponding to the C type
+`int (*)(int)`). (However, generics are not required to get the optimization. A simple
+`enum NullableIntRef { Int(Box<i32>), NotInt }` is also represented as a single pointer.)


I think this can be removed if the "instantiated" wording above is replaced with a generics-agnostic description (i.e. one of the variants has no data, the other variant contains a non-nullable pointer - which also works if it's a nested struct).

durka · 2016-06-13T20:14:01Z

@eddyb check out the expanded (very contrived) example

eddyb · 2016-06-13T20:17:29Z

@durka Amazing! I love it, although I'm not sure how usable it is for someone just getting into Rust.
I'm curious what @steveklabnik thinks of it.

eddyb · 2016-06-13T21:49:10Z

src/doc/book/ffi.md

-as a single pointer, and the non-data variant is represented as the null pointer. This is
-called an "optimization", but unlike other optimizations it is guaranteed to apply to
-eligible types.
+a single field of one of the non-nullable types listed above (or a struct containing such a type).


Don't think it's limited to a single field either. Just there's a non-nullable field somewhere in there, even nested.

durka · 2016-06-14T01:39:28Z

I changed the wording that was pointed out on IRC. It was also suggested that I choose a simpler example, maybe from within snappy, but snappy doesn't have any callbacks or function pointers in its API. It does have functions that take pointers as parameters (but when the snappy API is introduced in the chapter, before this section, nullable-pointer-optimized Options are not used), so I could write an example like the following, but someone else on IRC said it's questionable to use Option<&mut T> in FFI (???).

extern "C" {
    fn snappy_uncompressed_length(compressed: *const u8,
                                  compressed_length: size_t,
                                  result: Option<&mut size_t>) -> c_int;
}

let compressed: Vec<u8> = get_compressed_data();
let mut uncompressed_length = 0;
snappy_uncompressed_length(compressed.as_ptr(),
                           compressed.len(),
                           Some(&mut uncompressed_length));

strega-nil · 2016-06-14T02:01:47Z

@durka We've guaranteed that Option<&mut T> is the same as an &mut T.

As a special case, a generic enum that contains exactly two variants, one of which contains no data and the other containing a single field, is eligible for the "nullable pointer optimization". When such an enum is instantiated with one of the non-nullable types, it is represented as a single pointer, and the non-data variant is represented as the null pointer.

http://doc.rust-lang.org/stable/book/ffi.html#the-nullable-pointer-optimization

durka · 2016-06-14T02:07:16Z

@ubsan note that you're quoting from the very paragraph being edited in
this PR :)
On Jun 13, 2016 10:02 PM, "Nicole Mazzuca" notifications@github.com wrote:

@durka https://github.com/durka We've guaranteed that Option<&mut T> is
the same as an &mut T.

As a special case, a generic enum that contains exactly two variants, one
of which contains no data and the other containing a single field, is
eligible for the "nullable pointer optimization". When such an enum is
instantiated with one of the non-nullable types, it is represented as a
single pointer, and the non-data variant is represented as the null pointer.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#34258 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAC3n5lvum9Gh_LZg6G-IIK81x593r72ks5qLguogaJpZM4I0jUc
.

strega-nil · 2016-06-14T02:07:37Z

@durka I realize. That means that we can't change it :)

durka · 2016-06-14T02:10:20Z

OK. We should also make sure my edits don't introduce any new undesirable
guarantees (for instance, I added the part about searching through nested
structs for the nonzero field).
On Jun 13, 2016 10:08 PM, "Nicole Mazzuca" notifications@github.com wrote:

@durka https://github.com/durka I realize. That means that we can't
change it :)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#34258 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAC3n2eIT9oYNjq4IBtsYXqgUn6z1fpcks5qLg0GgaJpZM4I0jUc
.

strega-nil · 2016-06-14T02:13:59Z

@durka I don't think you should add that part; I think the only guarantee should be for exactly Option<NullableType>, where {&T, &mut T, Box<T>, fn()}: NullableType, since that is the current guarantee.

durka · 2016-06-14T05:11:03Z

Well, @eddyb was encouraging me (I think) to document how it actually
works, but maybe I went into too much detail. Seems to me we're likely to
make enum layout optimization more aggressive in the future, not less.
On Jun 13, 2016 22:14, "Nicole Mazzuca" notifications@github.com wrote:

@durka https://github.com/durka I don't think you should add that part;
I think the only guarantee should be for exactly Option,
where {&T, &mut T, Box, fn()}: NullableType, since that is the current
guarantee.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#34258 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAC3nz_jmr2n0DTWCgiqPI6CxOfkQJKHks5qLg6EgaJpZM4I0jUc
.

strega-nil · 2016-06-14T14:13:13Z

@durka We shouldn't specify that, however. Guarantees are hard to take back. We shouldn't specify anything about enum representation without a very good reason: back compatibility is one of those reasons.

durka · 2016-06-15T16:36:12Z

You may be right. On the other hand I suspect we'll only make enum layout optimization more aggressive, not less, in the future. Anyone else have an opinion on what should be documented? Maybe I could change it to say "Furthermore, the current compiler searches through nested structs to find a non-nullable field in which to stash the discriminant, but this level of enum layout optimization should not yet be relied upon."

strega-nil · 2016-06-15T18:22:50Z

@durka it should never be relied upon. We should guarantee as little as is reasonable, imo. #[repr(Rust)] should be entirely up to us, or whatever compiler implementor.

edit: got it

durka · 2016-06-15T20:01:48Z

I've heard your opinion. I partially agree. I'd also like to get a sense of what the compiler folks think, but we're drowning them out :)

steveklabnik · 2016-07-05T17:28:18Z

I also noticed that many of the examples use the libc crate just for types such as c_char and c_int, which are now available through std::os::raw. I changed the ones that don't need to rely on libc. I'm glad to revert that part of the commit if it's unwanted churn.

It is always really unclear to me when we should be using these and when we should be using libc. @rust-lang/libs , what's the gudieline here?

steveklabnik · 2016-07-05T17:31:38Z

I like this PR, just the question about libc vs std::os::raw.

sfackler · 2016-07-05T17:43:25Z

I have always used libc for those kinds of definitions.

bors · 2016-07-06T00:32:28Z

☔ The latest upstream changes (presumably #34294) made this pull request unmergeable. Please resolve the merge conflicts.

steveklabnik · 2016-07-25T18:27:01Z

@durka any chance you can update this PR?

durka · 2016-07-26T16:12:44Z

Sure thing. I'm getting the sense I should roll back the libc commit and explain the nullable pointer thing in less detail so as to make fewer guarantees.

steveklabnik · 2016-07-26T17:46:15Z

👍

Expand the "nullable pointer optimization" section with a code example. Change examples to use std::os::raw instead of libc, when applicable.

Not sure the example is going to stay, but I can try to pass Travis for the bragging rights.

…ype, even though that is how it works

durka · 2016-07-27T18:00:39Z

Done. There's still the question of whether my example is too contrived/complicated.

steveklabnik · 2016-07-27T18:43:58Z

I am okay with it. Thanks!

@bors: r+ rollup

bors · 2016-07-27T18:44:00Z

📌 Commit 29546dd has been approved by steveklabnik

book/ffi: nullable pointer cleanup Expand the "nullable pointer optimization" section with a code example. Fixes rust-lang#34250. I also noticed that many of the examples use the libc crate just for types such as `c_char` and `c_int`, which are now available through `std::os::raw`. I changed the ones that don't need to rely on libc. I'm glad to revert that part of the commit if it's unwanted churn.

Rollup of 7 pull requests - Successful merges: #34258, #34894, #35050, #35062, #35066, #35072, #35087 - Failed merges:

rust-highfive assigned steveklabnik Jun 13, 2016

durka mentioned this pull request Jun 13, 2016

Book section on FFI with nullable pointers doesn't explain None == null. #34250

Closed

durka force-pushed the patch-25 branch from 1030065 to fe15221 Compare June 13, 2016 17:27

eddyb reviewed Jun 13, 2016
View reviewed changes

durka force-pushed the patch-25 branch from 0d6259b to b3c11b0 Compare July 27, 2016 17:57

durka added 2 commits July 27, 2016 13:58

book/ffi: nullable pointer, libc cleanups

032ea41

Expand the "nullable pointer optimization" section with a code example. Change examples to use std::os::raw instead of libc, when applicable.

generics-agnostic description

48ce206

durka and others added 10 commits July 27, 2016 13:58

expand nullable pointer example

cc2fc48

recursion

84366b6

not just a single field

0016af5

foreign function interface interface

1cceca8

extern fns require named parameters

fae3335

Not sure the example is going to stay, but I can try to pass Travis for the bragging rights.

change confusing wording about discriminant

5276b29

hack to make example compile

54ecc21

revert libc changes

8e7abea

fix typo

1319b29

remove claim about searching through nested fields for the nullable t…

29546dd

…ype, even though that is how it works

durka force-pushed the patch-25 branch from b3c11b0 to 29546dd Compare July 27, 2016 17:58

durka changed the title ~~book/ffi: nullable pointer, libc cleanups~~ book/ffi: nullable pointer cleanup Jul 27, 2016

GuillaumeGomez mentioned this pull request Jul 28, 2016

Rollup of 7 pull requests #35100

Merged

bors added a commit that referenced this pull request Jul 29, 2016

Auto merge of #35100 - GuillaumeGomez:rollup, r=GuillaumeGomez

5a7773a

Rollup of 7 pull requests - Successful merges: #34258, #34894, #35050, #35062, #35066, #35072, #35087 - Failed merges:

bors merged commit 29546dd into rust-lang:master Jul 29, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

book/ffi: nullable pointer cleanup #34258

book/ffi: nullable pointer cleanup #34258

durka commented Jun 13, 2016

rust-highfive commented Jun 13, 2016

eddyb Jun 13, 2016

durka commented Jun 13, 2016 •

edited

Loading

eddyb commented Jun 13, 2016

eddyb Jun 13, 2016

durka commented Jun 14, 2016 •

edited

Loading

strega-nil commented Jun 14, 2016 •

edited

Loading

durka commented Jun 14, 2016

strega-nil commented Jun 14, 2016

durka commented Jun 14, 2016

strega-nil commented Jun 14, 2016

durka commented Jun 14, 2016

strega-nil commented Jun 14, 2016

durka commented Jun 15, 2016

strega-nil commented Jun 15, 2016 •

edited

Loading

durka commented Jun 15, 2016

steveklabnik commented Jul 5, 2016

steveklabnik commented Jul 5, 2016

sfackler commented Jul 5, 2016

bors commented Jul 6, 2016

steveklabnik commented Jul 25, 2016

durka commented Jul 26, 2016

steveklabnik commented Jul 26, 2016

durka commented Jul 27, 2016

steveklabnik commented Jul 27, 2016

bors commented Jul 27, 2016

book/ffi: nullable pointer cleanup #34258

book/ffi: nullable pointer cleanup #34258

Conversation

durka commented Jun 13, 2016

rust-highfive commented Jun 13, 2016

eddyb Jun 13, 2016

Choose a reason for hiding this comment

durka commented Jun 13, 2016 • edited Loading

eddyb commented Jun 13, 2016

eddyb Jun 13, 2016

Choose a reason for hiding this comment

durka commented Jun 14, 2016 • edited Loading

strega-nil commented Jun 14, 2016 • edited Loading

durka commented Jun 14, 2016

strega-nil commented Jun 14, 2016

durka commented Jun 14, 2016

strega-nil commented Jun 14, 2016

durka commented Jun 14, 2016

strega-nil commented Jun 14, 2016

durka commented Jun 15, 2016

strega-nil commented Jun 15, 2016 • edited Loading

durka commented Jun 15, 2016

steveklabnik commented Jul 5, 2016

steveklabnik commented Jul 5, 2016

sfackler commented Jul 5, 2016

bors commented Jul 6, 2016

steveklabnik commented Jul 25, 2016

durka commented Jul 26, 2016

steveklabnik commented Jul 26, 2016

durka commented Jul 27, 2016

steveklabnik commented Jul 27, 2016

bors commented Jul 27, 2016

durka commented Jun 13, 2016 •

edited

Loading

durka commented Jun 14, 2016 •

edited

Loading

strega-nil commented Jun 14, 2016 •

edited

Loading

strega-nil commented Jun 15, 2016 •

edited

Loading