Skip to content

Conversation

@k0kubun
Copy link
Member

@k0kubun k0kubun commented Sep 19, 2025

This PR fixes make zjit-test on builds that don't have the disasm feature.

@k0kubun k0kubun force-pushed the zjit-fix-disasm branch 3 times, most recently from 087e546 to 3566dd7 Compare September 19, 2025 20:49
@k0kubun k0kubun marked this pull request as ready for review September 19, 2025 21:12
@matzbot matzbot requested a review from a team September 19, 2025 21:12
let cb = compile(|cb| add(cb, X0, X1, X2));
assert_snapshot!(cb.disasm(), @" 0x0: add x0, x1, x2");
assert_snapshot!(cb.string(), @"2000028b");
cb.with_disasm(|disasm| assert_snapshot!(disasm, @" 0x0: add x0, x1, x2"));
Copy link
Member

Choose a reason for hiding this comment

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

Would it be even better if we have a marco like: assert_disasm_snapshot!(cb, @" 0x0: add x0, x1, x2")?

Copy link
Member Author

Choose a reason for hiding this comment

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

My understanding is that insta doesn't support updating a custom macro like that

Copy link
Member

Choose a reason for hiding this comment

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

Ah I see that makes sense. Thanks for the context 👍

Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

Thanks for fixing it!

@k0kubun k0kubun merged commit e44bec9 into ruby:master Sep 19, 2025
88 of 90 checks passed
@k0kubun k0kubun deleted the zjit-fix-disasm branch September 19, 2025 22:19
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.

2 participants