-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add language support for bitfields #1449
Conversation
I like bit fields. Just some additional alternatives: Idea 1. Use a bitfield generator that builds an object with methods for manipulating bits from a bitfield specification. It could be either a macro or an external program, similar to protobuf or captnproto. As a result, it'd look pretty different from other rust syntax, but maybe that's an advantage. Idea 2. There might be future enhancements to the type system that allow a slightly prettier presentation as type parameters. I doubt this fits rust system that well, but just mentioning it. It might look like :
|
@burdges I already use a macro to handle bitfields in winapi. It is a bit more verbose than I'd like due to the lack of ident concatenation but it works fine otherwise https://github.com/retep998/winapi-rs/blob/master/src/usp10.rs#L78-L124 |
The rfc was written to be as unintrusive as possible so that adding it to the language proper doesn't add any complexity that could be avoided by using compiler plugins. In particular:
The approach with external tools is problematic:
In conclusion:
|
@retep998 It works fine because you only want to use it on windows and windows bitfields are particularly simple. The examples in the RFC already show that it's not at all simple on other platforms and consider the following example: struct X {
char a;
int b: 8;
short c;
}; On x86_64 linux this struct has a size of |
declared type of the field. It specifies the width of the field. | ||
|
||
Many details of the behavior of such fields are left to the implementation. | ||
However, the implementation is encouraged to follow the behavior of the dominant |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not just encouraged, but probably should be required, since if there's no guarantee it becomes useless for FFI which will likely be the primary consumer of bitfields.
@mahkoh Yes, it is indeed 12. I am thankful that Windows has simple rules. Anyway, this RFC looks fine to me. The syntax is reasonable. If winapi's minimum rust version ever increases to a version which has feature in stable, then I'll even use it since it'll make bitfield access more convenient than with the inherent methods of the macro solution. |
[design]: #detailed-design | ||
|
||
The attribute `#[bitfield(N)]` is added. Here, `N` can be any constant | ||
expression that evaluates to a value of type `usize`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Constant expressions in attributes look like a sufficiently large new feature.
(So this RFC in its current form is not as unintrusive as it wants to be)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eventually such things should be added anyway especially for attributes that calculate the alignment of types. Until then it's fine if this part is only partially implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, I'd like the have a solution which can be later extended to support "repr(Rust)" bitfields with specified and standardized ABI (not "implementation defined"), FFI is certainly not the only application of bitfields.
If constant expressions in attributes are indeed implemented, then this seems to be a good enough for native Rust bitfields as well. (Maybe there are better ways, I don't know)
While I see the necessity for something like this, I'm strongly against going with the easy way (attributes simulating c bitfields). Ada has a different and much more powerful system. It has representation specifications which are completely independent of the regular specification. Ada's system has two advantages over the one suggested here:
|
How would you write the following bitfield with your idea: struct X {
char a;
int b: 8;
short c;
}; Make up some syntax if necessary. |
Copying Ada's syntax somewhat: struct X {
a: u8,
b: i32,
c: i16,
}
for X use {
a: 0..8,
b: 8..16,
c: 16..32,
} Optionally there could be "simplifications" for single bitfields: for X use {
a: 0..8,
b: 8..16,
c: 16..31,
d: 31, // just one bit
} And "length" assertions either as static_assert!(X::b::BITS == 8); or directly for X use {
a: 0..8,
b: 8..16 where b::BITS == 8,
c: 16..32,
} |
Then those bitfields are not C compatible and require the user to write platform dependent code for every platform (x86_64 linux, x86_64 windows, arm linux, arm windows, etc.) The RFC is supposed to solve this. |
@oli-obk |
|
||
For the purpose of overflow checking, bitfields are treated as signed or | ||
unsigned integers of their declared width. In this context, the only value that | ||
should be written to a bitfield of size `0` is `0`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, doing something else should panic? I think this should specify what happens, not what users of the bitfields should do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens in non-debug builds where we don't panic on overflow? Is the write just ignored?
Right, ignore that. Just for C-interop this is fine. |
Is this really needed? I thought C bitfields were an esoteric feature nobody uses. I've never encountered them in any API. |
cough Windows API cough |
@pornel they seem to be common in very low-level programming - kernels, drivers, etc. That is a space where Rust would be well-suited but is currently a bit weak. Also useful for FFI/interacting with C, which is another slightly weak area. I'd like to see some kind of bitfields to help address those weaknesses. |
@nrc indeed. Though bitfields for platform compat (need to match how the platform lays out bitfields) might not be the same as how we'd want bitfields for drivers/hardware interaction (need to always be laid out the same way, not something platform specific). |
I am in favor of this RFC. A syntax for packed bitfields (useful for assemblers, networking, drivers, etc) would also be useful. I like Ada's syntax for this, but it is rather verbose. One note is that in most applications of packed bitfields, one does not want any padding, so needing to add padding should be either a compile-time error or a lint that is deny-by-default. |
Values stored in a bitfield have a binary representation using the `m` bits it | ||
occupies in its memory location. | ||
|
||
The `&` and `&mut` operators cannot be applied to bitfields. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And presumably implicit borrows too
Nominating for discussion at the lang meeting. My thoughts:
|
I'm interested in a pure-rust solution for working with hardware (network packets, processor-specific data structures, etc). Quite unlike working with C, one wants representation details mandated ad nauseam for this. |
We need something like this for rust-lang/rust#27060 anyway.
This doesn't need to be quite so general. Maybe "the representation, padding and alignment of a bitfield field are target-dependent"?
The restriction that you can't borrow a bitfield is enough to for safety in safe Rust: that makes it impossible to concurrently mutate two bitfields in the same object. You have to be a little bit careful how exactly you do code generation for bitfields to make sure that writing to a bitfield doesn't overwrite bytes associated with other non-bitfield fields, but that's straightforward (and the C11 rules impose a similar restriction on C compilers). |
I still personally feel that this RFC is undershooting with ambitions here. I feel like there is a strong potential for a superset of functionality (e.g. arbitrary structure layout) while at the same time solving the issue of bit fields in a more convenient way. |
Err the wrapper method is orthogonal to the payload type. I'm a bit confused when it needs to take a periph.reg.update(|r, w| w.foo = r.bar + 1)) Even if the API is the same, it's still easier to generate code that can use this feature (resulting code is simpler). |
@Ericson2314 Oh you're absolutely right, disregard what I said. |
@whitequark I do agree that C-style bitfields would not be sufficient for MMIO usage. However, my conclusion there is instead "we should do better than C", rather than "we should give up". C bitfields have multiple problems, most notably a lack of any way to specify the granularity of the operation. I think we can fix that, and I don't think it'll require extensive attributes to do so. Beyond that, I would also like to see this to simplify interoperability with C via FFI (and in particular, |
I think you misunderstood me. My conclusion is not "we should give up", it's "getting this to work is not really worth the payoff". |
@whitequark I understood your argument, but I disagree; I think this is important to get right and to make simple. |
@joshtriplett For me, based on what I've been proposing, "doing the right thing for hardware" ( |
@Ericson2314 That's entirely fair. And I'd love to see an extension that's good enough for C FFI interoperability, with some thoughts on forward-compatibility with hardware MMIO. |
Err because approaches to each are so different, I don't think there's much forward compatibility to worry about. I'd start with hardware/marshalling, because C bitfields (I agree with what others said) are basically a misfeature in that they are too complex and unintuitive, and any feature that wants to interop with them basically has to mimic them exactly---there's no way to trim the fat without losing the flavor. Maybe it would be very useful, but replicating the ugliness sounds like a boring chore, and I wouldn't want developers of new non-C-interfacing code to be tempted to use them because of the absence of |
Err, wow. An explosion of discussion here. You'll have to excuse me as some of it goes over my head and beyond my experience. I do need to state what I thought I understood the purpose of this to be, and that is solely to enable use of C bitfields over FFI. This is the last missing piece of the puzzle for quite a few bindings, especially in the GNOME world with such things as struct _GIOChannel
{
/*< private >*/
gint ref_count;
GIOFuncs *funcs;
gchar *encoding;
GIConv read_cd;
GIConv write_cd;
gchar *line_term; /* String which indicates the end of a line of text */
guint line_term_len; /* So we can have null in the line term */
gsize buf_size;
GString *read_buf; /* Raw data from the channel */
GString *encoded_read_buf; /* Channel data converted to UTF-8 */
GString *write_buf; /* Data ready to be written to the file */
gchar partial_write_buf[6]; /* UTF-8 partial characters, null terminated */
/* Group the flags together, immediately after partial_write_buf, to save memory */
guint use_buffer : 1; /* The encoding uses the buffers */
guint do_encode : 1; /* The encoding uses the GIConv coverters */
guint close_on_unref : 1; /* Close the channel on final unref */
guint is_readable : 1; /* Cached GIOFlag */
guint is_writeable : 1; /* ditto */
guint is_seekable : 1; /* ditto */
gpointer reserved1;
gpointer reserved2;
}; If there is a way to do bitfields better (and it certainly looks like there is), can this be relegated to another RFC in the future? To restate from this RFC;
Thus it is my hope to have the #[repr(C)]
pub struct GIOChannel {
pub ref_count: c_int,
pub funcs: *mut GIOFuncs,
pub encoding: *mut c_char,
pub read_cd: GIConv,
pub write_cd: GIConv,
pub line_term: *mut c_char,
pub line_term_len: c_uint,
pub buf_size: size_t,
pub read_buf: *mut GString,
pub encoded_read_buf: *mut GString,
pub write_buf: *mut GString,
pub partial_write_buf: [c_char; 6],
#[bitfield(1)] pub use_buffer: u8,
#[bitfield(1)] pub do_encode: u8,
#[bitfield(1)] pub close_on_unref: u8,
#[bitfield(1)] pub is_readable: u8,
#[bitfield(1)] pub is_writeable: u8,
#[bitfield(1)] pub is_seekable: u8,
pub reserved1: gpointer,
pub reserved2: gpointer,
} I think @mahkoh was on the right track here. I don't see any other way to do this without adding a large amount of complexity to written code. And please, the purpose of the RFC is for FFI with C, nothing more or less than that. If we can get this done, then it opens up a world of possibilities to migrate a lot of things to Rust that rely on the use of such structs as shown above. |
@whitequark if this RFC is specifically for C style bitfields (and largely only for FFI), will that affect your use cases for embedded? This is an area I am interested in personally. |
Yes. C style bitfields are very unergonomic for embedded dev. For example, a baseline requirement for such an interface, in my view, is an ability to define a field as spanning bits n...n+m, regardless of where other fields are laid out, so that I can add, remove, or even |
I'd very much like to see this RFC clarified and the alternatives and tradeoffs filled out. I see no problem with having an RFC specifically for C-FFI-compatible bitfields, with the explicit acknowledgement that more work would be needed to support hardware bitfields. |
@whitequark Are you familiar with Ada record representation clauses? Specifically: for Device_Register use
record
Ready at 0 range 0 .. 0;
Error at 0 range 1 .. 1;
-- Reserved bits
Data at 0 range 16 .. 31;
end record; (Read It sounds a lot like what you want, since the padding bits are completely implied here. It would be cool if Rust had a general system like this for those cases where you want total layout control. EDIT: Ah, it appears this was already mentioned earlier. |
@whitequark I'm not sure how you mean with that - is there an example I can look at? If there were to a come a future RFC geared towards the hardware issues, how do you think the syntax @joshtriplett I am collating much of the discussion here in to my notes and seeing what I need to do. May be a few days since I also have to study (and do my GSoC project work), but I am definitely leaning towards being specifically C-FFI only for this RFC. It would be nice to find a compromise but that doesn't look likely. |
This syntax would be completely orthogonal and not used in interfacing with hardware.
No, but what you describe is a very good start. There are some subtleties, e.g. it is perfectly reasonable for fields to overlap, even fully. |
Yeah, for sure. I imagine having a completely general layout toolkit that could simulate |
The split between C-FFI bitfields and layout control for hardware or other reasons seems pretty irreconcilable. The C-FFI notion of bitfields is about a layout the programmer can't know, since C bitfield representation varies across platforms, whereas @whitequark's notion is for when the programmer has very specific knowledge of the exact layout they need or want. (I mean, technically the programmer could try to match C layout with an Ada-like system and their own collection of There might still be some room for overlapping syntax, but it seems like it might be more confusing than helpful. So @Luke-Nukem's idea of splitting into separate RFCs makes sense to me. |
This discussion actually seems very familiar. When RFC 1444 discussed unions, there were suggestion for complete structure layout control, allowing the arbitrary placement of fields (including overlapping, which would allow for unions). In my opinion, I think the answer there applies here as well: I think we want both of these things, but separately. I would propose the following: for this RFC, let's focus on C FFI bitfields, for which we just need an updated RFC that discusses alternatives/documentation/etc. And someone interested in modeling hardware registers may wish to propose an additional RFC with the level of control they'd propose as sufficient for that task. |
Thanks for the input so far everyone (giving me a lot to digest). Anyone willing to mentor me through this? My time is a little restricted due to study and GSoC, but we could easily do this through email or even matrix clients. |
I haven't followed the thread in detail, but I wanted to weigh in here. We've had a lot of success having a highly expressive underlying mechanism, and then providing more "sugary" high-level forms that can be understood as a kind of sugar using that. And I agree with @joshtriplett's characterization that we resolved similar issues with unions in this way (though the "underlying mechanism" is still waiting to be proposed.) There are a few dangers to starting with the sugar, though:
These kinds of concerns ended up shifting our strategy with In any case, I raise these concerns not to block the RFC, but just as something we need to keep in mind prior to stabilization. If there's clarity that we want to do something in this area, and there's a well-understood, sugary subset, experimenting with that subset seems wholly beneficial. But we need to get on the same page with the endpoint before stabilization. |
@aturon I agree with this principle, but in this case I don't think it applies in the same way. You could take the same view with existing I tried to articulate this difference in an earlier comment:
In other words, both features provide something the other doesn't - in one, target knowledge, in the other, control - and neither is a subset of the other.
I agree we shouldn't block, and the primary thing to worry about before stabilization is that the two syntaxes jive with each other (i.e. they should seem like they belong in the same language 😃). Also, I suppose, that they build on top of the same rustc implementation work as much as possible, to avoid duplicated effort. |
I'd imagine that we'd still want the C version's fields to be properly-sized |
I think the question really boils down to whether a given C bitfield just has the given type with special behavior on read/write to only read/write that many bits (with some sort of overflow checking in debug), or whether they have special |
Hmm? If, given the other motivation I mention above we get those types anyways, I don't see much casting involved using them for this? |
New RFC: #3064 |
@flukejones I was in need of something similar for my Windows related crate, as Windows extensively uses bit fields in API parameters and structures. I added an example of your use case to my bitfield crate, which supports:
|
Add support for bitfields via a single per-field attribute.