Skip to content

Commit

Permalink
feat!: replace opaque type arguments with String (#1328)
Browse files Browse the repository at this point in the history
Closes #1308

BREAKING CHANGE: opaque type parameters replaced with string parameters.
  • Loading branch information
ss2165 authored Jul 24, 2024
1 parent dbb3232 commit 24b2217
Show file tree
Hide file tree
Showing 17 changed files with 315 additions and 465 deletions.
4 changes: 2 additions & 2 deletions hugr-core/src/extension/declarative.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,13 +313,13 @@ extensions:
- name: MyType
description: A simple type with no parameters
# Parametric types are not currently supported.
params: [Any, ["An unbounded natural number", USize]]
params: [String]
operations:
- name: UnsupportedOperation
description: An operation from 3 qubits to 3 qubits
params:
# Parametric operations are not currently supported.
param1: USize
param1: String
signature:
# Type declarations will have their own syntax.
inputs: []
Expand Down
58 changes: 7 additions & 51 deletions hugr-core/src/extension/declarative/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

use crate::extension::{TypeDef, TypeDefBound};
use crate::types::type_param::TypeParam;
use crate::types::{CustomType, TypeBound, TypeName};
use crate::types::{TypeBound, TypeName};
use crate::Extension;

use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -100,14 +100,13 @@ impl From<TypeDefBoundDeclaration> for TypeDefBound {

/// A declarative type parameter definition.
///
/// Either a type, or a 2-element list containing a human-readable name and a type id.
/// Only supports [TypeParam::String]s for now.
#[derive(Debug, Clone, Hash, PartialEq, Eq, Serialize, Deserialize)]
#[serde(untagged)]
enum TypeParamDeclaration {
/// Just the type id.
Type(TypeName),
String,
/// A 2-tuple containing a human-readable name and a type id.
WithDescription(String, TypeName),
WithDescription(String),
}

impl TypeParamDeclaration {
Expand All @@ -119,52 +118,9 @@ impl TypeParamDeclaration {
/// TODO: The parameter description is currently ignored.
pub fn make_type_param(
&self,
extension: &Extension,
ctx: DeclarationContext<'_>,
_extension: &Extension,
_ctx: DeclarationContext<'_>,
) -> Result<TypeParam, ExtensionDeclarationError> {
let instantiate_type = |ty: &TypeDef| -> Result<CustomType, ExtensionDeclarationError> {
match ty.params() {
[] => Ok(ty.instantiate([]).unwrap()),
_ => Err(ExtensionDeclarationError::ParametricTypeParameter {
ext: extension.name().clone(),
ty: self.type_name().clone(),
}),
}
};

// First try the previously defined types in the current extension.
if let Some(ty) = extension.get_type(self.type_name()) {
return Ok(TypeParam::Opaque {
ty: instantiate_type(ty)?,
});
}

// Try every extension in scope.
//
// TODO: Can we resolve the extension id from the type name instead?
for ext in ctx.scope.iter() {
if let Some(ty) = ctx
.registry
.get(ext)
.and_then(|ext| ext.get_type(self.type_name()))
{
return Ok(TypeParam::Opaque {
ty: instantiate_type(ty)?,
});
}
}

Err(ExtensionDeclarationError::MissingType {
ext: extension.name().clone(),
ty: self.type_name().clone(),
})
}

/// Returns the type name of this type parameter.
fn type_name(&self) -> &TypeName {
match self {
Self::Type(ty) => ty,
Self::WithDescription(_, ty) => ty,
}
Ok(TypeParam::String)
}
}
6 changes: 3 additions & 3 deletions hugr-core/src/hugr/serialize/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::ops::custom::{ExtensionOp, OpaqueOp};
use crate::ops::{self, dataflow::IOTrait, Input, Module, Noop, Output, Value, DFG};
use crate::std_extensions::arithmetic::float_types::FLOAT64_TYPE;
use crate::std_extensions::arithmetic::int_ops::INT_OPS_REGISTRY;
use crate::std_extensions::arithmetic::int_types::{int_custom_type, ConstInt, INT_TYPES};
use crate::std_extensions::arithmetic::int_types::{ConstInt, INT_TYPES};
use crate::std_extensions::logic::NotOp;
use crate::types::type_param::TypeParam;
use crate::types::{
Expand Down Expand Up @@ -473,7 +473,7 @@ fn polyfunctype2() -> PolyFuncTypeRV {
#[rstest]
#[case(Signature::new_endo(type_row![]).into())]
#[case(polyfunctype1())]
#[case(PolyFuncType::new([TypeParam::Opaque { ty: int_custom_type(TypeArg::BoundedNat { n: 1 }) }], Signature::new_endo(type_row![Type::new_var_use(0, TypeBound::Copyable)])))]
#[case(PolyFuncType::new([TypeParam::String], Signature::new_endo(type_row![Type::new_var_use(0, TypeBound::Copyable)])))]
#[case(PolyFuncType::new([TypeBound::Eq.into()], Signature::new_endo(type_row![Type::new_var_use(0, TypeBound::Eq)])))]
#[case(PolyFuncType::new([TypeParam::new_list(TypeBound::Any)], Signature::new_endo(type_row![])))]
#[case(PolyFuncType::new([TypeParam::Tuple { params: [TypeBound::Any.into(), TypeParam::bounded_nat(2.try_into().unwrap())].into() }], Signature::new_endo(type_row![])))]
Expand All @@ -486,7 +486,7 @@ fn roundtrip_polyfunctype_fixedlen(#[case] poly_func_type: PolyFuncType) {

#[rstest]
#[case(FuncValueType::new_endo(type_row![]).into())]
#[case(PolyFuncTypeRV::new([TypeParam::Opaque { ty: int_custom_type(TypeArg::BoundedNat { n: 1 }) }], FuncValueType::new_endo(type_row![Type::new_var_use(0, TypeBound::Copyable)])))]
#[case(PolyFuncTypeRV::new([TypeParam::String], FuncValueType::new_endo(type_row![Type::new_var_use(0, TypeBound::Copyable)])))]
#[case(PolyFuncTypeRV::new([TypeBound::Eq.into()], FuncValueType::new_endo(type_row![Type::new_var_use(0, TypeBound::Eq)])))]
#[case(PolyFuncTypeRV::new([TypeParam::new_list(TypeBound::Any)], FuncValueType::new_endo(type_row![])))]
#[case(PolyFuncTypeRV::new([TypeParam::Tuple { params: [TypeBound::Any.into(), TypeParam::bounded_nat(2.try_into().unwrap())].into() }], FuncValueType::new_endo(type_row![])))]
Expand Down
4 changes: 2 additions & 2 deletions hugr-core/src/types/poly_func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ pub(crate) mod test {
use cool_asserts::assert_matches;
use lazy_static::lazy_static;

use crate::extension::prelude::{BOOL_T, PRELUDE_ID, USIZE_CUSTOM_T, USIZE_T};
use crate::extension::prelude::{BOOL_T, PRELUDE_ID, USIZE_T};
use crate::extension::{
ExtensionId, ExtensionRegistry, SignatureError, TypeDefBound, EMPTY_REG, PRELUDE,
PRELUDE_REGISTRY,
Expand Down Expand Up @@ -277,7 +277,7 @@ pub(crate) mod test {
TypeParam::List {
param: Box::new(TypeParam::max_nat()),
},
TypeParam::Opaque { ty: USIZE_CUSTOM_T },
TypeParam::String,
TypeParam::Tuple {
params: vec![TypeBound::Any.into(), TypeParam::max_nat()],
},
Expand Down
104 changes: 19 additions & 85 deletions hugr-core/src/types/type_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,12 @@ use proptest_derive::Arbitrary;
use std::num::NonZeroU64;
use thiserror::Error;

use super::row_var::MaybeRV;
use super::{check_typevar_decl, RowVariable, Substitution, Type, TypeBase, TypeBound};
use crate::extension::ExtensionRegistry;
use crate::extension::ExtensionSet;
use crate::extension::SignatureError;

use super::row_var::MaybeRV;
use super::{check_typevar_decl, CustomType, RowVariable, Substitution, Type, TypeBase, TypeBound};

/// The upper non-inclusive bound of a [`TypeParam::BoundedNat`]
// A None inner value implies the maximum bound: u64::MAX + 1 (all u64 values valid)
#[derive(
Expand Down Expand Up @@ -68,11 +67,8 @@ pub enum TypeParam {
/// Upper bound for the Nat parameter.
bound: UpperBound,
},
/// Argument is a [TypeArg::Opaque], defined by a [CustomType].
Opaque {
/// The [CustomType] defining the parameter.
ty: CustomType,
},
/// Argument is a [TypeArg::String].
String,
/// Argument is a [TypeArg::Sequence]. A list of indeterminate size containing
/// parameters all of the (same) specified element type.
List {
Expand Down Expand Up @@ -119,7 +115,7 @@ impl TypeParam {
(TypeParam::BoundedNat { bound: b1 }, TypeParam::BoundedNat { bound: b2 }) => {
b1.contains(b2)
}
(TypeParam::Opaque { ty: c1 }, TypeParam::Opaque { ty: c2 }) => c1 == c2,
(TypeParam::String, TypeParam::String) => true,
(TypeParam::List { param: e1 }, TypeParam::List { param: e2 }) => e1.contains(e2),
(TypeParam::Tuple { params: es1 }, TypeParam::Tuple { params: es2 }) => {
es1.len() == es2.len() && es1.iter().zip(es2).all(|(e1, e2)| e1.contains(e2))
Expand Down Expand Up @@ -157,11 +153,10 @@ pub enum TypeArg {
#[allow(missing_docs)]
n: u64,
},
///Instance of [TypeParam::Opaque] An opaque value, stored as serialized blob.
Opaque {
///Instance of [TypeParam::String]. UTF-8 encoded string argument.
String {
#[allow(missing_docs)]
#[serde(flatten)]
arg: CustomTypeArg,
arg: String,
},
/// Instance of [TypeParam::List] or [TypeParam::Tuple], defined by a
/// sequence of elements.
Expand Down Expand Up @@ -199,9 +194,9 @@ impl From<u64> for TypeArg {
}
}

impl From<CustomTypeArg> for TypeArg {
fn from(arg: CustomTypeArg) -> Self {
Self::Opaque { arg }
impl From<String> for TypeArg {
fn from(arg: String) -> Self {
TypeArg::String { arg }
}
}

Expand Down Expand Up @@ -261,14 +256,7 @@ impl TypeArg {
) -> Result<(), SignatureError> {
match self {
TypeArg::Type { ty } => ty.validate(extension_registry, var_decls),
TypeArg::BoundedNat { .. } => Ok(()),
TypeArg::Opaque { arg: custarg } => {
// We could also add a facility to Extension to validate that the constant *value*
// here is a valid instance of the type.
// The type must be equal to that declared (in a TypeParam) by the instantiated TypeDef,
// so cannot contain variables declared by the instantiator (providing the TypeArgs)
custarg.typ.validate(extension_registry, &[])
}
TypeArg::BoundedNat { .. } | TypeArg::String { .. } => Ok(()),
TypeArg::Sequence { elems } => elems
.iter()
.try_for_each(|a| a.validate(extension_registry, var_decls)),
Expand All @@ -293,15 +281,7 @@ impl TypeArg {
// RowVariables are represented as TypeArg::Variable
ty.substitute1(t).into()
}
TypeArg::BoundedNat { .. } => self.clone(), // We do not allow variables as bounds on BoundedNat's
TypeArg::Opaque {
arg: CustomTypeArg { typ, .. },
} => {
// The type must be equal to that declared (in a TypeParam) by the instantiated TypeDef,
// so cannot contain variables declared by the instantiator (providing the TypeArgs)
debug_assert_eq!(&typ.substitute(t), typ);
self.clone()
}
TypeArg::BoundedNat { .. } | TypeArg::String { .. } => self.clone(), // We do not allow variables as bounds on BoundedNat's
TypeArg::Sequence { elems } => {
let mut are_types = elems.iter().map(|ta| match ta {
TypeArg::Type { .. } => true,
Expand Down Expand Up @@ -356,29 +336,6 @@ impl TypeArgVariable {
}
}

/// A serialized representation of a value of a [CustomType]
/// restricted to equatable types.
#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
pub struct CustomTypeArg {
/// The type of the constant.
/// (Exact matches only - the constant is exactly this type.)
pub typ: CustomType,
/// Serialized representation.
pub value: serde_yaml::Value,
}

impl CustomTypeArg {
/// Create a new CustomTypeArg. Enforces that the type must be checkable for
/// equality.
pub fn new(typ: CustomType, value: serde_yaml::Value) -> Result<Self, &'static str> {
if typ.bound() == TypeBound::Eq {
Ok(Self { typ, value })
} else {
Err("Only TypeBound::Eq CustomTypes can be used as TypeArgs")
}
}
}

/// Checks a [TypeArg] is as expected for a [TypeParam]
pub fn check_type_arg(arg: &TypeArg, param: &TypeParam) -> Result<(), TypeArgError> {
match (arg, param) {
Expand Down Expand Up @@ -424,11 +381,7 @@ pub fn check_type_arg(arg: &TypeArg, param: &TypeParam) -> Result<(), TypeArgErr
Ok(())
}

(TypeArg::Opaque { arg }, TypeParam::Opaque { ty: param })
if param.bound() == TypeBound::Eq && &arg.typ == param =>
{
Ok(())
}
(TypeArg::String { .. }, TypeParam::String) => Ok(()),
(TypeArg::Extensions { .. }, TypeParam::Extensions) => Ok(()),
_ => Err(TypeArgError::TypeMismatch {
arg: arg.clone(),
Expand Down Expand Up @@ -639,25 +592,10 @@ mod test {

use proptest::prelude::*;

use super::super::{CustomTypeArg, TypeArg, TypeArgVariable, TypeParam, UpperBound};
use super::super::{TypeArg, TypeArgVariable, TypeParam, UpperBound};
use crate::extension::ExtensionSet;
use crate::proptest::{any_serde_yaml_value, RecursionDepth};
use crate::types::{CustomType, Type, TypeBound};

impl Arbitrary for CustomTypeArg {
type Parameters = RecursionDepth;
type Strategy = BoxedStrategy<Self>;
fn arbitrary_with(depth: Self::Parameters) -> Self::Strategy {
(
any_with::<CustomType>(
<CustomType as Arbitrary>::Parameters::new(depth).with_bound(TypeBound::Eq),
),
any_serde_yaml_value(),
)
.prop_map(|(ct, value)| CustomTypeArg::new(ct, value.clone()).unwrap())
.boxed()
}
}
use crate::proptest::RecursionDepth;
use crate::types::{Type, TypeBound};

impl Arbitrary for TypeArgVariable {
type Parameters = RecursionDepth;
Expand All @@ -677,13 +615,11 @@ mod test {
use prop::strategy::Union;
let mut strat = Union::new([
Just(Self::Extensions).boxed(),
Just(Self::String).boxed(),
any::<TypeBound>().prop_map(|b| Self::Type { b }).boxed(),
any::<UpperBound>()
.prop_map(|bound| Self::BoundedNat { bound })
.boxed(),
any_with::<CustomType>(depth.into())
.prop_map(|ty| Self::Opaque { ty })
.boxed(),
]);
if !depth.leaf() {
// we descend here because we these constructors contain TypeParams
Expand All @@ -708,15 +644,13 @@ mod test {
use prop::strategy::Union;
let mut strat = Union::new([
any::<u64>().prop_map(|n| Self::BoundedNat { n }).boxed(),
any::<String>().prop_map(|arg| Self::String { arg }).boxed(),
any::<ExtensionSet>()
.prop_map(|es| Self::Extensions { es })
.boxed(),
any_with::<Type>(depth)
.prop_map(|ty| Self::Type { ty })
.boxed(),
any_with::<CustomTypeArg>(depth)
.prop_map(|arg| Self::Opaque { arg })
.boxed(),
// TODO this is a bit dodgy, TypeArgVariables are supposed
// to be constructed from TypeArg::new_var_use. We are only
// using this instance for serialization now, but if we want
Expand Down
Loading

0 comments on commit 24b2217

Please sign in to comment.