New/validate types #467
New/validate types #467
Conversation
src/types/custom.rs
Outdated
| SmolStr::new_inline("MyRsrc"), | ||
| TypeBound::Eq, | ||
| ); | ||
|
|
||
| pub(crate) const COPYABLE_CUST: CustomType = CustomType::new_simple( |
There was a problem hiding this comment.
Could replace this with FLOAT64_T?
There was a problem hiding this comment.
I guess it is actually used to test custom types later
There was a problem hiding this comment.
I'd very happily move this back, or indeed remove it, if it let me use prelude_registry everywhere rather than needing test_registry as well...but FLOAT64_T is in arithmetic which is outside prelude, right
There was a problem hiding this comment.
I found at least one place calling COPYABLE_T "float" and using it as if it were such, so I replaced that one with FLOAT64_TYPE....then if I'd done one then I've now done all the others...RIP COPYABLE_T.
I've kept test_registry (==prelude + float_types), but could do the move yes
There was a problem hiding this comment.
Let's leave moving until someone adds a load of rotation operations that use angles...
src/extension.rs
Outdated
| let mut reg = Self::new(); | ||
| for ext in value.into_iter() { | ||
| let prev = reg.0.insert(ext.name.clone(), ext); | ||
| assert!(prev.is_none()); |
There was a problem hiding this comment.
Could be a try from, or at least add a message to the assert
There was a problem hiding this comment.
Done the latter, try-from leads to a conflicting instance error, or else one about uncovered type params. (There's blanket TryFrom = From + Infallible thrown into the mix.)
| // And check they fit into the TypeParams declared by the TypeDef | ||
| let ex = extension_registry.get(&self.extension); | ||
| // Even if OpDef's (+binaries) are not available, the part of the Extension definition | ||
| // describing the TypeDefs can easily be passed around (serialized), so should be available. |
There was a problem hiding this comment.
@ss2165 note this is a significant policy commitment!
There was a problem hiding this comment.
a commitment in extension loading/unloading you mean?
There was a problem hiding this comment.
Yes, or at least, that when we serialize a Hugr, we should serialize out this part of all Extensions with it
| TypeArg::Type(ty) => ty.validate(extension_registry), | ||
| TypeArg::BoundedNat(_) => Ok(()), | ||
| TypeArg::Opaque(custarg) => { | ||
| // We could also add a facility to Extension to validate that the constant *value* |
There was a problem hiding this comment.
I'm inclined to make an issue for this...?
acl-cqc
force-pushed
the
new/validate_types
branch
from
896fba3
to
cad92b0
Compare
| } | ||
|
|
||
| #[fixture] | ||
| pub(crate) fn simple_dfg_hugr() -> Hugr { | ||
| let dfg_builder = | ||
| DFGBuilder::new(FunctionType::new(type_row![BIT], type_row![BIT])).unwrap(); | ||
| let [i1] = dfg_builder.input_wires_arr(); | ||
| dfg_builder.finish_hugr_with_outputs([i1]).unwrap() | ||
| dfg_builder.finish_prelude_hugr_with_outputs([i1]).unwrap() |
There was a problem hiding this comment.
method names are getting unwieldy, one for another time
BREAKING CHANGE(1): Hugr::validate() and the various finish_hugr[_with_outputs] Builder methods require an ExtensionRegistry, e.g. &EMPTY_REG or &prelude_registry(). Note new Builder methods finish_prelude_hugr[_with_outputs]
BREAKING CHANGE(2): crate::types::test::{EQ_T, COPYABLE_T, ANY_T} are gone - try USIZE_T, FLOAT64_TYPE, QB_T from prelude and/or arithmetic extensions.