Skip to content
This repository has been archived by the owner on Jul 12, 2024. It is now read-only.

Remove most of the state from WasmContext. #138

Merged
merged 11 commits into from
May 22, 2024

Conversation

sjrd
Copy link
Collaborator

@sjrd sjrd commented May 21, 2024

The only state that is left in WasmContext is directly related to code generation, rather than building knowledge about the program.


Based on #137, but otherwise reviewable.

@sjrd sjrd requested a review from tanishiking May 21, 2024 10:00
sjrd added 11 commits May 21, 2024 14:17
We can allocate the IDs without looking at the body of methods, so
this allows to have a fixed assignment from the start.
We can use `stringPool.size` instead.
We can compute that in `Emitter.genStartFunction` instead with no
additional cost.
The use sites that need that information already have a
`LinkedClass` from which they can get it.
Compute the two internal information that needed it from
`Preprocessor` instead.
Instead, keep the stateful computations inside `Preprocessor`, and
freeze the state when constructing `WasmContext`.

This means that it is the responsibility of `Preprocessor` to
create the instance of `WasmContext`, instead of `Emitter`.
We already have the set of all imported modules. There is no need
to compute it while generating code.
We can recompute it in `genStartFunction` instead.
@sjrd sjrd force-pushed the remove-state-from-wasmcontext branch from f4ccc1e to 892fdc5 Compare May 21, 2024 12:17
@sjrd
Copy link
Collaborator Author

sjrd commented May 21, 2024

Rebased.

Copy link
Owner

@tanishiking tanishiking left a comment

Choose a reason for hiding this comment

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

LGTM!

@tanishiking tanishiking merged commit 5a3d5f2 into tanishiking:main May 22, 2024
1 check passed
@sjrd sjrd deleted the remove-state-from-wasmcontext branch May 22, 2024 05:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants