Skip to content

Commit

Permalink
feat: Validate TypeArgs to ExtensionOp (#509)
Browse files Browse the repository at this point in the history
The TypeArgs might contain malformed things like "the type List<5>" - we
can only check that they make sense (i.e. that `5` is valid as an
argument for List) *given the ExtensionRegistry*, and nowhere outside
validation has the ExtensionRegistry, so we need to do this here.

Also rename `validate_children` (in ValidateOp) to
`validate_op_children)`, and `validate_operation` to
`validate_children`.
  • Loading branch information
acl-cqc authored Sep 12, 2023
1 parent 17b4eec commit 7806540
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 14 deletions.
18 changes: 14 additions & 4 deletions src/hugr/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use crate::extension::{
validate::{ExtensionError, ExtensionValidator},
ExtensionRegistry, ExtensionSolution, InferExtensionError,
};

use crate::ops::validate::{ChildrenEdgeData, ChildrenValidationError, EdgeValidationError};
use crate::ops::{OpTag, OpTrait, OpType, ValidateOp};
use crate::types::{EdgeKind, Type};
Expand Down Expand Up @@ -158,8 +159,17 @@ impl<'a, 'b> ValidationContext<'a, 'b> {
}
}

// Check operation-specific constraints
self.validate_operation(node, node_type)?;
// Check operation-specific constraints. Firstly that type args are correct
// (Good to call `resolve_extension_ops` immediately before this
// - see https://github.com/CQCL-DEV/hugr/issues/508 )
if let OpType::LeafOp(crate::ops::LeafOp::CustomOp(b)) = op_type {
for arg in b.args() {
arg.validate(self.extension_registry)
.map_err(|cause| ValidationError::SignatureError { node, cause })?;
}
}
// Secondly that the node has correct children
self.validate_children(node, node_type)?;

// If this is a container with I/O nodes, check that the extension they
// define match the extensions of the container.
Expand Down Expand Up @@ -260,7 +270,7 @@ impl<'a, 'b> ValidationContext<'a, 'b> {
/// Check operation-specific constraints.
///
/// These are flags defined for each operation type as an [`OpValidityFlags`] object.
fn validate_operation(&self, node: Node, node_type: &NodeType) -> Result<(), ValidationError> {
fn validate_children(&self, node: Node, node_type: &NodeType) -> Result<(), ValidationError> {
let op_type = &node_type.op;
let flags = op_type.validity_flags();

Expand Down Expand Up @@ -301,7 +311,7 @@ impl<'a, 'b> ValidationContext<'a, 'b> {
}
// Additional validations running over the full list of children optypes
let children_optypes = all_children.map(|c| (c.index, self.hugr.get_optype(c)));
if let Err(source) = op_type.validate_children(children_optypes) {
if let Err(source) = op_type.validate_op_children(children_optypes) {
return Err(ValidationError::InvalidChildren {
parent: node,
parent_optype: op_type.clone(),
Expand Down
2 changes: 1 addition & 1 deletion src/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ pub trait ValidateOp {

/// Validate the ordered list of children.
#[inline]
fn validate_children<'a>(
fn validate_op_children<'a>(
&self,
_children: impl DoubleEndedIterator<Item = (NodeIndex, &'a OpType)>,
) -> Result<(), validate::ChildrenValidationError> {
Expand Down
16 changes: 8 additions & 8 deletions src/ops/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! Adds a `validity_flags` method to [`OpType`] that returns a series of flags
//! used by the [`crate::hugr::validate`] module.
//!
//! It also defines a `validate_children` method for more complex tests that
//! It also defines a `validate_op_children` method for more complex tests that
//! require traversing the children.

use itertools::Itertools;
Expand Down Expand Up @@ -93,7 +93,7 @@ impl ValidateOp for super::FuncDefn {
}
}

fn validate_children<'a>(
fn validate_op_children<'a>(
&self,
children: impl DoubleEndedIterator<Item = (NodeIndex, &'a OpType)>,
) -> Result<(), ChildrenValidationError> {
Expand All @@ -118,7 +118,7 @@ impl ValidateOp for super::DFG {
}
}

fn validate_children<'a>(
fn validate_op_children<'a>(
&self,
children: impl DoubleEndedIterator<Item = (NodeIndex, &'a OpType)>,
) -> Result<(), ChildrenValidationError> {
Expand All @@ -141,7 +141,7 @@ impl ValidateOp for super::Conditional {
}
}

fn validate_children<'a>(
fn validate_op_children<'a>(
&self,
children: impl DoubleEndedIterator<Item = (NodeIndex, &'a OpType)>,
) -> Result<(), ChildrenValidationError> {
Expand Down Expand Up @@ -188,7 +188,7 @@ impl ValidateOp for super::TailLoop {
}
}

fn validate_children<'a>(
fn validate_op_children<'a>(
&self,
children: impl DoubleEndedIterator<Item = (NodeIndex, &'a OpType)>,
) -> Result<(), ChildrenValidationError> {
Expand All @@ -214,7 +214,7 @@ impl ValidateOp for super::CFG {
}
}

fn validate_children<'a>(
fn validate_op_children<'a>(
&self,
children: impl Iterator<Item = (NodeIndex, &'a OpType)>,
) -> Result<(), ChildrenValidationError> {
Expand Down Expand Up @@ -334,7 +334,7 @@ impl ValidateOp for BasicBlock {
}

/// Validate the ordered list of children.
fn validate_children<'a>(
fn validate_op_children<'a>(
&self,
children: impl DoubleEndedIterator<Item = (NodeIndex, &'a OpType)>,
) -> Result<(), ChildrenValidationError> {
Expand Down Expand Up @@ -369,7 +369,7 @@ impl ValidateOp for super::Case {
}

/// Validate the ordered list of children.
fn validate_children<'a>(
fn validate_op_children<'a>(
&self,
children: impl DoubleEndedIterator<Item = (NodeIndex, &'a OpType)>,
) -> Result<(), ChildrenValidationError> {
Expand Down
2 changes: 1 addition & 1 deletion src/types/type_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ pub enum TypeArg {
}

impl TypeArg {
pub(super) fn validate(
pub(crate) fn validate(
&self,
extension_registry: &ExtensionRegistry,
) -> Result<(), SignatureError> {
Expand Down

0 comments on commit 7806540

Please sign in to comment.