Stream: git-wasmtime

Topic: wasmtime / PR #5969 cranelift-meta: Don't let-bind operan...

Wasmtime GitHub notifications bot (Mar 09 2023 at 01:51):

jameysharp opened PR #5969 from inline-operand-defs to main:

We've adopted this pattern in Cranelift's instruction definitions where we let-bind some calls to Operand::new and then later use them in one or more calls to Inst::new.

That pattern has two problems:

• It puts the type of each operand somewhere potentially far removed from the instruction in which it's used.
• We let-bind the same name for many different operands, compounding the first problem by making it harder to find _which_ definition is used.

So instead this commit removes all let-bindings for operand definitions and constructs a new Operand every time.

Constructing an Operand at every use means we duplicate some documentation strings, but not all that many of them as it turns out.

I've left the let-bound type-sets alone, so those are currently still shared across many instructions. They have some of the same problems and should be reviewed as well.

Wasmtime GitHub notifications bot (Mar 09 2023 at 01:51):

jameysharp requested elliottt for a review on PR #5969.

Wasmtime GitHub notifications bot (Mar 09 2023 at 01:51):

jameysharp requested cfallin for a review on PR #5969.

Wasmtime GitHub notifications bot (Mar 09 2023 at 02:25):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Mar 09 2023 at 18:30):

jameysharp merged PR #5969.

Last updated: Oct 23 2024 at 20:03 UTC