refactor!: Separate Signature from FuncValueType by parametrizing Type(/Row)/etc. #1138

acl-cqc · 2024-05-30T08:13:16Z

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!)

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

…kes impl Into, update Dataflow/Ops etc.

Do PartialEq better, and conversions Lots of special cases in TypeParam removed as we can rule them out statically (switch representation - a row variable is a TypeArg::Variable not a TypeArg::Type)

…ROW(_REF)

…type

…<true>

…substitute)

…type

doug-q

Brilliant. I admit I have not carefully re-reviewed every line, having done so multiple times in previous iterations. I have carefully reviewed all of create::types::*. A few comments, but I recommend merging immediately.

doug-q · 2024-07-16T07:27:22Z

hugr-core/src/extension/op_def.rs

@@ -186,15 +186,21 @@ impl<T: CustomSignatureFunc + 'static> From<T> for SignatureFunc {
    }
 }

-impl From<PolyFuncType> for SignatureFunc {
-    fn from(v: PolyFuncType) -> Self {
+impl From<TypeSchemeRV> for SignatureFunc {


I think impl From<TypeScheme> is missing. I think you could do all four instances with impl<T: Into<TypeSchemRV>> From<T> for SignatureFunc

Rust complains, because some future type could define both Into (currently Into<PolyFuncTypeRV>) and Into<CustomSignatureFunc> and then it would be ambiguous...

I tried impl <RV: MaybeRV> From<PolyFuncTypeBase<RV>> for SignatureFunc but there is no conversion from PolyFuncTypeBase<RV> (parametrized) to <RowVariable>... I could add a .into_() but would rather avoid that if I can...

I can add the fourth (From<PolyFuncType>) but I couldn't see a scenario where you're making an OpDef but didn't know to create a PolyFuncTypeRV (rather than PolyFuncType) in the first place...

It's easy to add though, done

doug-q · 2024-07-17T07:18:53Z

hugr-core/src/types/type_row.rs

+        }
+    }
+
+    pub(super) fn validate(


I think this should be pub, doesn't need to be done in this PR.

doug-q · 2024-07-17T07:47:17Z

hugr-core/src/types/type_param.rs

@@ -185,7 +185,16 @@ pub enum TypeArg {

 impl From<Type> for TypeArg {


I think this can be implemented as impl<RV: MaybeRV> From<TypeBase<RV>> for TypeArg.

Can be done in later

Sadly not - try_into has a different Result type according to RV, the Err could be RowVariable or Infallible (the latter from a library blanket impl). Could add a impl<RV:MaybeRV> TypeBase<RV> { fn try_into_(self) -> Result<Type, RowVariable> } I guess...

I think that is slightly better, though

Added try_into_type

…eArg

acl-cqc · 2024-07-17T10:06:47Z

Ok thanks @doug-q! :).

I'm not touching serialization. I think it's all fine 🔥 we just don't extend the separation into the python world. (At least not yet, possibly never.)

## 🤖 New release * `hugr`: 0.8.0 -> 0.9.0 * `hugr-core`: 0.5.0 -> 0.6.0 * `hugr-passes`: 0.5.0 -> 0.6.0 * `hugr-cli`: 0.1.4 -> 0.2.0 <details><summary>Changelog</summary> ## `hugr` <blockquote> ## 0.9.0 (2024-07-19) ### Bug Fixes - Add op's extension to signature check in `resolve_opaque_op` ([#1317](#1317)) - Panic on `SimpleReplace` with multiports ([#1324](#1324)) ### Refactor - [**breaking**] Separate Signature from FuncValueType by parametrizing Type(/Row)/etc. ([#1138](#1138)) ### Testing - Verify order edges ([#1293](#1293)) - Add failing test case for [#1315](#1315) ([#1316](#1316)) </blockquote> ## `hugr-core` <blockquote> ## 0.6.0 (2024-07-19) ### Bug Fixes - Add op's extension to signature check in `resolve_opaque_op` ([#1317](#1317)) - Panic on `SimpleReplace` with multiports ([#1324](#1324)) ### Refactor - [**breaking**] Separate Signature from FuncValueType by parametrizing Type(/Row)/etc. ([#1138](#1138)) ### Testing - Verify order edges ([#1293](#1293)) - Add failing test case for [#1315](#1315) ([#1316](#1316)) </blockquote> ## `hugr-passes` <blockquote> ## 0.6.0 (2024-07-19) ### Refactor - [**breaking**] Separate Signature from FuncValueType by parametrizing Type(/Row)/etc. ([#1138](#1138)) </blockquote> ## `hugr-cli` <blockquote> ## 0.2.0 (2024-07-19) ### Refactor - [**breaking**] Separate Signature from FuncValueType by parametrizing Type(/Row)/etc. ([#1138](#1138)) </blockquote> </details> --- This PR was generated with [release-plz](https://github.com/MarcoIeni/release-plz/). --------- Co-authored-by: Douglas Wilson <douglas.wilson@quantinuum.com>

acl-cqc added 16 commits May 26, 2024 09:21

FunctionType<bool> defaults true, false->Signature, PFT<bool>::new ta…

b5ddc8f

…kes impl Into, update Dataflow/Ops etc.

PartialEq FunctionType for Signature (?)

066259e

PolyFuncType defaults to <true>

077dabe

Fix a few more funcs to return Signature

f2f420e

Fix called_function_type to return PFT<false>, update a validate

71b6782

imports

6aeffbc

Builder/validation checks against rowvars now impossible

748b49f

Try parametrizing Type(,Row, but not Enum)

b180d96

Do PartialEq better, and conversions Lots of special cases in TypeParam removed as we can rule them out statically (switch representation - a row variable is a TypeArg::Variable not a TypeArg::Type)

Fix a few new_sums

7e0c49e

imports

f4b8ad2

Drop any_non_row_var, use any::<Type<false>>

02aca28

Oops, missed a Signature::try_new, no need now

824ca29

Restore Eq for EdgeKind

12936c6

Make type_row! macro always produce a TypeRow<false>, keep EMPTY_TYPE…

db11b17

…ROW(_REF)

Parametrize new_tuple over <RV2:bool>, also new_sum (awkwardly)

2fa7feb

Comment in leaf.rs re. Tag - could take some TypeRow<true>s

825e911

acl-cqc requested a review from doug-q May 30, 2024 08:13

acl-cqc force-pushed the refactor/parametrize_type branch from 0b608a5 to 825e911 Compare May 30, 2024 09:33

acl-cqc added 11 commits May 30, 2024 10:37

Merge remote-tracking branch 'origin/main' into refactor/parametrize_…

d82e3a7

…type

Double-parametrized PartialEq for TypeRow

b83bee1

(FunctionType,PolyFuncType,TypeRow)::validate for <RV>

3ccef19

sum_with_error: fix with 2*.into

c9b80f9

TypeRow: drop false->true Into, add TypeRow<RV>::into_rv() -> TypeRow…

5849a4f

…<true>

Signature -> SignatureFunc

0e3691c

logic.rs: add extra TypeRow::from

d3ccd79

Add SumTypeError::VariantNotConcrete

c2c0cd9

fmt

eefc35d

Various clippy's

a22e5a1

Rename Type<false>::substitute(=>substitute1), Type<RV>::subst_vec(=>…

15a1bbd

…substitute)

acl-cqc mentioned this pull request May 30, 2024

Infer extension-requirements for nested DFGs #640

Closed

acl-cqc added 8 commits July 15, 2024 09:44

remove type annots, fix doclinks, various comments

23db41d

Merge 'origin/main', fix as_tuple

9421011

fmt

3245b45

TypeScheme(RV/Base) back to PolyFuncType(RV/Base)

eb3e86c

FunctionTypeRV -> FuncValueType

4a40b9b

Merge remote-tracking branch 'origin/main' into refactor/parametrize_…

f173c50

…type

FunctionType -> Signature (take 2)

dcef178

Rename {in_out,endo}_{ft=>sig}

849fd35

acl-cqc force-pushed the refactor/parametrize_type branch from 53e1c4d to 849fd35 Compare July 16, 2024 09:46

acl-cqc added 2 commits July 16, 2024 10:55

fix extension tests

889b91f

doc fixes

46b54e3

acl-cqc changed the title ~~refactor!: Split (Poly/)(Function/)Type(/Row) into RowVariable/NoRV parametrizations~~ refactor!: Separate Signature from FuncValueType by parametrizing Type(/Row)/etc. Jul 16, 2024

acl-cqc added 2 commits July 16, 2024 16:06

Merge remote-tracking branch 'origin/main' into refactor/parametrize_…

bb2cb35

…type

Give exact expected error in dataflow.rs / no_outer_row_variables

58d66ff

doug-q approved these changes Jul 17, 2024

View reviewed changes

Add From<PolyFuncType> for SignatureFunc

40c989b

acl-cqc enabled auto-merge July 17, 2024 09:40

acl-cqc added this pull request to the merge queue Jul 17, 2024

acl-cqc removed this pull request from the merge queue due to a manual request Jul 17, 2024

acl-cqc added 2 commits July 17, 2024 10:48

Add TypeBase<RV>::try_into_type(), combine From<TypeBase<RV>> for Typ…

17ea37e

…eArg

oops, fix doc

eb4152a

acl-cqc added this pull request to the merge queue Jul 17, 2024

Merged via the queue into main with commit b832274 Jul 17, 2024
20 checks passed

acl-cqc deleted the refactor/parametrize_type branch July 17, 2024 10:08

hugrbot mentioned this pull request Jul 17, 2024

chore: release #1314

Merged

hugrbot mentioned this pull request Jul 22, 2024

chore: release #1330

Merged

doug-q mentioned this pull request Jul 25, 2024

Separate type of node signature from function value #1094

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor!: Separate Signature from FuncValueType by parametrizing Type(/Row)/etc. #1138

refactor!: Separate Signature from FuncValueType by parametrizing Type(/Row)/etc. #1138

acl-cqc commented May 30, 2024 •

edited

Loading

doug-q left a comment

doug-q Jul 16, 2024

acl-cqc Jul 17, 2024

acl-cqc Jul 17, 2024

acl-cqc Jul 17, 2024

acl-cqc Jul 17, 2024

doug-q Jul 17, 2024

doug-q Jul 17, 2024

acl-cqc Jul 17, 2024

acl-cqc Jul 17, 2024

acl-cqc Jul 17, 2024

acl-cqc commented Jul 17, 2024

		@@ -185,7 +185,16 @@ pub enum TypeArg {

		impl From<Type> for TypeArg {

refactor!: Separate Signature from FuncValueType by parametrizing Type(/Row)/etc. #1138

refactor!: Separate Signature from FuncValueType by parametrizing Type(/Row)/etc. #1138

Conversation

acl-cqc commented May 30, 2024 • edited Loading

doug-q left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acl-cqc commented Jul 17, 2024

acl-cqc commented May 30, 2024 •

edited

Loading