Skip to content

Commit

Permalink
feat: make errors more readable with Display impls
Browse files Browse the repository at this point in the history
Particularly useful for cli validation errors.

- Blanket Display for OpType that uses name, can be made more specific in future.
- user derive_more::Display in more places
  • Loading branch information
ss2165 committed Oct 18, 2024
1 parent 6097391 commit 79c58a0
Show file tree
Hide file tree
Showing 23 changed files with 108 additions and 79 deletions.
6 changes: 3 additions & 3 deletions hugr-core/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,10 @@ pub enum BuildError {
#[error("Constant failed typechecking: {0}")]
BadConstant(#[from] ConstTypeError),
/// CFG can only have one entry.
#[error("CFG entry node already built for CFG node: {0:?}.")]
#[error("CFG entry node already built for CFG node: {0}.")]
EntryBuiltError(Node),
/// Node was expected to have a certain type but was found to not.
#[error("Node with index {node:?} does not have type {op_desc:?} as expected.")]
#[error("Node with index {node} does not have type {op_desc} as expected.")]
#[allow(missing_docs)]
UnexpectedType {
/// Index of node where error occurred.
Expand All @@ -168,7 +168,7 @@ pub enum BuildError {
ConditionalError(#[from] conditional::ConditionalBuildError),

/// Wire not found in Hugr
#[error("Wire not found in Hugr: {0:?}.")]
#[error("Wire not found in Hugr: {0}.")]
WireNotFound(Wire),

/// Error in CircuitBuilder
Expand Down
6 changes: 3 additions & 3 deletions hugr-core/src/builder/conditional.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ pub type CaseBuilder<B> = DFGWrapper<B, BuildHandle<CaseID>>;
#[non_exhaustive]
pub enum ConditionalBuildError {
/// Case already built.
#[error("Case {case} of Conditional node {conditional:?} has already been built.")]
#[error("Case {case} of Conditional node {conditional} has already been built.")]
CaseBuilt { conditional: Node, case: usize },
/// Case already built.
#[error("Conditional node {conditional:?} has no case with index {case}.")]
#[error("Conditional node {conditional} has no case with index {case}.")]
NotCase { conditional: Node, case: usize },
/// Not all cases of Conditional built.
#[error("Cases {cases:?} of Conditional node {conditional:?} have not been built.")]
#[error("Cases {cases:?} of Conditional node {conditional} have not been built.")]
NotAllCasesBuilt {
conditional: Node,
cases: HashSet<usize>,
Expand Down
8 changes: 7 additions & 1 deletion hugr-core/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,12 @@ impl Wire {
}
}

impl std::fmt::Display for Wire {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "Wire({}, {})", self.0.index(), self.1.index)
}
}

/// Enum for uniquely identifying the origin of linear wires in a circuit-like
/// dataflow region.
///
Expand Down Expand Up @@ -323,4 +329,4 @@ macro_rules! impl_display_from_debug {
)*
};
}
impl_display_from_debug!(Node, Port, IncomingPort, OutgoingPort, Wire, CircuitUnit);
impl_display_from_debug!(Node, Port, IncomingPort, OutgoingPort, CircuitUnit);
4 changes: 2 additions & 2 deletions hugr-core/src/extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ pub enum SignatureError {
#[error("Definition name ({0}) and instantiation name ({1}) do not match.")]
NameMismatch(TypeName, TypeName),
/// Extension mismatch
#[error("Definition extension ({0:?}) and instantiation extension ({1:?}) do not match.")]
#[error("Definition extension ({0}) and instantiation extension ({1}) do not match.")]
ExtensionMismatch(ExtensionId, ExtensionId),
/// When the type arguments of the node did not match the params declared by the OpDef
#[error("Type arguments of node did not match params declared by definition: {0}")]
Expand All @@ -198,7 +198,7 @@ pub enum SignatureError {
expected: TypeBound,
},
/// A Type Variable's cache of its declared kind is incorrect
#[error("Type Variable claims to be {cached:?} but actual declaration {actual:?}")]
#[error("Type Variable claims to be {cached} but actual declaration {actual}")]
TypeVarDoesNotMatchDeclaration {
actual: TypeParam,
cached: TypeParam,
Expand Down
4 changes: 2 additions & 2 deletions hugr-core/src/extension/prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ impl ConstUsize {
#[typetag::serde]
impl CustomConst for ConstUsize {
fn name(&self) -> ValueName {
format!("ConstUsize({:?})", self.0).into()
format!("ConstUsize({})", self.0).into()
}

fn equal_consts(&self, other: &dyn CustomConst) -> bool {
Expand Down Expand Up @@ -394,7 +394,7 @@ impl ConstError {
#[typetag::serde]
impl CustomConst for ConstError {
fn name(&self) -> ValueName {
format!("ConstError({:?}, {:?})", self.signal, self.message).into()
format!("ConstError({}, {:?})", self.signal, self.message).into()
}

fn equal_consts(&self, other: &dyn CustomConst) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion hugr-core/src/hugr/rewrite/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub enum RemoveError {
#[error("Node is invalid (either not in HUGR or not correct operation).")]
InvalidNode(Node),
/// Node in use.
#[error("Node: {0:?} has non-zero outgoing connections.")]
#[error("Node: {0} has non-zero outgoing connections.")]
ValueUsed(Node),
}

Expand Down
2 changes: 1 addition & 1 deletion hugr-core/src/hugr/rewrite/insert_identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub enum IdentityInsertionError {
#[error("Node is invalid.")]
InvalidNode(),
/// Invalid port kind.
#[error("post_port has invalid kind {0:?}. Must be Value.")]
#[error("post_port has invalid kind {}. Must be Value.", _0.as_ref().map_or("None".to_string(), ToString::to_string))]
InvalidPortKind(Option<EdgeKind>),
}

Expand Down
8 changes: 4 additions & 4 deletions hugr-core/src/hugr/rewrite/outline_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,17 +223,17 @@ pub enum OutlineCfgError {
#[error("The nodes did not all have the same parent")]
NotSiblings,
/// The parent node was not a CFG node
#[error("The parent node {0:?} was not a CFG but a {1:?}")]
#[error("The parent node {0} was not a CFG but a {1}")]
ParentNotCfg(Node, OpType),
/// Multiple blocks had incoming edges
#[error("Multiple blocks had predecessors outside the set - at least {0:?} and {1:?}")]
#[error("Multiple blocks had predecessors outside the set - at least {0} and {1}")]
MultipleEntryNodes(Node, Node),
/// Multiple blocks had outgoing edges
// Note possible TODO: straightforward if all outgoing edges target the same BB
#[error("Multiple blocks had edges leaving the set - at least {0:?} and {1:?}")]
#[error("Multiple blocks had edges leaving the set - at least {0} and {1}")]
MultipleExitNodes(Node, Node),
/// One block had multiple outgoing edges
#[error("Exit block {0:?} had edges to multiple external blocks {1:?}")]
#[error("Exit block {0} had edges to multiple external blocks {1:?}")]
MultipleExitEdges(Node, Vec<Node>),
/// No block was identified as an entry block
#[error("No block had predecessors outside the set")]
Expand Down
4 changes: 2 additions & 2 deletions hugr-core/src/hugr/rewrite/replace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ pub enum ReplaceError {
replacement: OpTag,
},
/// Keys in [Replacement::adoptions] were not valid container nodes in [Replacement::replacement]
#[error("Node {0:?} was not an empty container node in the replacement")]
#[error("Node {0} was not an empty container node in the replacement")]
InvalidAdoptingParent(Node),
/// Some values in [Replacement::adoptions] were either descendants of other values, or not
/// descendants of the [Replacement::removal]. The nodes are indicated on a best-effort basis.
Expand Down Expand Up @@ -599,7 +599,7 @@ mod test {

fn find_node(h: &Hugr, s: &str) -> crate::Node {
h.nodes()
.filter(|n| format!("{:?}", h.get_optype(*n)).contains(s))
.filter(|n| format!("{}", h.get_optype(*n)).contains(s))
.exactly_one()
.ok()
.unwrap()
Expand Down
10 changes: 5 additions & 5 deletions hugr-core/src/hugr/serialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,13 @@ struct SerHugrLatest {
#[non_exhaustive]
pub enum HUGRSerializationError {
/// Unexpected hierarchy error.
#[error("Failed to attach child to parent: {0:?}.")]
#[error("Failed to attach child to parent: {0}.")]
AttachError(#[from] AttachError),
/// Failed to add edge.
#[error("Failed to build edge when deserializing: {0:?}.")]
#[error("Failed to build edge when deserializing: {0}.")]
LinkError(#[from] LinkError),
/// Edges without port offsets cannot be present in operations without non-dataflow ports.
#[error("Cannot connect an {dir:?} edge without port offset to node {node:?} with operation type {op_type:?}.")]
#[error("Cannot connect an {dir:?} edge without port offset to node {node} with operation type {op_type}.")]
MissingPortOffset {
/// The node that has the port without offset.
node: Node,
Expand All @@ -119,13 +119,13 @@ pub enum HUGRSerializationError {
op_type: OpType,
},
/// Edges with wrong node indices
#[error("The edge endpoint {node:?} is not a node in the graph.")]
#[error("The edge endpoint {node} is not a node in the graph.")]
UnknownEdgeNode {
/// The node that has the port without offset.
node: Node,
},
/// First node in node list must be the HUGR root.
#[error("The first node in the node list has parent {0:?}, should be itself (index 0)")]
#[error("The first node in the node list has parent {0}, should be itself (index 0)")]
FirstNodeNotRoot(Node),
}

Expand Down
48 changes: 25 additions & 23 deletions hugr-core/src/hugr/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl Hugr {
OpTag::ModuleRoot => continue,
_ => {
assert!(self.children(parent).next().is_none(),
"Unknown parent node type {:?} - not a DataflowParent, Module, Cfg or Conditional",
"Unknown parent node type {} - not a DataflowParent, Module, Cfg or Conditional",
parent_op);
continue;
}
Expand Down Expand Up @@ -635,13 +635,13 @@ impl<'a, 'b> ValidationContext<'a, 'b> {
#[non_exhaustive]
pub enum ValidationError {
/// The root node of the Hugr is not a root in the hierarchy.
#[error("The root node of the Hugr {node:?} is not a root in the hierarchy.")]
#[error("The root node of the Hugr {node} is not a root in the hierarchy.")]
RootNotRoot { node: Node },
/// The root node of the Hugr should not have any edges.
#[error("The root node of the Hugr {node:?} has edges when it should not.")]
#[error("The root node of the Hugr {node} has edges when it should not.")]
RootWithEdges { node: Node },
/// The node ports do not match the operation signature.
#[error("The node {node:?} has an invalid number of ports. The operation {optype:?} cannot have {actual:?} {dir:?} ports. Expected {expected:?}.")]
#[error("The node {node} has an invalid number of ports. The operation {optype} cannot have {actual} {dir:?} ports. Expected {expected}.")]
WrongNumberOfPorts {
node: Node,
optype: OpType,
Expand All @@ -650,21 +650,23 @@ pub enum ValidationError {
dir: Direction,
},
/// A dataflow port is not connected.
#[error("The node {node:?} has an unconnected port {port:?} of type {port_kind:?}.")]
#[error("The node {node} has an unconnected port {port} of type {port_kind}.")]
UnconnectedPort {
node: Node,
port: Port,
port_kind: EdgeKind,
},
/// A linear port is connected to more than one thing.
#[error("The node {node:?} has a port {port:?} of type {port_kind:?} with more than one connection.")]
#[error(
"The node {node} has a port {port} of type {port_kind} with more than one connection."
)]
TooManyConnections {
node: Node,
port: Port,
port_kind: EdgeKind,
},
/// Connected ports have different types, or non-unifiable types.
#[error("Connected ports {from_port:?} in node {from:?} and {to_port:?} in node {to:?} have incompatible kinds. Cannot connect {from_kind:?} to {to_kind:?}.")]
#[error("Connected ports {from_port} in node {from} and {to_port} in node {to} have incompatible kinds. Cannot connect {from_kind} to {to_kind}.")]
IncompatiblePorts {
from: Node,
from_port: Port,
Expand All @@ -674,10 +676,10 @@ pub enum ValidationError {
to_kind: EdgeKind,
},
/// The non-root node has no parent.
#[error("The node {node:?} has no parent.")]
#[error("The node {node} has no parent.")]
NoParent { node: Node },
/// The parent node is not compatible with the child node.
#[error("The operation {parent_optype:?} cannot contain a {child_optype:?} as a child. Allowed children: {}. In node {child:?} with parent {parent:?}.", allowed_children.description())]
#[error("The operation {parent_optype} cannot contain a {child_optype} as a child. Allowed children: {}. In node {child} with parent {parent}.", allowed_children.description())]
InvalidParentOp {
child: Node,
child_optype: OpType,
Expand All @@ -686,7 +688,7 @@ pub enum ValidationError {
allowed_children: OpTag,
},
/// Invalid first/second child.
#[error("A {optype:?} operation cannot be the {position} child of a {parent_optype:?}. Expected {expected}. In parent node {parent:?}")]
#[error("A {optype} operation cannot be the {position} child of a {parent_optype}. Expected {expected}. In parent node {parent}")]
InvalidInitialChild {
parent: Node,
parent_optype: OpType,
Expand All @@ -696,8 +698,8 @@ pub enum ValidationError {
},
/// The children list has invalid elements.
#[error(
"An operation {parent_optype:?} contains invalid children: {source}. In parent {parent:?}, child {child:?}",
child=source.child(),
"An operation {parent_optype} contains invalid children: {source}. In parent {parent}, child Node({child})",
child=source.child().index(),
)]
InvalidChildren {
parent: Node,
Expand All @@ -706,7 +708,7 @@ pub enum ValidationError {
},
/// The children graph has invalid edges.
#[error(
"An operation {parent_optype:?} contains invalid edges between its children: {source}. In parent {parent:?}, edge from {from:?} port {from_port:?} to {to:?} port {to_port:?}",
"An operation {parent_optype} contains invalid edges between its children: {source}. In parent {parent}, edge from {from:?} port {from_port:?} to {to:?} port {to_port:?}",
from=source.edge().source,
from_port=source.edge().source_port,
to=source.edge().target,
Expand All @@ -718,13 +720,13 @@ pub enum ValidationError {
source: EdgeValidationError,
},
/// The node operation is not a container, but has children.
#[error("The node {node:?} with optype {optype:?} is not a container, but has children.")]
#[error("The node {node} with optype {optype} is not a container, but has children.")]
NonContainerWithChildren { node: Node, optype: OpType },
/// The node must have children, but has none.
#[error("The node {node:?} with optype {optype:?} must have children, but has none.")]
#[error("The node {node} with optype {optype} must have children, but has none.")]
ContainerWithoutChildren { node: Node, optype: OpType },
/// The children of a node do not form a DAG.
#[error("The children of an operation {optype:?} must form a DAG. Loops are not allowed. In node {node:?}.")]
#[error("The children of an operation {optype} must form a DAG. Loops are not allowed. In node {node}.")]
NotADag { node: Node, optype: OpType },
/// There are invalid inter-graph edges.
#[error(transparent)]
Expand All @@ -733,7 +735,7 @@ pub enum ValidationError {
#[error(transparent)]
ExtensionError(#[from] ExtensionError),
/// A node claims to still be awaiting extension inference. Perhaps it is not acted upon by inference.
#[error("Node {node:?} needs a concrete ExtensionSet - inference will provide this for Case/CFG/Conditional/DataflowBlock/DFG/TailLoop only")]
#[error("Node {node} needs a concrete ExtensionSet - inference will provide this for Case/CFG/Conditional/DataflowBlock/DFG/TailLoop only")]
ExtensionsNotInferred { node: Node },
/// Error in a node signature
#[error("Error in signature of node {node}: {cause}")]
Expand Down Expand Up @@ -763,7 +765,7 @@ pub enum ValidationError {
#[non_exhaustive]
pub enum InterGraphEdgeError {
/// Inter-Graph edges can only carry copyable data.
#[error("Inter-graph edges can only carry copyable data. In an inter-graph edge from {from:?} ({from_offset:?}) to {to:?} ({to_offset:?}) with type {ty:?}.")]
#[error("Inter-graph edges can only carry copyable data. In an inter-graph edge from {from} ({from_offset}) to {to} ({to_offset}) with type {ty}.")]
NonCopyableData {
from: Node,
from_offset: Port,
Expand All @@ -772,7 +774,7 @@ pub enum InterGraphEdgeError {
ty: EdgeKind,
},
/// Inter-Graph edges may not enter into FuncDefns unless they are static
#[error("Inter-graph Value edges cannot enter into FuncDefns. Inter-graph edge from {from:?} ({from_offset:?}) to {to:?} ({to_offset:?} enters FuncDefn {func:?}")]
#[error("Inter-graph Value edges cannot enter into FuncDefns. Inter-graph edge from {from} ({from_offset}) to {to} ({to_offset} enters FuncDefn {func}")]
ValueEdgeIntoFunc {
from: Node,
from_offset: Port,
Expand All @@ -781,7 +783,7 @@ pub enum InterGraphEdgeError {
func: Node,
},
/// The grandparent of a dominator inter-graph edge must be a CFG container.
#[error("The grandparent of a dominator inter-graph edge must be a CFG container. Found operation {ancestor_parent_op:?}. In a dominator inter-graph edge from {from:?} ({from_offset:?}) to {to:?} ({to_offset:?}).")]
#[error("The grandparent of a dominator inter-graph edge must be a CFG container. Found operation {ancestor_parent_op}. In a dominator inter-graph edge from {from} ({from_offset}) to {to} ({to_offset}).")]
NonCFGAncestor {
from: Node,
from_offset: Port,
Expand All @@ -790,7 +792,7 @@ pub enum InterGraphEdgeError {
ancestor_parent_op: OpType,
},
/// The sibling ancestors of the external inter-graph edge endpoints must be have an order edge between them.
#[error("Missing state order between the external inter-graph source {from:?} and the ancestor of the target {to_ancestor:?}. In an external inter-graph edge from {from:?} ({from_offset:?}) to {to:?} ({to_offset:?}).")]
#[error("Missing state order between the external inter-graph source {from} and the ancestor of the target {to_ancestor}. In an external inter-graph edge from {from} ({from_offset}) to {to} ({to_offset}).")]
MissingOrderEdge {
from: Node,
from_offset: Port,
Expand All @@ -799,15 +801,15 @@ pub enum InterGraphEdgeError {
to_ancestor: Node,
},
/// The ancestors of an inter-graph edge are not related.
#[error("The ancestors of an inter-graph edge are not related. In an inter-graph edge from {from:?} ({from_offset:?}) to {to:?} ({to_offset:?}).")]
#[error("The ancestors of an inter-graph edge are not related. In an inter-graph edge from {from} ({from_offset}) to {to} ({to_offset}).")]
NoRelation {
from: Node,
from_offset: Port,
to: Node,
to_offset: Port,
},
/// The basic block containing the source node does not dominate the basic block containing the target node.
#[error(" The basic block containing the source node does not dominate the basic block containing the target node in the CFG. Expected node {from_parent:?} to dominate {ancestor:?}. In a dominator inter-graph edge from {from:?} ({from_offset:?}) to {to:?} ({to_offset:?}).")]
#[error(" The basic block containing the source node does not dominate the basic block containing the target node in the CFG. Expected node {from_parent} to dominate {ancestor}. In a dominator inter-graph edge from {from} ({from_offset}) to {to} ({to_offset}).")]
NonDominatedAncestor {
from: Node,
from_offset: Port,
Expand Down
8 changes: 7 additions & 1 deletion hugr-core/src/hugr/validate/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,13 @@ fn invalid_types() {
let (h, def) = identity_hugr_with_type(Type::new_extension(t));
match h.validate(&reg) {
Err(ValidationError::SignatureError { node, cause }) if node == def => cause,
e => panic!("Expected SignatureError at def node, got {:?}", e),
e => panic!(
"Expected SignatureError at def node, got {}",
match e {
Ok(()) => "Ok".to_owned(),
Err(e) => format!("{}", e),
}
),
}
};

Expand Down
Loading

0 comments on commit 79c58a0

Please sign in to comment.