Skip to content

Conversation

tlively
Copy link
Member

@tlively tlively commented Jul 16, 2024

When we switched to the new type printing machinery, we inserted this
extra space to minimize the diff in the test output compared with the
previous type printer. Improve the quality of the printed output by
removing it.

@tlively tlively requested a review from kripken July 16, 2024 01:49
Base automatically changed from threads-fix-encoding to main July 16, 2024 02:01
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

lgtm % questions

target = pick(wasm.functions).get();
}
if (target) {
if (heapType.isBasic()) {
Copy link
Member

Choose a reason for hiding this comment

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

Is this from another PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, from previous PRs. This PR is just the last two commits. I'll rebase and push.

Sub = -0x30, // 0x50
SubFinal = -0x31, // 0x4f
Shared = -0x24, // 0x65
Shared = -0x1b, // 0x65
Copy link
Member

Choose a reason for hiding this comment

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

This too?

tlively added 2 commits July 16, 2024 10:15
When we switched to the new type printing machinery, we inserted this extra space to minimize the diff in the test output compared with the previous type printer. Improve the quality of the printed output by removing it.
@tlively tlively force-pushed the remove-struct-space branch from eabe6db to 91a17e8 Compare July 16, 2024 17:15
@tlively tlively merged commit afef6d3 into main Jul 16, 2024
@tlively tlively deleted the remove-struct-space branch July 16, 2024 17:50
@gkdn gkdn mentioned this pull request Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants