Skip to content

Commit f7cc48a

Browse files
aborgna-qlmondada
authored andcommitted
refactor!: Reduce error type sizes (#2420)
Closes #2196 > the new clippy version releasing with 1.87 includes a new lint marking error types that are too big: > https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err > > This fails with a lot of our definitions. We should probably Box them when possible. The cost of the extra indirection is non important since error handling should not normally be in critical paths. This PR removes the clippy lint ignores from the workspace config. BREAKING CHANGE: Wrapped some error fields in `Box`es to reduce their footprint.
1 parent d93b009 commit f7cc48a

File tree

30 files changed

+226
-201
lines changed

30 files changed

+226
-201
lines changed

Cargo.toml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,6 @@ missing_docs = "warn"
3838
# https://github.com/rust-lang/rust-clippy/issues/5112
3939
debug_assert_with_mut_call = "warn"
4040

41-
# TODO: Reduce the size of error types.
42-
result_large_err = "allow"
43-
large_enum_variant = "allow"
44-
4541
[workspace.dependencies]
4642
anyhow = "1.0.98"
4743
insta = { version = "1.43.1" }

hugr-core/src/builder.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ pub enum BuildError {
189189
#[error("Found an error while setting the outputs of a {} container, {container_node}. {error}", .container_op.name())]
190190
#[allow(missing_docs)]
191191
OutputWiring {
192-
container_op: OpType,
192+
container_op: Box<OpType>,
193193
container_node: Node,
194194
#[source]
195195
error: BuilderWiringError,
@@ -201,7 +201,7 @@ pub enum BuildError {
201201
#[error("Got an input wire while adding a {} to the circuit. {error}", .op.name())]
202202
#[allow(missing_docs)]
203203
OperationWiring {
204-
op: OpType,
204+
op: Box<OpType>,
205205
#[source]
206206
error: BuilderWiringError,
207207
},
@@ -219,7 +219,7 @@ pub enum BuilderWiringError {
219219
#[error("Cannot copy linear type {typ} from output {src_offset} of node {src}")]
220220
#[allow(missing_docs)]
221221
NoCopyLinear {
222-
typ: Type,
222+
typ: Box<Type>,
223223
src: Node,
224224
src_offset: Port,
225225
},
@@ -244,7 +244,7 @@ pub enum BuilderWiringError {
244244
src_offset: Port,
245245
dst: Node,
246246
dst_offset: Port,
247-
typ: Type,
247+
typ: Box<Type>,
248248
},
249249
}
250250

hugr-core/src/builder/build_traits.rs

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,10 @@ pub trait Dataflow: Container {
213213
let num_outputs = optype.value_output_count();
214214
let node = self.add_hugr(hugr).inserted_entrypoint;
215215

216-
wire_up_inputs(input_wires, node, self)
217-
.map_err(|error| BuildError::OperationWiring { op: optype, error })?;
216+
wire_up_inputs(input_wires, node, self).map_err(|error| BuildError::OperationWiring {
217+
op: Box::new(optype),
218+
error,
219+
})?;
218220

219221
Ok((node, num_outputs).into())
220222
}
@@ -235,8 +237,10 @@ pub trait Dataflow: Container {
235237
let optype = hugr.get_optype(hugr.entrypoint()).clone();
236238
let num_outputs = optype.value_output_count();
237239

238-
wire_up_inputs(input_wires, node, self)
239-
.map_err(|error| BuildError::OperationWiring { op: optype, error })?;
240+
wire_up_inputs(input_wires, node, self).map_err(|error| BuildError::OperationWiring {
241+
op: Box::new(optype),
242+
error,
243+
})?;
240244

241245
Ok((node, num_outputs).into())
242246
}
@@ -253,7 +257,7 @@ pub trait Dataflow: Container {
253257
let [_, out] = self.io();
254258
wire_up_inputs(output_wires.into_iter().collect_vec(), out, self).map_err(|error| {
255259
BuildError::OutputWiring {
256-
container_op: self.hugr().get_optype(self.container_node()).clone(),
260+
container_op: Box::new(self.hugr().get_optype(self.container_node()).clone()),
257261
container_node: self.container_node(),
258262
error,
259263
}
@@ -662,8 +666,10 @@ fn add_node_with_wires<T: Dataflow + ?Sized>(
662666
let num_outputs = op.value_output_count();
663667
let op_node = data_builder.add_child_node(op.clone());
664668

665-
wire_up_inputs(inputs, op_node, data_builder)
666-
.map_err(|error| BuildError::OperationWiring { op, error })?;
669+
wire_up_inputs(inputs, op_node, data_builder).map_err(|error| BuildError::OperationWiring {
670+
op: Box::new(op),
671+
error,
672+
})?;
667673

668674
Ok((op_node, num_outputs))
669675
}
@@ -715,7 +721,7 @@ fn wire_up<T: Dataflow + ?Sized>(
715721
src_offset: src_port.into(),
716722
dst,
717723
dst_offset: dst_port.into(),
718-
typ,
724+
typ: Box::new(typ),
719725
});
720726
}
721727

@@ -746,7 +752,7 @@ fn wire_up<T: Dataflow + ?Sized>(
746752
} else if !typ.copyable() & base.linked_ports(src, src_port).next().is_some() {
747753
// Don't copy linear edges.
748754
return Err(BuilderWiringError::NoCopyLinear {
749-
typ,
755+
typ: Box::new(typ),
750756
src,
751757
src_offset: src_port.into(),
752758
});

hugr-core/src/builder/circuit.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,18 @@ pub struct CircuitBuilder<'a, T: ?Sized> {
2727
#[non_exhaustive]
2828
pub enum CircuitBuildError {
2929
/// Invalid index for stored wires.
30-
#[error("Invalid wire index {invalid_index} while attempting to add operation {}.", .op.as_ref().map(NamedOp::name).unwrap_or_default())]
30+
#[error("Invalid wire index {invalid_index} while attempting to add operation {}.", .op.as_ref().map(|op| op.name()).unwrap_or_default())]
3131
InvalidWireIndex {
3232
/// The operation.
33-
op: Option<OpType>,
33+
op: Option<Box<OpType>>,
3434
/// The invalid indices.
3535
invalid_index: usize,
3636
},
3737
/// Some linear inputs had no corresponding output wire.
3838
#[error("The linear inputs {:?} had no corresponding output wire in operation {}.", .index.as_slice(), .op.name())]
3939
MismatchedLinearInputs {
4040
/// The operation.
41-
op: OpType,
41+
op: Box<OpType>,
4242
/// The index of the input that had no corresponding output wire.
4343
index: Vec<usize>,
4444
},
@@ -143,7 +143,7 @@ impl<'a, T: Dataflow + ?Sized> CircuitBuilder<'a, T> {
143143

144144
let input_wires =
145145
input_wires.map_err(|invalid_index| CircuitBuildError::InvalidWireIndex {
146-
op: Some(op.clone()),
146+
op: Some(Box::new(op.clone())),
147147
invalid_index,
148148
})?;
149149

@@ -169,7 +169,7 @@ impl<'a, T: Dataflow + ?Sized> CircuitBuilder<'a, T> {
169169

170170
if !linear_inputs.is_empty() {
171171
return Err(CircuitBuildError::MismatchedLinearInputs {
172-
op,
172+
op: Box::new(op),
173173
index: linear_inputs.values().copied().collect(),
174174
}
175175
.into());
@@ -389,7 +389,7 @@ mod test {
389389
assert_matches!(
390390
circ.append(cx_gate(), [q0, invalid_index]),
391391
Err(BuildError::CircuitError(CircuitBuildError::InvalidWireIndex { op, invalid_index: idx }))
392-
if op == Some(cx_gate().into()) && idx == invalid_index,
392+
if op == Some(Box::new(cx_gate().into())) && idx == invalid_index,
393393
);
394394

395395
// Untracking an invalid index returns an error
@@ -403,7 +403,7 @@ mod test {
403403
assert_matches!(
404404
circ.append(q_discard(), [q1]),
405405
Err(BuildError::CircuitError(CircuitBuildError::MismatchedLinearInputs { op, index }))
406-
if op == q_discard().into() && index == [q1],
406+
if *op == q_discard().into() && index == [q1],
407407
);
408408

409409
let outs = circ.finish();

hugr-core/src/builder/dataflow.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ pub(crate) mod test {
437437
error: BuilderWiringError::NoCopyLinear { typ, .. },
438438
..
439439
})
440-
if typ == qb_t()
440+
if *typ == qb_t()
441441
);
442442
}
443443

hugr-core/src/envelope.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,15 +97,15 @@ fn gen_str(generator: &Option<String>) -> String {
9797
#[derive(Error, Debug)]
9898
#[error("{inner}{}", gen_str(&self.generator))]
9999
pub struct WithGenerator<E: std::fmt::Display> {
100-
inner: E,
100+
inner: Box<E>,
101101
/// The name of the generator that produced the envelope, if any.
102102
generator: Option<String>,
103103
}
104104

105105
impl<E: std::fmt::Display> WithGenerator<E> {
106106
fn new(err: E, modules: &[impl HugrView]) -> Self {
107107
Self {
108-
inner: err,
108+
inner: Box::new(err),
109109
generator: get_generator(modules),
110110
}
111111
}

hugr-core/src/extension.rs

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,8 @@ impl ExtensionRegistry {
136136
match self.exts.entry(extension.name().clone()) {
137137
btree_map::Entry::Occupied(prev) => Err(ExtensionRegistryError::AlreadyRegistered(
138138
extension.name().clone(),
139-
prev.get().version().clone(),
140-
extension.version().clone(),
139+
Box::new(prev.get().version().clone()),
140+
Box::new(extension.version().clone()),
141141
)),
142142
btree_map::Entry::Vacant(ve) => {
143143
ve.insert(extension);
@@ -408,8 +408,8 @@ pub enum SignatureError {
408408
/// A Type Variable's cache of its declared kind is incorrect
409409
#[error("Type Variable claims to be {cached} but actual declaration {actual}")]
410410
TypeVarDoesNotMatchDeclaration {
411-
actual: TypeParam,
412-
cached: TypeParam,
411+
actual: Box<TypeParam>,
412+
cached: Box<TypeParam>,
413413
},
414414
/// A type variable that was used has not been declared
415415
#[error("Type variable {idx} was not declared ({num_decls} in scope)")]
@@ -425,8 +425,8 @@ pub enum SignatureError {
425425
"Incorrect result of type application in Call - cached {cached} but expected {expected}"
426426
)]
427427
CallIncorrectlyAppliesType {
428-
cached: Signature,
429-
expected: Signature,
428+
cached: Box<Signature>,
429+
expected: Box<Signature>,
430430
},
431431
/// The result of the type application stored in a [`LoadFunction`]
432432
/// is not what we get by applying the type-args to the polymorphic function
@@ -436,8 +436,8 @@ pub enum SignatureError {
436436
"Incorrect result of type application in LoadFunction - cached {cached} but expected {expected}"
437437
)]
438438
LoadFunctionIncorrectlyAppliesType {
439-
cached: Signature,
440-
expected: Signature,
439+
cached: Box<Signature>,
440+
expected: Box<Signature>,
441441
},
442442

443443
/// Extension declaration specifies a binary compute signature function, but none
@@ -697,7 +697,7 @@ pub enum ExtensionRegistryError {
697697
#[error(
698698
"The registry already contains an extension with id {0} and version {1}. New extension has version {2}."
699699
)]
700-
AlreadyRegistered(ExtensionId, Version, Version),
700+
AlreadyRegistered(ExtensionId, Box<Version>, Box<Version>),
701701
/// A registered extension has invalid signatures.
702702
#[error("The extension {0} contains an invalid signature, {1}.")]
703703
InvalidSignature(ExtensionId, #[source] SignatureError),
@@ -712,7 +712,13 @@ pub enum ExtensionRegistryLoadError {
712712
SerdeError(#[from] serde_json::Error),
713713
/// Error when resolving internal extension references.
714714
#[error(transparent)]
715-
ExtensionResolutionError(#[from] ExtensionResolutionError),
715+
ExtensionResolutionError(Box<ExtensionResolutionError>),
716+
}
717+
718+
impl From<ExtensionResolutionError> for ExtensionRegistryLoadError {
719+
fn from(error: ExtensionResolutionError) -> Self {
720+
Self::ExtensionResolutionError(Box::new(error))
721+
}
716722
}
717723

718724
/// An error that can occur in building a new extension.
@@ -889,8 +895,8 @@ pub mod test {
889895
reg.register(ext1_1.clone()),
890896
Err(ExtensionRegistryError::AlreadyRegistered(
891897
ext_1_id.clone(),
892-
Version::new(1, 0, 0),
893-
Version::new(1, 1, 0)
898+
Box::new(Version::new(1, 0, 0)),
899+
Box::new(Version::new(1, 1, 0))
894900
))
895901
);
896902

hugr-core/src/extension/op_def.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -281,8 +281,8 @@ pub enum LowerFunc {
281281
/// [OpDef]
282282
///
283283
/// [ExtensionOp]: crate::ops::ExtensionOp
284-
#[serde_as(as = "AsStringEnvelope")]
285-
hugr: Hugr,
284+
#[serde_as(as = "Box<AsStringEnvelope>")]
285+
hugr: Box<Hugr>,
286286
},
287287
/// Custom binary function that can (fallibly) compute a Hugr
288288
/// for the particular instance and set of available extensions.
@@ -377,7 +377,7 @@ impl OpDef {
377377
.filter_map(|f| match f {
378378
LowerFunc::FixedHugr { extensions, hugr } => {
379379
if available_extensions.is_superset(extensions) {
380-
Some(hugr.clone())
380+
Some(hugr.as_ref().clone())
381381
} else {
382382
None
383383
}
@@ -664,7 +664,7 @@ pub(super) mod test {
664664
let def = ext.add_op(OP_NAME, "desc".into(), type_scheme, extension_ref)?;
665665
def.add_lower_func(LowerFunc::FixedHugr {
666666
extensions: ExtensionSet::new(),
667-
hugr: crate::builder::test::simple_dfg_hugr(), // this is nonsense, but we are not testing the actual lowering here
667+
hugr: Box::new(crate::builder::test::simple_dfg_hugr()), // this is nonsense, but we are not testing the actual lowering here
668668
});
669669
def.add_misc("key", Default::default());
670670
assert_eq!(def.description(), "desc");
@@ -754,8 +754,8 @@ pub(super) mod test {
754754
assert_eq!(
755755
def.validate_args(&args, &[TypeBound::Linear.into()]),
756756
Err(SignatureError::TypeVarDoesNotMatchDeclaration {
757-
actual: TypeBound::Linear.into(),
758-
cached: TypeBound::Copyable.into()
757+
actual: Box::new(TypeBound::Linear.into()),
758+
cached: Box::new(TypeBound::Copyable.into())
759759
})
760760
);
761761

@@ -807,8 +807,8 @@ pub(super) mod test {
807807
def.compute_signature(&[arg.clone()]),
808808
Err(SignatureError::TypeArgMismatch(
809809
TermTypeError::TypeMismatch {
810-
type_: TypeBound::Linear.into(),
811-
term: arg,
810+
type_: Box::new(TypeBound::Linear.into()),
811+
term: Box::new(arg),
812812
}
813813
))
814814
);
@@ -851,7 +851,7 @@ pub(super) mod test {
851851
any::<ExtensionSet>()
852852
.prop_map(|extensions| LowerFunc::FixedHugr {
853853
extensions,
854-
hugr: simple_dfg_hugr(),
854+
hugr: Box::new(simple_dfg_hugr()),
855855
})
856856
.boxed()
857857
}

hugr-core/src/extension/resolution/ops.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ pub(crate) fn resolve_op_extensions<'e>(
9898
node,
9999
extension: opaque.extension().clone(),
100100
op: def.name().clone(),
101-
computed: ext_op.signature().into_owned(),
102-
stored: opaque.signature().into_owned(),
101+
computed: Box::new(ext_op.signature().into_owned()),
102+
stored: Box::new(opaque.signature().into_owned()),
103103
}
104104
.into());
105105
}

hugr-core/src/extension/type_def.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,8 +272,8 @@ mod test {
272272
def.instantiate([qb_t().into()]),
273273
Err(SignatureError::TypeArgMismatch(
274274
TermTypeError::TypeMismatch {
275-
term: qb_t().into(),
276-
type_: TypeBound::Copyable.into()
275+
term: Box::new(qb_t().into()),
276+
type_: Box::new(TypeBound::Copyable.into())
277277
}
278278
))
279279
);

0 commit comments

Comments
 (0)