Skip to content

Fix per-field alignment attribute being dropped during transpilation - #1974

Merged
fw-immunant merged 1 commit into
immunant:masterfrom
user3polarjet:fix/preserve-field-aligned-attribute
Aug 18, 2026
Merged

Fix per-field alignment attribute being dropped during transpilation#1974
fw-immunant merged 1 commit into
immunant:masterfrom
user3polarjet:fix/preserve-field-aligned-attribute

Conversation

@user3polarjet

@user3polarjet user3polarjet commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

[...] my intent was to try to transpile x264 encoder to rust. It extensively uses alignment attributes for msvc, clang, gcc. And i hit this bug. It kinda pissed me off, so i tried to fix it [with an LLM, generating this change].

LLM-generated PR description follows.

Summary

  • c2rust preserved __attribute__((aligned(N))) on whole structs but silently dropped it when applied to an individual field, since the alignment was never captured by the AST exporter, threaded through CDeclKind::Field, or used when laying out the generated struct.
  • The AST exporter now encodes a field's manual alignment (mirroring what it already does for whole records).
  • The transpiler forces the generated struct's repr to align(M) (M = max manual alignment among its fields) and inserts the padding needed to reach the field's real byte offset, matching clang's layout without inflating the field's own size (an initial #[repr(align(N))]-wrapper-type approach was tried and rejected after direct testing showed it inflates the field's size to a multiple of N, which clang's per-field attribute does not).
  • __alignof__/_Alignof applied directly to such a field is also fixed to report the manual alignment instead of the field's natural type alignment.

Test plan

  • Added FieldAlignStruct / test_field_alignment to tests/unit/structs, covering alignment, offset, size, and read/write through the field.
  • Verified generated layout (alignment/offset/size) against real clang-compiled output directly (not just the Rust side) for a __attribute__((aligned(16))) field.
  • Confirmed the rest of tests/unit/structs/src/structs.c (existing struct/bitfield/packing/alignment coverage) still compiles unaffected.

Comment thread c2rust-transpile/src/translator/mod.rs
@user3polarjet
user3polarjet requested a review from Rua August 14, 2026 12:39
@fw-immunant

Copy link
Copy Markdown
Contributor

The commits here claim to be authored by "Your Name" from an account with activity set to private, and the interactions from the PR author here read like unedited LLM output without a spark of human involvement. While we do want this bug fixed, this is fairly disrespectful of the time spent by human collaborators here and it doesn't make me happy to see. I encourage the human responsible for the @user3polarjet account to drop by and say hello in their own words.

@fw-immunant
fw-immunant force-pushed the fix/preserve-field-aligned-attribute branch 3 times, most recently from 869c905 to a093987 Compare August 17, 2026 18:45

@fw-immunant fw-immunant left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code changes from the LLM look reasonable, including comments which seem informative and appropriate. This does seem to address the relevant issue, which is a deficiency we've wanted to fix for some time.

@fw-immunant

fw-immunant commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

CI is failing because the added test imports std::mem::offset_of, which was stabilized in rustc 1.77, after our target nightly (nightly-2023-04-15). I think in this nightly, offset_of! is in the prelude but not present in std::mem.

@fw-immunant
fw-immunant force-pushed the fix/preserve-field-aligned-attribute branch from a093987 to 45fe5d9 Compare August 17, 2026 20:37
@fw-immunant
fw-immunant merged commit 58f5db9 into immunant:master Aug 18, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants