Skip to content

ctest: add tests for size and alignment of structs, unions, and aliases, and signededness of aliases. #4556

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mbyx
Copy link
Contributor

@mbyx mbyx commented Jul 17, 2025

Description

This PR adds tests for verifying size and alignment of struct, union, and typedef types, as well as signededness of alias types (if they are signed).

Sources

Checklist

  • Relevant tests in libc-test/semver have been updated
  • No placeholder or unstable values like *LAST or *MAX are
    included (see #3131)
  • Tested locally (cd libc-test && cargo test --target mytarget);
    especially relevant for platforms that may not be checked in CI

@rustbot rustbot added ctest Issues relating to the ctest crate S-waiting-on-review labels Jul 17, 2025
@mbyx
Copy link
Contributor Author

mbyx commented Jul 18, 2025

Not sure what the CI error is, doesn't seem to be related to the PR.

Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

Few small things but this looks pretty good

uint32_t ctest_signededness_of__{{ alias.id }}(void) {
return ((({{ alias.c_ty }}) -1) < 0);
}
{%- endfor +%}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is missing a trailing newline.

Does your editor have an option to insert this by default? E.g. vscode has files.insertFinalNewline

// Return `1` if the type is signed, otherwise return `0`.
// Casting -1 to the aliased type if signed evaluates to `-1 < 0`, if unsigned to `MAX_VALUE < 0`
uint32_t ctest_signededness_of__{{ alias.id }}(void) {
return ((({{ alias.c_ty }}) -1) < 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a bit easier to read

Suggested change
return ((({{ alias.c_ty }}) -1) < 0);
{ alias.c_ty } all_ones = -1;
return all_ones < 0;

Comment on lines 332 to 334
"char" | "short" | "int" | "long" | "long long" | "int8_t" | "int16_t"
| "int32_t" | "int64_t" | "uint8_t" | "uint16_t" | "uint32_t" | "uint64_t"
| "size_t" | "ssize_t" => true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just add an arm s.starts_with("int") || s.starts_with("uint")

helper: &TranslateHelper,
) -> Result<(), TranslationError> {
for alias in helper.ffi_items.aliases() {
// && skip_signededness
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// && skip_signededness

Comment on lines 167 to 168
if helper.translator.is_signed(helper.ffi_items, &alias.ty)
&& !should_skip_signededness_test
Copy link
Contributor

Choose a reason for hiding this comment

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

Early continue to unnest a level

Suggested change
if helper.translator.is_signed(helper.ffi_items, &alias.ty)
&& !should_skip_signededness_test
if !helper.translator.is_signed(helper.ffi_items, &alias.ty) || should_skip_signededness_test {
continue;
}

@mbyx mbyx force-pushed the ctest-size-align-test branch from 2e05afe to 5ced981 Compare July 19, 2025 17:39
@mbyx mbyx force-pushed the ctest-size-align-test branch from 5ced981 to 48031a5 Compare July 19, 2025 17:44
@mbyx mbyx requested a review from tgross35 July 19, 2025 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ctest Issues relating to the ctest crate S-waiting-on-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants