refactor(ast): calculate which structs are transparent in ast_tools#23958
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
916651c to
ee3fd99
Compare
9d0721b to
e575745
Compare
|
@claude review |
Merge activity
|
ee3fd99 to
735949f
Compare
…#23958) Similar to #23957. Move work from `#[ast]` macro into `ast_tools`. Calculate whether a struct can be marked `#[repr(transparent)]` or not in `ast_tools` and pass that info to `#[ast]` macro via the `StructDetails` struct. This has 2 advantages: 1. Avoids having to iterate over all fields of each struct in the macro. 2. It's more accurate. `ast_tools` knows the size of fields, so it can exclude ZST fields from the count. A struct like this will now be marked `#[repr(transparent)]` despite having more than 1 field, because 2 of the fields contain ZSTs: ```rust struct Foo<'a> { num: u32, _align: [usize; 0], _marker: PhantomData<&'a ()>, } ``` Moving work from macro into `ast_tools` is preferable because `ast_tools` runs only after changes to AST, whereas the macro code runs every time `oxc_ast` crate is compiled.
e575745 to
d9a1829
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9a1829a1b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
735949f to
e94b6cd
Compare
d9a1829 to
a30c73f
Compare
…#23958) Similar to #23957. Move work from `#[ast]` macro into `ast_tools`. Calculate whether a struct can be marked `#[repr(transparent)]` or not in `ast_tools` and pass that info to `#[ast]` macro via the `StructDetails` struct. This has 2 advantages: 1. Avoids having to iterate over all fields of each struct in the macro. 2. It's more accurate. `ast_tools` knows the size of fields, so it can exclude ZST fields from the count. A struct like this will now be marked `#[repr(transparent)]` despite having more than 1 field, because 2 of the fields contain ZSTs: ```rust struct Foo<'a> { num: u32, _empty: [u8; 0], _marker: PhantomData<&'a ()>, } ``` Moving work from macro into `ast_tools` is preferable because `ast_tools` runs only after changes to AST, whereas the macro code runs every time `oxc_ast` crate is compiled.
a30c73f to
ca13cdd
Compare
…#23958) Similar to #23957. Move work from `#[ast]` macro into `ast_tools`. Calculate whether a struct can be marked `#[repr(transparent)]` or not in `ast_tools` and pass that info to `#[ast]` macro via the `StructDetails` struct. This has 2 advantages: 1. Avoids having to iterate over all fields of each struct in the macro. 2. It's more accurate. `ast_tools` knows the size of fields, so it can exclude ZST fields from the count. A struct like this will now be marked `#[repr(transparent)]` despite having more than 1 field, because 2 of the fields contain ZSTs: ```rust struct Foo<'a> { num: u32, _empty: [u8; 0], _marker: PhantomData<&'a ()>, } ``` Moving work from macro into `ast_tools` is preferable because `ast_tools` runs only after changes to AST, whereas the macro code runs every time `oxc_ast` crate is compiled.

Similar to #23957.
Move work from
#[ast]macro intoast_tools. Calculate whether a struct can be marked#[repr(transparent)]or not inast_toolsand pass that info to#[ast]macro via theStructDetailsstruct. This has 2 advantages:ast_toolsknows the size of fields, so it can exclude ZST fields from the count.A struct like this will now be marked
#[repr(transparent)]despite having more than 1 field, because 2 of the fields contain ZSTs:Moving work from macro into
ast_toolsis preferable becauseast_toolsruns only after changes to AST, whereas the macro code runs every timeoxc_astcrate is compiled.