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 toInst::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.
jameysharp requested elliottt for a review on PR #5969.
jameysharp requested cfallin for a review on PR #5969.
cfallin submitted PR review.
jameysharp merged PR #5969.
Last updated: Dec 23 2024 at 12:05 UTC