Skip to content

ctest: Add extraction of relevant types. #4485

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

Merged
merged 1 commit into from
Jun 16, 2025

Conversation

mbyx
Copy link
Contributor

@mbyx mbyx commented Jun 13, 2025

Adds support for extracting relevant types (such as Structs, Unions, Aliases, etc.) from Rust code.

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

mbyx commented Jun 13, 2025

Would it be a good idea to add tests at this point for extraction as well? By just counting up how many aliases, structs, unions etc. FfiItems could find in the test inputs and if it matches previous runs.

@tgross35 tgross35 changed the title Add extraction of relevant types. ctest: Add extraction of relevant types. Jun 15, 2025
@tgross35
Copy link
Contributor

Would it be a good idea to add tests at this point for extraction as well? By just counting up how many aliases, structs, unions etc. FfiItems could find in the test inputs and if it matches previous runs.

A test wouldn't be bad. I would just make a single tests.rs unit test file and provide an input string of Rust code that includes one of each item, parse that to syn::parse_file, get it to a FfiItems instance, and assert that the items you expect are in there (just checking the ident is fine).

@mbyx mbyx requested a review from tgross35 June 16, 2025 07:17
@mbyx mbyx requested a review from tgross35 June 16, 2025 08:13
@mbyx
Copy link
Contributor Author

mbyx commented Jun 16, 2025

There seems to be some kind of bug with #[expect(unused)] when used with unit tests. When I keep it on a method that is only used in a unit test, it says that the method is actually in use so the lint isn't being upheld. But when I remove the #[expect(unused)] it says that the method is not in use.

@tgross35
Copy link
Contributor

That's not a bug - the type is indeed used when cfg(test) is set, and unused otherwise. So they need to be cfg_attr(not(test), expect(unused)).

@mbyx mbyx force-pushed the ctest-ffi-parsing branch from 8eeae00 to 2683528 Compare June 16, 2025 18:06
}

#[cfg(test)]
mod tests {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you put the tests in a separate tests.rs file? And then move the all_items.rs contents to a const ALL_ITEMS: &str = r#"..."#; in tests.rs so it doesn't need to be a standalone file. You can just skip the expand step.

This is the last fix then please squash and LGTM!

@tgross35
Copy link
Contributor

You will need to rebase as well to fix some of the CI here

@mbyx mbyx force-pushed the ctest-ffi-parsing branch from 858bb6a to 7538533 Compare June 16, 2025 18:18
@mbyx mbyx force-pushed the ctest-ffi-parsing branch from 7538533 to f5621c6 Compare June 16, 2025 18:18
@tgross35 tgross35 added the stable-unneeded This PR is applied to main but already exists on libc-0.2 label Jun 16, 2025
@tgross35
Copy link
Contributor

Thank you!

@tgross35 tgross35 enabled auto-merge June 16, 2025 18:19
@tgross35 tgross35 added this pull request to the merge queue Jun 16, 2025
Merged via the queue into rust-lang:main with commit 9cc3dab Jun 16, 2025
50 of 51 checks passed
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 stable-unneeded This PR is applied to main but already exists on libc-0.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants