Skip to content

Conversation

@maciektr
Copy link
Contributor

@maciektr maciektr commented Oct 16, 2025

The test attr is a hot path, so it may make sense to avoid forcing the expansion to happen twice for every test. The time penalty is not big, but seems easy enough to avoid.

Events breakdown for OZ before / after:

image image

@maciektr maciektr force-pushed the maciektr/plugin-micro-opt branch from 6e03d13 to a0d58c9 Compare October 16, 2025 19:57
@maciektr maciektr marked this pull request as ready for review October 16, 2025 20:23
@maciektr maciektr requested a review from a team as a code owner October 16, 2025 20:23
Comment on lines +111 to +114
if snforge_std::_internals::is_config_run() {
#if_content

return;
Copy link
Member

Choose a reason for hiding this comment

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

I think we should be able to do the same in fuzzer wrapper in wrapper.rs file.

Replace appending #[#internal_config_attr] there with simply creating an another if branch like you did here.

Then we could get rid of the internal config statement attribute entirely.

Copy link
Member

Choose a reason for hiding this comment

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

cc @ddoktorski @franciszekjob please check if I'm not making things up

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, I think that should be possible to do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cptartur This sounds like a good refactoring idea, but tbh I don't think this matters at all apart from code cleanliness - you would normally expect a lot more test attrs than any other.

Especially as I would need to have some equivalent of

pub fn get_statements(db: &dyn SyntaxGroup, func: &FunctionWithBody) -> (String, String) {
that accepts TokenStream instead of FunctionWithBody to keep the logic exactly the same.

Copy link
Member

Choose a reason for hiding this comment

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

We can do that refactor at a later date then.

Comment on lines +111 to +114
if snforge_std::_internals::is_config_run() {
#if_content

return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, I think that should be possible to do

@maciektr maciektr requested a review from cptartur October 21, 2025 07:54
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