Refactor wasmprinter to rely less on a String output
#1592
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Delegate the space-before-immediate to printing the immediate itself rather than forcing a space between each immediate. Avoids the need to `pop()` or otherwise test if space is already present in situations where immediates are omitted.
Previously operators were printed to a temporary buffer and then depending on what happened it would conditionally go into the actual module or instead be discarded (e.g. the last `end`). This commit refactors to instead avoid a temporary buffer entirely and unconditionally emit instructions to the real output. This is done by restructuring the printing slightly by having "before_op" and "after_op" hooks while printing which are used to manage nesting/newlines instead of the outer loop in the main printer. This is combined with a few other minor things such as a `is_end_then_eof` method which is used to avoid printing the final `end`. One minor test update happened here with a `delegate` instruction from the legacy exception-handling proposal which I think is a bugfix given my read of the older proposal.
sunfishcode
reviewed
Jun 5, 2024
sunfishcode
approved these changes
Jun 5, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit aims at starting to refactor
wasmprinterto be able to output to a stream rather than unconditionally to aString. The fact that the current output is always an in-memoryStringis leveraged in a number of places in the internals to undo prints or capture temporary prints. These were all the easiest thing to do at the time but do not play well with the idea of streaming output. This commit instead refactorswasmprinterto remove all these bits and be more naturally oriented around "just stream out bytes at that's it". This is achieved with some internal refactorings, reorganization, and juggling some responsibilities.A future PR will switch the internals to actually be
std::io::Writeor, my final goal,termcolor::WriteColor