{struct,union}: support #[repr(align(...))] and #[repr(packed)]

[cyphar]

As with #[must_use], this new support requires users to specify what
annotations they would like to apply when an alignment-related repr is
encountered.

In order to facilitate the fact that #[repr(align(...))] takes an
argument which will likely need to be included in the final annotation,
the user must specify a function-like macro which takes the alignment
value as an argument.

It's not clear to me how we should handle aligned enums, so I've
punted on those for now.

Fixes: #430
Signed-off-by: Aleksa Sarai cyphar@cyphar.com

[emilio]

Thanks, this is a great start! I need to take a closer look but looks great off-hand modulo my comments.

We should also add some documentation to docs.md.

[cyphar]

The changes you requested have been implemented, but I still have one remaining question (which appears to be a bit difficult to implement with the current way the parser is set up -- the parser doesn't know the full configuration of cbindgen).

Arguably it's not really correct to generate a non-opaque version of a structure without including its alignment (and especially if we don't include its packing) because that will just cause lots of problems when interacting between the C and Rust code. So ideally I think we should treat structs that have alignment as opaque if there isn't a matching alignment macro defined in the configuration. WDYT?

I implemented the above semantics. It wasn't too bad, but I've kept them in a separate commit to hopefully make it easier to review (and easier to revert if you don't like the way it was implemented).

[emilio]

This looks awesome, thanks!

Just some minor nits and questions about whether there's a use-case for the two different configs. If not I think just using the struct config would be fine, the same way we use the struct.derive_* config for enums that happen to be represented like unions, or the config.structure.rename_fields for unions already.

This looks good to me with those addressed, or if you don't have the time for that I can also do it after landing, your call :)

Thanks for this, and for splitting your commits in a way that it was a breeze to review :)

[emilio]

And to elaborate: the unfortunate bit that makes must_use weirder (I hope) than this, is that C compilers have different attributes for functions / enum / structs. In particular __attribute__((warn_unused_result)) works for functions, __attribute__((warn_unused)) works for structs / unions, and at least in the past we needed custom annotations/static analysis for enums.

[cyphar]

My one follow-up question is are you sure that you want to re-use the struct annotations instead of defining them globally? I'd find it a bit odd (as a user) that a field set in [struct] will affect unions (and eventually enums).

[emilio]

My one follow-up question is are you sure that you want to re-use the struct annotations instead of defining them globally? I'd find it a bit odd (as a user) that a field set in [struct] will affect unions (and eventually enums).

I don't know, I think it'd be a bit unfortunate to have to define the same thing three times to get packed / aligned structs to show up right...

On the other hand I see your point. I guess it depends on whether you see struct as "thing that generates a struct" (in which case rust ADTs will fall into it too, for example) or just as "rust struct".

Maybe there should be an [attributes] (or [layout]) section for these? Anyhow given the precedent, probably it is fine to just use struct for now, unless you feel strongly against it. I'd prefer if stuff just worked instead of having to duplicate the same configuration 3 times.

[cyphar]

Sorry, I should clarify -- what I meant was that I'm on-board to not having the same config option multiple times. My question was whether there was a reason to not put it in the main Config struct (or as you mentioned, a separate [layout] section)? I'd prefer to do the latter (either one), but if you think it makes sense I'll put it in [struct] (my usecase only involves structs anyway 😉 ).

[emilio]

I'm totally fine making it a separate section of the configuration, it's your call :)

[cyphar]

There, all the relevant changes have been made.

[emilio]

Looks great, thank you!

[emilio]

Let me know if you need a release with these changes and I can try to prepare one soon-ish.