cranelift-meta: Don't let-bind operand definitions (#5969)

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.
This commit is contained in:
Jamey Sharp
2023-03-09 09:24:19 -08:00
committed by GitHub
parent 9141fcf8cf
commit f877141668
2 changed files with 804 additions and 724 deletions

View File

@@ -159,15 +159,15 @@ impl InstructionBuilder {
} }
} }
pub fn operands_in(mut self, operands: Vec<&Operand>) -> Self { pub fn operands_in(mut self, operands: Vec<Operand>) -> Self {
assert!(self.operands_in.is_none()); assert!(self.operands_in.is_none());
self.operands_in = Some(operands.iter().map(|x| (*x).clone()).collect()); self.operands_in = Some(operands);
self self
} }
pub fn operands_out(mut self, operands: Vec<&Operand>) -> Self { pub fn operands_out(mut self, operands: Vec<Operand>) -> Self {
assert!(self.operands_out.is_none()); assert!(self.operands_out.is_none());
self.operands_out = Some(operands.iter().map(|x| (*x).clone()).collect()); self.operands_out = Some(operands);
self self
} }

File diff suppressed because it is too large Load Diff