Skip to content

Commit

Permalink
refactor!: Separate Signature from FuncValueType by parametrizing Typ…
Browse files Browse the repository at this point in the history
…e(/Row)/etc. (#1138)

* Type now means a single type, not a row variable; TypeRV can be a
single type *or* a row variable. (Type --Into--> TypeRV).
* These are actually parameterizations: `TypeBase<NoRV>` (NoRV is a type
with no instances) vs `TypeBase<RowVariable>` (RowVariable is a struct
i.e. index + bound)
* Similarly for the other type structures - all names have changed
uniformly except FunctionType:

| No Row Vars | With Row Vars | Generic |
| ----- | ----- | ----- |
| Type | TypeRV | `TypeBase<RV>` |
| TypeRow | TypeRowRV | `TypeRowBase<RV>` |
| Signature | FuncValueType | `FuncTypeBase<RV>` |
| PolyFuncType | PolyFuncTypeRV | `PolyFuncTypeBase<RV>` |

* This allows us to make static guarantees (and remove dynamic checks):
    * node signatures do not have row variables as inputs or outputs
* Neither do FuncDefn type schemes, even though they may quantify over
row variables
* "port"-indexing methods are specific to Signature (not FuncValueType)
- this justifies the exceptional naming

Planning to rename PolyFuncType to (Op/Fn)TypeScheme in a followup PR
(lots of fields want renaming too, whereas many FunctionType things are
*already* called `signature`!)

**One question** is that currently `Type` and `TypeRV` are aliases for
instantiations of `TypeBase`. An alternative is to parameterize
`Type<RV: MaybeRV = NoRV>` and define as alias only `type TypeRV =
Type<RowVariable>` (no `TypeBase`). A type annotation e.g. `x: Type`
then uses the default, but a call like `Type::new_function` then tries
to infer a unique possible parameter. This would slightly worsen
Hugr-building code (some code like `let x = Type::new_function` have to
become either `let x = Type::<NoRV>::new_function` or `let x: Type =
Type::new_function` - sadly the *default* for RV is ignored for
members/namespacing but not when forming a type, see
rust-lang/rust#98931). OTOH some OpDef code
would be a bit easier (needs fewer explicit `RV` suffices). In general I
think we favour making Hugr-building code as easy as possible over OpDef
code, but there is always the question of, *how much* easier etc...see
commit bc565ba which removed defaults
for Type (with an earlier parametrization by bool true/false rather than
by RowVariable/NoRV). (And actually that commit understates the
simplification - there were a bunch more `let x:Type`s required for the
default method that can be removed for the always-explicit method, I
took these out in 23db41d, so maybe we
should stick with always-explicit.)

* [ ] TODO python classes need names updating to match, etc. 

BREAKING CHANGE: (1) In OpDefs, Type/TypeRow/FunctionType/PolyFuncType
will generally need replacing by
TypeRV/TypeRowRV/FuncValueType/PolyFuncTypeRV. (2) FuncValueType omits
the various numbered-"port" accessors, you really shouldn't be doing
that except on a Signature.... (4)
SignatureError::RowVarWhereTypeExpected now contains a RowVariable
struct (including bound) rather than only the index
  • Loading branch information
acl-cqc authored Jul 17, 2024
1 parent 6f8dac2 commit 79029b8
Show file tree
Hide file tree
Showing 61 changed files with 1,368 additions and 1,033 deletions.
6 changes: 3 additions & 3 deletions hugr-cli/tests/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use hugr_core::{
builder::{Container, Dataflow, DataflowHugr},
extension::prelude::{BOOL_T, QB_T},
type_row,
types::FunctionType,
types::Signature,
Hugr,
};
use predicates::{prelude::*, str::contains};
Expand All @@ -25,7 +25,7 @@ fn cmd() -> Command {

#[fixture]
fn test_hugr() -> Hugr {
let df = DFGBuilder::new(FunctionType::new_endo(type_row![BOOL_T])).unwrap();
let df = DFGBuilder::new(Signature::new_endo(type_row![BOOL_T])).unwrap();
let [i] = df.input_wires_arr();
df.finish_prelude_hugr_with_outputs([i]).unwrap()
}
Expand Down Expand Up @@ -83,7 +83,7 @@ fn test_mermaid(test_hugr_file: NamedTempFile, mut cmd: Command) {

#[rstest]
fn test_bad_hugr(mut cmd: Command) {
let df = DFGBuilder::new(FunctionType::new_endo(type_row![QB_T])).unwrap();
let df = DFGBuilder::new(Signature::new_endo(type_row![QB_T])).unwrap();
let bad_hugr = df.hugr().clone();

let bad_hugr_string = serde_json::to_string(&bad_hugr).unwrap();
Expand Down
25 changes: 12 additions & 13 deletions hugr-core/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
//! # use hugr::builder::{BuildError, BuildHandle, Container, DFGBuilder, Dataflow, DataflowHugr, ModuleBuilder, DataflowSubContainer, HugrBuilder};
//! use hugr::extension::prelude::BOOL_T;
//! use hugr::std_extensions::logic::{EXTENSION_ID, LOGIC_REG, NotOp};
//! use hugr::types::FunctionType;
//! use hugr::types::Signature;
//!
//! # fn doctest() -> Result<(), BuildError> {
//! let hugr = {
Expand All @@ -42,7 +42,7 @@
//! let _dfg_handle = {
//! let mut dfg = module_builder.define_function(
//! "main",
//! FunctionType::new_endo(BOOL_T).with_extension_delta(EXTENSION_ID),
//! Signature::new_endo(BOOL_T).with_extension_delta(EXTENSION_ID),
//! )?;
//!
//! // Get the wires from the function inputs.
Expand All @@ -59,7 +59,7 @@
//! let _circuit_handle = {
//! let mut dfg = module_builder.define_function(
//! "circuit",
//! FunctionType::new_endo(vec![BOOL_T, BOOL_T])
//! Signature::new_endo(vec![BOOL_T, BOOL_T])
//! .with_extension_delta(EXTENSION_ID),
//! )?;
//! let mut circuit = dfg.as_circuit(dfg.input_wires());
Expand Down Expand Up @@ -93,7 +93,7 @@ use crate::hugr::ValidationError;
use crate::ops::handle::{BasicBlockID, CfgID, ConditionalID, DfgID, FuncID, TailLoopID};
use crate::ops::{NamedOp, OpType};
use crate::types::Type;
use crate::types::{ConstTypeError, FunctionType, TypeRow};
use crate::types::{ConstTypeError, Signature, TypeRow};
use crate::{Node, Port, Wire};

pub mod handle;
Expand Down Expand Up @@ -124,14 +124,14 @@ pub use circuit::{CircuitBuildError, CircuitBuilder};

/// Return a FunctionType with the same input and output types (specified)
/// whose extension delta, when used in a non-FuncDefn container, will be inferred.
pub fn endo_ft(types: impl Into<TypeRow>) -> FunctionType {
FunctionType::new_endo(types).with_extension_delta(TO_BE_INFERRED)
pub fn endo_sig(types: impl Into<TypeRow>) -> Signature {
Signature::new_endo(types).with_extension_delta(TO_BE_INFERRED)
}

/// Return a FunctionType with the specified input and output types
/// whose extension delta, when used in a non-FuncDefn container, will be inferred.
pub fn inout_ft(inputs: impl Into<TypeRow>, outputs: impl Into<TypeRow>) -> FunctionType {
FunctionType::new(inputs, outputs).with_extension_delta(TO_BE_INFERRED)
pub fn inout_sig(inputs: impl Into<TypeRow>, outputs: impl Into<TypeRow>) -> Signature {
Signature::new(inputs, outputs).with_extension_delta(TO_BE_INFERRED)
}

#[derive(Debug, Clone, PartialEq, Error)]
Expand Down Expand Up @@ -236,7 +236,7 @@ pub(crate) mod test {
use crate::hugr::{views::HugrView, HugrMut};
use crate::ops;
use crate::std_extensions::arithmetic::float_ops::FLOAT_OPS_REGISTRY;
use crate::types::{FunctionType, PolyFuncType, Type};
use crate::types::{PolyFuncType, Signature, Type};
use crate::{type_row, Hugr};

use super::handle::BuildHandle;
Expand Down Expand Up @@ -272,24 +272,23 @@ pub(crate) mod test {

#[fixture]
pub(crate) fn simple_dfg_hugr() -> Hugr {
let dfg_builder =
DFGBuilder::new(FunctionType::new(type_row![BIT], type_row![BIT])).unwrap();
let dfg_builder = DFGBuilder::new(Signature::new(type_row![BIT], type_row![BIT])).unwrap();
let [i1] = dfg_builder.input_wires_arr();
dfg_builder.finish_prelude_hugr_with_outputs([i1]).unwrap()
}

#[fixture]
pub(crate) fn simple_cfg_hugr() -> Hugr {
let mut cfg_builder =
CFGBuilder::new(FunctionType::new(type_row![NAT], type_row![NAT])).unwrap();
CFGBuilder::new(Signature::new(type_row![NAT], type_row![NAT])).unwrap();
super::cfg::test::build_basic_cfg(&mut cfg_builder).unwrap();
cfg_builder.finish_prelude_hugr().unwrap()
}

/// A helper method which creates a DFG rooted hugr with Input and Output node
/// only (no wires), given a function type with extension delta.
// TODO consider taking two type rows and using TO_BE_INFERRED
pub(crate) fn closed_dfg_root_hugr(signature: FunctionType) -> Hugr {
pub(crate) fn closed_dfg_root_hugr(signature: Signature) -> Hugr {
let mut hugr = Hugr::new(ops::DFG {
signature: signature.clone(),
});
Expand Down
20 changes: 5 additions & 15 deletions hugr-core/src/builder/build_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,8 @@ use crate::{
types::EdgeKind,
};

use crate::extension::{
ExtensionRegistry, ExtensionSet, SignatureError, PRELUDE_REGISTRY, TO_BE_INFERRED,
};
use crate::types::{FunctionType, PolyFuncType, Type, TypeArg, TypeRow};
use crate::extension::{ExtensionRegistry, ExtensionSet, PRELUDE_REGISTRY, TO_BE_INFERRED};
use crate::types::{PolyFuncType, Signature, Type, TypeArg, TypeRow};

use itertools::Itertools;

Expand Down Expand Up @@ -276,7 +274,7 @@ pub trait Dataflow: Container {
// TODO: Should this be one function, or should there be a temporary "op" one like with the others?
fn dfg_builder(
&mut self,
signature: FunctionType,
signature: Signature,
input_wires: impl IntoIterator<Item = Wire>,
) -> Result<DFGBuilder<&mut Hugr>, BuildError> {
let op = ops::DFG {
Expand All @@ -297,7 +295,7 @@ pub trait Dataflow: Container {
) -> Result<DFGBuilder<&mut Hugr>, BuildError> {
let (types, input_wires): (Vec<Type>, Vec<Wire>) = inputs.into_iter().unzip();
self.dfg_builder(
FunctionType::new_endo(types).with_extension_delta(TO_BE_INFERRED),
Signature::new_endo(types).with_extension_delta(TO_BE_INFERRED),
input_wires,
)
}
Expand Down Expand Up @@ -325,7 +323,7 @@ pub trait Dataflow: Container {
let (cfg_node, _) = add_node_with_wires(
self,
ops::CFG {
signature: FunctionType::new(inputs.clone(), output_types.clone())
signature: Signature::new(inputs.clone(), output_types.clone())
.with_extension_delta(extension_delta),
},
input_wires,
Expand Down Expand Up @@ -658,14 +656,6 @@ fn add_node_with_wires<T: Dataflow + ?Sized>(
inputs: impl IntoIterator<Item = Wire>,
) -> Result<(Node, usize), BuildError> {
let op = nodetype.into();
// Check there are no row variables, as that would prevent us
// from indexing into the node's ports in order to wire up
op.dataflow_signature()
.as_ref()
.and_then(FunctionType::find_rowvar)
.map_or(Ok(()), |(idx, _)| {
Err(SignatureError::RowVarWhereTypeExpected { idx })
})?;
let num_outputs = op.value_output_count();
let op_node = data_builder.add_child_node(op.clone());

Expand Down
34 changes: 17 additions & 17 deletions hugr-core/src/builder/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
};
use crate::{
extension::{ExtensionRegistry, ExtensionSet},
types::FunctionType,
types::Signature,
};
use crate::{hugr::views::HugrView, types::TypeRow};

Expand Down Expand Up @@ -46,17 +46,17 @@ use crate::{hugr::HugrMut, type_row, Hugr};
/// +------------+
/// */
/// use hugr::{
/// builder::{BuildError, CFGBuilder, Container, Dataflow, HugrBuilder, endo_ft, inout_ft},
/// builder::{BuildError, CFGBuilder, Container, Dataflow, HugrBuilder, endo_sig, inout_sig},
/// extension::{prelude, ExtensionSet},
/// ops, type_row,
/// types::{FunctionType, SumType, Type},
/// types::{Signature, SumType, Type},
/// Hugr,
/// };
///
/// const NAT: Type = prelude::USIZE_T;
///
/// fn make_cfg() -> Result<Hugr, BuildError> {
/// let mut cfg_builder = CFGBuilder::new(FunctionType::new(type_row![NAT], type_row![NAT]))?;
/// let mut cfg_builder = CFGBuilder::new(Signature::new_endo(NAT))?;
///
/// // Outputs from basic blocks must be packed in a sum which corresponds to
/// // which successor to pick. We'll either choose the first branch and pass
Expand Down Expand Up @@ -84,7 +84,7 @@ use crate::{hugr::HugrMut, type_row, Hugr};
/// // `NAT` arguments: one from the `sum_variants` type, and another from the
/// // entry node's `other_outputs`.
/// let mut successor_builder = cfg_builder.simple_block_builder(
/// inout_ft(type_row![NAT, NAT], NAT),
/// inout_sig(type_row![NAT, NAT], NAT),
/// 1, // only one successor to this block
/// )?;
/// let successor_a = {
Expand All @@ -98,7 +98,7 @@ use crate::{hugr::HugrMut, type_row, Hugr};
/// };
///
/// // The only argument to this block is the entry node's `other_outputs`.
/// let mut successor_builder = cfg_builder.simple_block_builder(endo_ft(NAT), 1)?;
/// let mut successor_builder = cfg_builder.simple_block_builder(endo_sig(NAT), 1)?;
/// let successor_b = {
/// let sum_unary = successor_builder.add_load_value(ops::Value::unary_unit_sum());
/// let [in_wire] = successor_builder.input_wires_arr();
Expand Down Expand Up @@ -151,7 +151,7 @@ impl<H: AsMut<Hugr> + AsRef<Hugr>> SubContainer for CFGBuilder<H> {

impl CFGBuilder<Hugr> {
/// New CFG rooted HUGR builder
pub fn new(signature: FunctionType) -> Result<Self, BuildError> {
pub fn new(signature: Signature) -> Result<Self, BuildError> {
let cfg_op = ops::CFG {
signature: signature.clone(),
};
Expand Down Expand Up @@ -272,7 +272,7 @@ impl<B: AsMut<Hugr> + AsRef<Hugr>> CFGBuilder<B> {
/// This function will return an error if there is an error adding the node.
pub fn simple_block_builder(
&mut self,
signature: FunctionType,
signature: Signature,
n_cases: usize,
) -> Result<BlockBuilder<&mut Hugr>, BuildError> {
self.block_builder_exts(
Expand Down Expand Up @@ -463,7 +463,7 @@ pub(crate) mod test {
let build_result = {
let mut module_builder = ModuleBuilder::new();
let mut func_builder = module_builder
.define_function("main", FunctionType::new(vec![NAT], type_row![NAT]))?;
.define_function("main", Signature::new(vec![NAT], type_row![NAT]))?;
let _f_id = {
let [int] = func_builder.input_wires_arr();

Expand All @@ -489,7 +489,7 @@ pub(crate) mod test {
}
#[test]
fn basic_cfg_hugr() -> Result<(), BuildError> {
let mut cfg_builder = CFGBuilder::new(FunctionType::new(type_row![NAT], type_row![NAT]))?;
let mut cfg_builder = CFGBuilder::new(Signature::new(type_row![NAT], type_row![NAT]))?;
build_basic_cfg(&mut cfg_builder)?;
assert_matches!(cfg_builder.finish_prelude_hugr(), Ok(_));

Expand All @@ -511,8 +511,8 @@ pub(crate) mod test {
let sum = entry_b.make_sum(1, sum2_variants, [inw])?;
entry_b.finish_with_outputs(sum, [])?
};
let mut middle_b = cfg_builder
.simple_block_builder(FunctionType::new(type_row![NAT], type_row![NAT]), 1)?;
let mut middle_b =
cfg_builder.simple_block_builder(Signature::new(type_row![NAT], type_row![NAT]), 1)?;
let middle = {
let c = middle_b.add_load_const(ops::Value::unary_unit_sum());
let [inw] = middle_b.input_wires_arr();
Expand All @@ -526,7 +526,7 @@ pub(crate) mod test {
}
#[test]
fn test_dom_edge() -> Result<(), BuildError> {
let mut cfg_builder = CFGBuilder::new(FunctionType::new(type_row![NAT], type_row![NAT]))?;
let mut cfg_builder = CFGBuilder::new(Signature::new(type_row![NAT], type_row![NAT]))?;
let sum_tuple_const = cfg_builder.add_constant(ops::Value::unary_unit_sum());
let sum_variants = vec![type_row![]];

Expand All @@ -542,7 +542,7 @@ pub(crate) mod test {
entry_b.finish_with_outputs(sum, [])?
};
let mut middle_b =
cfg_builder.simple_block_builder(FunctionType::new(type_row![], type_row![NAT]), 1)?;
cfg_builder.simple_block_builder(Signature::new(type_row![], type_row![NAT]), 1)?;
let middle = {
let c = middle_b.load_const(&sum_tuple_const);
middle_b.finish_with_outputs(c, [inw])?
Expand All @@ -557,11 +557,11 @@ pub(crate) mod test {

#[test]
fn test_non_dom_edge() -> Result<(), BuildError> {
let mut cfg_builder = CFGBuilder::new(FunctionType::new(type_row![NAT], type_row![NAT]))?;
let mut cfg_builder = CFGBuilder::new(Signature::new(type_row![NAT], type_row![NAT]))?;
let sum_tuple_const = cfg_builder.add_constant(ops::Value::unary_unit_sum());
let sum_variants = vec![type_row![]];
let mut middle_b = cfg_builder
.simple_block_builder(FunctionType::new(type_row![NAT], type_row![NAT]), 1)?;
let mut middle_b =
cfg_builder.simple_block_builder(Signature::new(type_row![NAT], type_row![NAT]), 1)?;
let [inw] = middle_b.input_wires_arr();
let middle = {
let c = middle_b.load_const(&sum_tuple_const);
Expand Down
12 changes: 6 additions & 6 deletions hugr-core/src/builder/circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,13 +254,13 @@ mod test {
extension::prelude::BOOL_T,
ops::{custom::OpaqueOp, CustomOp},
type_row,
types::FunctionType,
types::Signature,
};

#[test]
fn simple_linear() {
let build_res = build_main(
FunctionType::new(type_row![QB, QB], type_row![QB, QB])
Signature::new(type_row![QB, QB], type_row![QB, QB])
.with_extension_delta(test_quantum_extension::EXTENSION_ID)
.with_extension_delta(float_types::EXTENSION_ID)
.into(),
Expand Down Expand Up @@ -300,10 +300,10 @@ mod test {
"MyOp",
"unknown op".to_string(),
vec![],
FunctionType::new(vec![QB, NAT], vec![QB]),
Signature::new(vec![QB, NAT], vec![QB]),
));
let build_res = build_main(
FunctionType::new(type_row![QB, QB, NAT], type_row![QB, QB, BOOL_T])
Signature::new(type_row![QB, QB, NAT], type_row![QB, QB, BOOL_T])
.with_extension_delta(test_quantum_extension::EXTENSION_ID)
.into(),
|mut f_build| {
Expand All @@ -330,7 +330,7 @@ mod test {
#[test]
fn ancillae() {
let build_res = build_main(
FunctionType::new_endo(QB)
Signature::new_endo(QB)
.with_extension_delta(test_quantum_extension::EXTENSION_ID)
.into(),
|mut f_build| {
Expand Down Expand Up @@ -368,7 +368,7 @@ mod test {
#[test]
fn circuit_builder_errors() {
let _build_res = build_main(
FunctionType::new_endo(type_row![QB, QB]).into(),
Signature::new_endo(type_row![QB, QB]).into(),
|mut f_build| {
let mut circ = f_build.as_circuit(f_build.input_wires());
let [q0, q1] = circ.tracked_units_arr();
Expand Down
10 changes: 5 additions & 5 deletions hugr-core/src/builder/conditional.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::extension::ExtensionRegistry;
use crate::hugr::views::HugrView;
use crate::ops::dataflow::DataflowOpTrait;
use crate::types::{FunctionType, TypeRow};
use crate::types::{Signature, TypeRow};

use crate::ops;
use crate::ops::handle::CaseID;
Expand Down Expand Up @@ -118,7 +118,7 @@ impl<B: AsMut<Hugr> + AsRef<Hugr>> ConditionalBuilder<B> {

let outputs = cond.outputs;
let case_op = ops::Case {
signature: FunctionType::new(inputs.clone(), outputs.clone())
signature: Signature::new(inputs.clone(), outputs.clone())
.with_extension_delta(extension_delta.clone()),
};
let case_node =
Expand All @@ -134,7 +134,7 @@ impl<B: AsMut<Hugr> + AsRef<Hugr>> ConditionalBuilder<B> {
let dfg_builder = DFGBuilder::create_with_io(
self.hugr_mut(),
case_node,
FunctionType::new(inputs, outputs).with_extension_delta(extension_delta),
Signature::new(inputs, outputs).with_extension_delta(extension_delta),
)?;

Ok(CaseBuilder::from_dfg_builder(dfg_builder))
Expand Down Expand Up @@ -186,7 +186,7 @@ impl ConditionalBuilder<Hugr> {

impl CaseBuilder<Hugr> {
/// Initialize a Case rooted HUGR
pub fn new(signature: FunctionType) -> Result<Self, BuildError> {
pub fn new(signature: Signature) -> Result<Self, BuildError> {
let op = ops::Case {
signature: signature.clone(),
};
Expand Down Expand Up @@ -233,7 +233,7 @@ mod test {
let build_result: Result<Hugr, BuildError> = {
let mut module_builder = ModuleBuilder::new();
let mut fbuild = module_builder
.define_function("main", FunctionType::new(type_row![NAT], type_row![NAT]))?;
.define_function("main", Signature::new(type_row![NAT], type_row![NAT]))?;
let tru_const = fbuild.add_constant(Value::true_val());
let _fdef = {
let const_wire = fbuild.load_const(&tru_const);
Expand Down
Loading

0 comments on commit 79029b8

Please sign in to comment.