Stream: git-wasmtime

Topic: wasmtime / PR #8566 cranelift: Remove `next_fixed_nonallo...

view this post on Zulip Wasmtime GitHub notifications bot (May 07 2024 at 08:30):

jameysharp opened PR #8566 from jameysharp:remove-trivial-methods to bytecodealliance:main:

The next_fixed_nonallocatable method doesn't do anything any more and doesn't return anything so calls to it can just be deleted.

The with_allocs, allocate, and to_string_with_alloc methods are all trivial at this point, so inline them. The bulk of this change was performed this way:

git grep -lF '.with_allocs(' | xargs sed -i 's/\.with_allocs([^)]*)/.clone()/g'

In a couple cases, this makes the AllocationConsumer unused at these methods' call sites. Rather than changing function signatures in this PR, just mark those arguments as deliberately unused.

The number of structures being cloned here is unfortunate, and unnecessary now that we don't need to mutate any of them. But switching to borrowing them is a bigger change than I want to include here.

view this post on Zulip Wasmtime GitHub notifications bot (May 07 2024 at 08:30):

jameysharp requested cfallin for a review on PR #8566.

view this post on Zulip Wasmtime GitHub notifications bot (May 07 2024 at 08:30):

jameysharp requested wasmtime-compiler-reviewers for a review on PR #8566.

view this post on Zulip Wasmtime GitHub notifications bot (May 07 2024 at 15:17):

cfallin submitted PR review:

LGTM, thanks!

Re: cloning, I agree that taking borrows instead is the next step, but (as I'm sure you've thought through but stating for the record here) the original .with_allocs() also returned a copy so this shouldn't be a regression at least. I'm very much in favor of the incremental approach here :-)

view this post on Zulip Wasmtime GitHub notifications bot (May 07 2024 at 17:42):

jameysharp merged PR #8566.

Last updated: Dec 23 2024 at 13:07 UTC