Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upSpecify #[repr(transparent)] #1758
Conversation
Amanieu
reviewed
Sep 26, 2016
| # Alternatives | ||
| [alternatives]: #alternatives | ||
|
|
||
| None. |
This comment has been minimized.
This comment has been minimized.
Amanieu
Sep 26, 2016
Contributor
One obvious alternative would be to do this automatically for all structs that have just a single field.
This comment has been minimized.
This comment has been minimized.
nox
Sep 26, 2016
Author
Contributor
It is done automatically for all newtypes, but then you can't pass that to FFI stuff without it becoming a struct on the C side too.
This comment has been minimized.
This comment has been minimized.
nrc
Sep 27, 2016
Member
Is there a tooling solution here? Could a sufficiently smart bindgen work around this problem?
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
nox
Sep 28, 2016
Author
Contributor
Because there is no tooling that can be invented to do something that is not possible without this attribute. Either you do struct Foo(f64) and it can't be used in FFI, or you do #[repr(C)] struct Foo(f64) and it won't behave as a bare f64 in FFI.
This comment has been minimized.
This comment has been minimized.
sfackler
Sep 28, 2016
Member
It seems like something like bindgen could pretty easily create wrappers that unbox transparent structs.
This comment has been minimized.
This comment has been minimized.
nox
Sep 28, 2016
Author
Contributor
This means an additional type, additional transmutes and/or additional boxing/unboxing functions. That's code bloat to me and I don't see why I would want that. It also doesn't help for structures with UnsafeCell<T> fields.
This comment has been minimized.
This comment has been minimized.
nikomatsakis
Oct 5, 2016
Contributor
It seems like something like bindgen could pretty easily create wrappers that unbox transparent structs.
This would mean that the fields can't be private, however. Not sure how much that matters in practice.
This comment has been minimized.
This comment has been minimized.
|
I feel that the RFC could do with a note about |
This comment has been minimized.
This comment has been minimized.
|
I can also see this attribute being useful for atomic types such as |
This comment has been minimized.
This comment has been minimized.
Yep the whole reason I'm writing this RFC is for |
nox
force-pushed the
nox:repr-transparent
branch
from
4056178
to
a70131c
Sep 26, 2016
This comment has been minimized.
This comment has been minimized.
|
@Amanieu Added a note about FFI and AtomicUsize use cases. |
nagisa
reviewed
Sep 26, 2016
|
I feel like this RFC is quite underspecified, especially around interactions with some more obscure features. Overall the idea seems okay to me, though. |
| the same type as the single field. For example on ARM64, functions returning | ||
| a structure with a single `f64` field return nothing and take a pointer to be | ||
| filled with the return value, whereas functions returning a `f64` return the | ||
| floating-point number directly. |
This comment has been minimized.
This comment has been minimized.
nagisa
Sep 26, 2016
•
Contributor
I do not see why the (unspecified) Rust ABI {w,sh}ould share the same behaviour, as opposed to being “transparent” by default.
This comment has been minimized.
This comment has been minimized.
petrochenkov
Sep 26, 2016
Contributor
+, transparent seems to be relevant only for repr(C) stuff, as a tweak to the default C ABI
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
|
||
| Syntactically, the `repr` meta list will be extended to accept a meta item | ||
| with the name "transparent". This attribute can be placed only on newtypes, | ||
| which means structures (and structure tuples) with a single field. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
nox
Sep 28, 2016
Author
Contributor
I tried to be conservative for now given I don't have a use case for univariant unions and enums in FFI context.
|
|
||
| This new representation is mostly useful when the structure it is put on must be | ||
| used in FFI context as a wrapper to the underlying type without actually being | ||
| affected by any ABI semantics. |
This comment has been minimized.
This comment has been minimized.
nagisa
Sep 26, 2016
•
Contributor
Oh, so you expect to use this when passing stuff into foreign functions… Then a multiple questions arise, for example:
#[repr(transparent)] struct Transp<T>(T);
extern { fn banana(x: Transp<SomeRustType>); } // does the non-c-types lint warn? probably yes, but not specified in the RFC
extern { fn banana(x: Transp<SomeCType>); } // does the non-c-types lint warn? probably no, but not specified in the RFC
#[repr(transparent)] struct TranspPack(PackedTy); // is packed? Probably yes.
#[repr(packed,transparent)] struct TransPack(SomeTy); // is packed? No idea.
#[repr(transparent, align="128")] struct TransU32(u32); // is valid? If so, is alignment 128 or `align_of::<u32>()`? What happens if native alignment of u32 is greater than alignment specified for the transparent newtype?
This comment has been minimized.
This comment has been minimized.
nox
Sep 26, 2016
Author
Contributor
This RFC specifies that Transp<SomeRustType> should warn because it says the representation of Transp is whatever the representation of its single field is. So if the single field is not a proper C type, using it in FFI should warn.
But yeah, it should mention that no other repr should be used with it, so no align nor packed.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I have no idea how to specify more, feel free to amend or give me directions. |
This comment has been minimized.
This comment has been minimized.
camlorn
commented
Sep 26, 2016
|
I agree with the people saying this should only apply to I am working with @eddyb on optimizing struct layout, the first PR of which just merged into the compiler. While we could apply This feels really hacky and temporary, and hacky/temporary-feeling things that we have a need for seem like a good time to revisit the general pattern. There is never any reason to leave |
This comment has been minimized.
This comment has been minimized.
What? No, If I have a newtype without an explicit representation, I can't pass it to C functions. If I have a newtype with |
This comment has been minimized.
This comment has been minimized.
That doesn't help to use |
This comment has been minimized.
This comment has been minimized.
|
What about extra zero-sized fields? Out of scope? #[repr(transparent)]
struct Wrapper<T: SomeKindaMarker> {
wrapped: i32,
_ghost: PhantomData<T>
} |
This comment has been minimized.
This comment has been minimized.
|
@camlorn This is exactly the newtype unpacking optimization I had in mind, which requires writing a not-entirely-recursive algorithm to handle cycles. It's pretty nasty stuff and I was waiting for old trans to die. For everyone else: there's some confusion here: The reason that doing it automatically is not enough, and which @nox should most definitely make it the prime motivation of this RFC is that depending on it leaks implementation details by default. If I have: pub struct Opaque(T);Then why should This is why you need |
This comment has been minimized.
This comment has been minimized.
camlorn
commented
Sep 26, 2016
|
So shouldn't we just do a proper newtype, then say that If things like My problem here is that it feels like this takes a step in the direction of enshrining the single-field struct as a newtype forever. This is workable if we then do stuff like accept #1406 in some form, which may have some uses anyway. But it feels like this is one of those decision points: accepting this without a dedicated newtype makes it more likely that we never will. Also, unless it's already named, I hereby define FFI-capable type to refer to any type which can be safely used with the C FFI. Run with it or not as you like. |
This comment has been minimized.
This comment has been minimized.
|
@camlorn But these types with private fields are not newtypes in the classical sense: it's not just a type disambiguator, but the field is opaque modulo the ABI implications of "FFI-capable" sounds good, cc @steveklabnik. |
This comment has been minimized.
This comment has been minimized.
Sure, nobody ought to rely on default behaviour if it involves repr(Rust) stuff, whereas
…that being said, @camlorn’s critique seems very valid to me. There’s no reason why some alternative approach (e.g. At this point I would like to note that adding (in some cases) and removing (in all cases) Interestingly, AFAIR rustdoc does not in any way make note of any representational particularities. These are part of the API and probably should be prominently presented to everyone interested. Perhaps fixing that would help to add that weight I was speaking of in the previous paragraph. Bikeshed time. |
This comment has been minimized.
This comment has been minimized.
|
You mean an ABI-breaking change? Isn't that the entire point of the
OTOH, The problem is that nobody agrees what a newtype is. Is an |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
camlorn
commented
Sep 26, 2016
|
@arielb1 Under this definition, Nevertheless, two things should be noted. A newtype meeting my definition should be adequate here because any way we do that is going to have to conceptually look like a struct with one field. And because newtypes are a subset, going from I don't have a problem with us keeping it as it is and never having You could easily counter by saying "well, okay, trait forwarding will only work on single-field structs". Which is fine as far as it goes. But that's not a general feature, and I will argue for more general features over more specific ones. If we choose to leave newtypes as single-field structs then add newtype trait forwarding, someone has to answer the question of why we couldn't make it general. All I'm trying to say is that I think it's necessary for someone with the power to make decisions to make a definitive decision as to what kind of newtypes we want longterm before we consider this RFC: single-field structs plus general-case machinery that affects more than newtypes or a dedicated mechanism for newtypes with newtype-specific functionality. |
This comment has been minimized.
This comment has been minimized.
With |
This comment has been minimized.
This comment has been minimized.
|
@camlorn Replace every instance of "newtype" in that RFC by "structure with one field". This RFC doesn't need to be blocked by things that are completely unrelated like trait forwarding. |
This comment has been minimized.
This comment has been minimized.
camlorn
commented
Sep 26, 2016
|
@nox So, anyway, carry on. |
This comment has been minimized.
This comment has been minimized.
|
Though the two are different in meaning as far as I can tell, it may be worth considering the similarity in naming to nrc's proposal for |
nrc
added
the
T-lang
label
Sep 27, 2016
nrc
reviewed
Sep 27, 2016
| [summary]: #summary | ||
|
|
||
| Extend the existing `#[repr]` attribute on newtypes with a `transparent` option | ||
| specifying that the type representation is the representation of its only field. |
This comment has been minimized.
This comment has been minimized.
nrc
Sep 27, 2016
Member
I find this RFC confusing, I think because I am unclear what you mean by representation. Rust uses interior allocation for structs (which while not guaranteed because of no formal ABI, you can definitely rely on). So in that respect, the in-memory representation does not change at all. But you seem to be proposing changing the how newtypes interact with FFI? It might be better to write the RFC with that focus.
This comment has been minimized.
This comment has been minimized.
nox
Sep 27, 2016
Author
Contributor
#[repr(C)] can also influence how values are passed to and returned from FFI functions, notably struct Foo(f64) on ARM64.
This comment has been minimized.
This comment has been minimized.
parched
Sep 28, 2016
I think I agree with @nrc here. Unless I'm confused, this has nothing to do with 'repr' but is purely a calling convention issue.
However, if you change the calling convention like this then it is incompatible with FFI. Could you possibly give a more detailed example how you would use this in a FFI case?
This comment has been minimized.
This comment has been minimized.
|
I'll write a FFI example, but calling convention is also affected by representation, cf. the example where f64 and Struct(f64) don't behave the same in C. |
nox
force-pushed the
nox:repr-transparent
branch
from
a70131c
to
481d8f5
Sep 28, 2016
This comment has been minimized.
This comment has been minimized.
|
Added the |
nox
force-pushed the
nox:repr-transparent
branch
from
481d8f5
to
4ad9e69
Sep 28, 2016
This comment has been minimized.
This comment has been minimized.
parched
commented
Sep 28, 2016
•
or rather, calling convention is affected by the type, f64 and Struct(f64) have the same layout but are different types so (can) have different calling conventions. Why not just implement |
rfcbot
added
the
final-comment-period
label
Apr 27, 2017
This comment has been minimized.
This comment has been minimized.
rfcbot
commented
Apr 27, 2017
|
|
This comment has been minimized.
This comment has been minimized.
rfcbot
commented
May 7, 2017
|
The final comment period is now complete. |
This comment has been minimized.
This comment has been minimized.
|
An alternative that I expected to find discussed in the RFC but did not is to declare that in general, (i.e., no matter the In fact, #1789 does. That other RFC relies on EDIT: I see this has been brought up in discussion, but not finally resolved yet. All right. |
This comment has been minimized.
This comment has been minimized.
|
I agree with @RalfJung, I think this should be implicit as part of the Rust ABI. This would not apply to |
This comment has been minimized.
This comment has been minimized.
|
@Amanieu the main motivation for this feature is FFI use cases where it is normally very bad to pass a |
aturon
referenced this pull request
Jul 3, 2017
Closed
Tracking issue for RFC 1758: Specify `repr(transparent)` #43036
aturon
merged commit 2c49bef
into
rust-lang:master
Jul 3, 2017
This comment has been minimized.
This comment has been minimized.
|
Huzzah! This RFC has been merged. Tracking issue. (Apologies for the delay!) |
cramertj
referenced this pull request
Nov 10, 2017
Open
Document pointer cast safety in std::path #45910
nox
deleted the
nox:repr-transparent
branch
Nov 30, 2017
This comment has been minimized.
This comment has been minimized.
briansmith
commented
Dec 16, 2017
|
Sorry, it's been forever since we had that conversation. Maybe I'm overlooking something, but I think #[repr(transparent, align=16)]
struct Aes256Key([u8; 32]);
#[repr(transparent, align=16)]
struct AesNonce([u8; 16]);
// Keep this in sync with the C definition in aes.h
#[repr(C)]
struct Aes256Context {
key: Aes256Key,
...
nonce: AesNonce,
...
}
extern aes256Encrypt(context: &mut Aes256Context, ...);I'm not sure how one would express this otherwise. |
This comment has been minimized.
This comment has been minimized.
briansmith
commented
Dec 16, 2017
|
(Sorry, the previous comment was intended for another issue.) |
This comment has been minimized.
This comment has been minimized.
|
What C declaration is this intended to match? Aka why can't you use #[repr(C, align=16)]
struct AesNonce([u8; 16]); |
This comment has been minimized.
This comment has been minimized.
briansmith
commented
Dec 17, 2017
Because, for example, |
This was referenced Dec 30, 2017
This comment has been minimized.
This comment has been minimized.
tbelaire
commented
Jan 24, 2018
•
|
The RFC says that this should be illegal. #[repr(transparent, align = "128")]
struct BogusAlign(f64);But this should be legal #[repr(transparent, align = "128")]
struct OverAligned(f64); // Behaves as a bare f64 with 128 bits alignment.I think you've made an error somewhere. |
nox commentedSep 26, 2016
•
edited by mbrubeck
Summary: Extend the existing
#[repr]attribute on newtypes with atransparentoptionspecifying that the type representation is the representation of its only field.
This matters in FFI context where
struct Foo(T)might not behave the sameas
T.Cc @eddyb @Manishearth
rendered
[edited to update rendered link]