Skip to content

Commit

Permalink
feat!: Validate Extensions using hierarchy, ignore input_extensions, …
Browse files Browse the repository at this point in the history
…RIP inference (#1142)

This is probably phase 1 of 4, later steps being
* Remove `input_extensions` and `NodeType`
* Make every operation require it's own extension (#388)
* Infer deltas for parent nodes (#640) - this may be disruptive, and/or
take some subtlety, as currently every FunctionType stores an
ExtensionSet not an Option thereof
* Finally, remove the feature flag

So, this PR updates validation to ignore the input_extensions, and
removes the inference algorithm that would set them. There were a few
complications:
* A lot of tests create a DFGBuilder with an *empty* delta, then put
things in it that have extension requirements. The inference algorithm
figures out that this is all OK if it can just put that
extension-requirement on the `input_extensions` to the DFG node (root)
itself. So this might be a call for `input_extensions` - they do reduce
the need for #640 a bit.
* We can see roughly how painful life is (without delta-inference nor
input-extensions) looking at the various extra extension-deltas I've had
to specify here
* I think that given we are under the feature flag at the moment, we are
OK to continue, however the need for #640 is now somewhat increased, so
discussion there strongly encouraged, please! :)
* `TailLoop` turned out not to have an extension-delta, where it clearly
needs one (just as DataflowBlock and others). Also there was an
implementation of `OpParent` for it, whereas in fact we needed
`DataflowParent` as that would also have got us the `ValidateOp` for
free. This all in 08bb5a5.
* Another bug in DataflowBlock::inner_signature, and some others.

....That is to say, in some ways this validation scheme is *stricter*
than what we had; there is both the slight flexibility that
`input_extensions` gave us in mis-specifying deltas, and that this
scheme's simplicity makes it much harder to accidentally omit cases from
validation....

I've also kept `Hugr::infer_extensions` around as a no-op rather than
remove it and later bring it back (in #640). Maybe we should remove it,
but that would be a breaking change....OTOH I've removed
`validate_with_extension_closure` and in theory we could have that (a
closure containing deltas for nested DFGs) too...

BREAKING CHANGE: TailLoop node and associated builder functions now
require specifying an ExtensionSet; extension/validate.rs deleted; some
changes to Hugrs validated/rejected when the `extension_inference`
feature flag is turned on
  • Loading branch information
acl-cqc authored Jun 11, 2024
1 parent d41055a commit 8bec8e9
Show file tree
Hide file tree
Showing 23 changed files with 514 additions and 2,587 deletions.
2 changes: 2 additions & 0 deletions hugr-core/src/builder/build_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ pub trait Dataflow: Container {
just_inputs: impl IntoIterator<Item = (Type, Wire)>,
inputs_outputs: impl IntoIterator<Item = (Type, Wire)>,
just_out_types: TypeRow,
extension_delta: ExtensionSet,
) -> Result<TailLoopBuilder<&mut Hugr>, BuildError> {
let (input_types, mut input_wires): (Vec<Type>, Vec<Wire>) =
just_inputs.into_iter().unzip();
Expand All @@ -440,6 +441,7 @@ pub trait Dataflow: Container {
just_inputs: input_types.into(),
just_outputs: just_out_types,
rest: rest_types.into(),
extension_delta,
};
// TODO: Make input extensions a parameter
let (loop_node, _) = add_node_with_wires(self, tail_loop.clone(), input_wires)?;
Expand Down
17 changes: 12 additions & 5 deletions hugr-core/src/builder/tail_loop.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::extension::ExtensionSet;
use crate::ops;

use crate::hugr::{views::HugrView, NodeType};
Expand Down Expand Up @@ -74,11 +75,13 @@ impl TailLoopBuilder<Hugr> {
just_inputs: impl Into<TypeRow>,
inputs_outputs: impl Into<TypeRow>,
just_outputs: impl Into<TypeRow>,
extension_delta: ExtensionSet,
) -> Result<Self, BuildError> {
let tail_loop = ops::TailLoop {
just_inputs: just_inputs.into(),
just_outputs: just_outputs.into(),
rest: inputs_outputs.into(),
extension_delta,
};
// TODO: Allow input extensions to be specified
let base = Hugr::new(NodeType::new_open(tail_loop.clone()));
Expand All @@ -97,7 +100,6 @@ mod test {
DataflowSubContainer, HugrBuilder, ModuleBuilder,
},
extension::prelude::{ConstUsize, PRELUDE_ID, USIZE_T},
extension::ExtensionSet,
hugr::ValidationError,
ops::Value,
type_row,
Expand All @@ -107,7 +109,8 @@ mod test {
#[test]
fn basic_loop() -> Result<(), BuildError> {
let build_result: Result<Hugr, ValidationError> = {
let mut loop_b = TailLoopBuilder::new(vec![], vec![BIT], vec![USIZE_T])?;
let mut loop_b =
TailLoopBuilder::new(vec![], vec![BIT], vec![USIZE_T], PRELUDE_ID.into())?;
let [i1] = loop_b.input_wires_arr();
let const_wire = loop_b.add_load_value(ConstUsize::new(1));

Expand Down Expand Up @@ -141,8 +144,12 @@ mod test {
)?
.outputs_arr();
let loop_id = {
let mut loop_b =
fbuild.tail_loop_builder(vec![(BIT, b1)], vec![], type_row![NAT])?;
let mut loop_b = fbuild.tail_loop_builder(
vec![(BIT, b1)],
vec![],
type_row![NAT],
PRELUDE_ID.into(),
)?;
let signature = loop_b.loop_signature()?.clone();
let const_val = Value::true_val();
let const_wire = loop_b.add_load_const(Value::true_val());
Expand All @@ -161,7 +168,7 @@ mod test {
([type_row![], type_row![]], const_wire),
vec![(BIT, b1)],
output_row,
ExtensionSet::new(),
PRELUDE_ID.into(),
)?;

let mut branch_0 = conditional_b.case_builder(0)?;
Expand Down
7 changes: 0 additions & 7 deletions hugr-core/src/extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,6 @@ use crate::types::type_param::{TypeArg, TypeArgError, TypeParam};
use crate::types::{check_typevar_decl, CustomType, Substitution, TypeBound, TypeName};
use crate::types::{FunctionType, TypeNameRef};

#[allow(dead_code)]
mod infer;
#[cfg(feature = "extension_inference")]
pub use infer::infer_extensions;
pub use infer::{ExtensionSolution, InferExtensionError};

mod op_def;
pub use op_def::{
CustomSignatureFunc, CustomValidator, LowerFunc, OpDef, SignatureFromArgs, SignatureFunc,
Expand All @@ -35,7 +29,6 @@ pub use type_def::{TypeDef, TypeDefBound};
mod const_fold;
pub mod prelude;
pub mod simple_op;
pub mod validate;
pub use const_fold::{ConstFold, ConstFoldResult, Folder};
pub use prelude::{PRELUDE, PRELUDE_REGISTRY};

Expand Down
Loading

0 comments on commit 8bec8e9

Please sign in to comment.