rust-lang / rust-bindgen

Automatically generates Rust FFI bindings to C (and some C++) libraries.
https://rust-lang.github.io/rust-bindgen/
BSD 3-Clause "New" or "Revised" License
4.23k stars 679 forks source link

[RFC, V1] try to exclude packed attr for types that contain aligned types #2769

Open bertschingert opened 4 months ago

bertschingert commented 4 months ago

NOTE: I know this can use some cleanups and more documentation / comments, etc. but for now I'm just uploading it as an RFC to see if you like the general approach. If you do, then I'll clean it up, but if not I won't bother.


This patch handles some (but not all) cases of types with a packed attribute that contain a type with an align(N) attribute.

This uses information available in the Clang IR about types' layout to determine if a given type is likely to have an alignment attribute placed on it during the code generation phase. This is just a heuristic; the decision to actually place an alignment attribute depends on information that is not known until code generation, so the logic here may result in both false positives and false negatives.

Using the real information from codegen on whether an alignment attribute was placed on a child type is not possible in general, because the order in which types are generated is not guaranteed. In some cases code may be generated for parent types before code is generated for child types contained in that parent. Then, it will be impossible to make an accurate decision regarding whether to remove the packed attribute from the parent type.

The impact of a false negative is that an outer type may have a packed attr even when an inner type as an align attr. Such a type would not compile under rustc, but it already would not compile so this has no harmful impact.

The impact of a false positive is that an outer type may have its packed attr stripped needlessly, because no inner types actually have an align attr. Because we only remove the packed attr when we can confirm that there will be no change to the type's layout, this also should have no harmful impact.

bors-servo commented 3 months ago

:umbrella: The latest upstream changes (presumably 7e9043497297e04e91ae76dfe0d2e7998828e529) made this pull request unmergeable. Please resolve the merge conflicts.