From 78065400eed9c413e65bb453e16470cced825f70 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 12 Sep 2023 11:05:15 +0100 Subject: [PATCH] feat: Validate TypeArgs to ExtensionOp (#509) 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`. --- src/hugr/validate.rs | 18 ++++++++++++++---- src/ops.rs | 2 +- src/ops/validate.rs | 16 ++++++++-------- src/types/type_param.rs | 2 +- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/hugr/validate.rs b/src/hugr/validate.rs index a687eb308..9db0a4f7b 100644 --- a/src/hugr/validate.rs +++ b/src/hugr/validate.rs @@ -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}; @@ -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. @@ -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(); @@ -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(), diff --git a/src/ops.rs b/src/ops.rs index f054de053..cbd928fe7 100644 --- a/src/ops.rs +++ b/src/ops.rs @@ -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, ) -> Result<(), validate::ChildrenValidationError> { diff --git a/src/ops/validate.rs b/src/ops/validate.rs index 76c6cfd45..a5094e6d0 100644 --- a/src/ops/validate.rs +++ b/src/ops/validate.rs @@ -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; @@ -93,7 +93,7 @@ impl ValidateOp for super::FuncDefn { } } - fn validate_children<'a>( + fn validate_op_children<'a>( &self, children: impl DoubleEndedIterator, ) -> Result<(), ChildrenValidationError> { @@ -118,7 +118,7 @@ impl ValidateOp for super::DFG { } } - fn validate_children<'a>( + fn validate_op_children<'a>( &self, children: impl DoubleEndedIterator, ) -> Result<(), ChildrenValidationError> { @@ -141,7 +141,7 @@ impl ValidateOp for super::Conditional { } } - fn validate_children<'a>( + fn validate_op_children<'a>( &self, children: impl DoubleEndedIterator, ) -> Result<(), ChildrenValidationError> { @@ -188,7 +188,7 @@ impl ValidateOp for super::TailLoop { } } - fn validate_children<'a>( + fn validate_op_children<'a>( &self, children: impl DoubleEndedIterator, ) -> Result<(), ChildrenValidationError> { @@ -214,7 +214,7 @@ impl ValidateOp for super::CFG { } } - fn validate_children<'a>( + fn validate_op_children<'a>( &self, children: impl Iterator, ) -> Result<(), ChildrenValidationError> { @@ -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, ) -> Result<(), ChildrenValidationError> { @@ -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, ) -> Result<(), ChildrenValidationError> { diff --git a/src/types/type_param.rs b/src/types/type_param.rs index f20ab3eaf..468bb40d6 100644 --- a/src/types/type_param.rs +++ b/src/types/type_param.rs @@ -97,7 +97,7 @@ pub enum TypeArg { } impl TypeArg { - pub(super) fn validate( + pub(crate) fn validate( &self, extension_registry: &ExtensionRegistry, ) -> Result<(), SignatureError> {