feat!: Add `monomorphization` pass #1733

acl-cqc · 2024-12-02T10:40:40Z

#1772 tracks adding tests for substitute.

BREAKING CHANGE: Optrait gains Sized and Clone as supertraits and is no longer object safe.
BREAKING CHANGE: DataflowOptrait gains Sized as supertrait.
BREAKING CHANGE: DataflowOptrait has an additional required method substitute.

…e comment

… fn monomorphize

codecov · 2024-12-02T10:42:39Z

Codecov Report

Attention: Patch coverage is 90.61625% with 67 lines in your changes missing coverage. Please review.

Project coverage is 86.72%. Comparing base (187ea8f) to head (ae2b850).
Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
hugr-core/src/ops/dataflow.rs	55.55%	28 Missing ⚠️
hugr-passes/src/monomorphize.rs	95.91%	3 Missing and 17 partials ⚠️
hugr-core/src/ops/controlflow.rs	91.30%	10 Missing ⚠️
hugr-core/src/ops/custom.rs	72.00%	7 Missing ⚠️
hugr-core/src/types/type_param.rs	71.42%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1733      +/-   ##
==========================================
+ Coverage   86.68%   86.72%   +0.04%     
==========================================
  Files         184      186       +2     
  Lines       33169    34000     +831     
  Branches    30044    30875     +831     
==========================================
+ Hits        28752    29486     +734     
- Misses       2765     2845      +80     
- Partials     1652     1669      +17

Flag	Coverage Δ
python	`92.57% <ø> (ø)`
rust	`86.13% <90.61%> (+0.06%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…polyfunc)

hugrbot · 2024-12-02T19:14:55Z

This PR contains breaking changes to the public Rust API.

cargo-semver-checks summary


--- failure trait_added_supertrait: non-sealed trait added new supertraits ---

Description:
A non-sealed trait added one or more supertraits, which breaks downstream implementations of the trait
      ref: https://doc.rust-lang.org/cargo/reference/semver.html#generic-bounds-tighten
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.38.0/src/lints/trait_added_supertrait.ron

Failed in:
trait hugr_core::ops::OpTrait gained Sized in file /home/runner/work/hugr/hugr/PR_BRANCH/hugr-core/src/ops.rs:372
trait hugr_core::ops::OpTrait gained Clone in file /home/runner/work/hugr/hugr/PR_BRANCH/hugr-core/src/ops.rs:372
trait hugr_core::ops::dataflow::DataflowOpTrait gained Sized in file /home/runner/work/hugr/hugr/PR_BRANCH/hugr-core/src/ops/dataflow.rs:16
trait hugr_core::ops::DataflowOpTrait gained Sized in file /home/runner/work/hugr/hugr/PR_BRANCH/hugr-core/src/ops/dataflow.rs:16

--- failure trait_method_added: pub trait method added ---

Description:
A non-sealed public trait added a new method without a default implementation, which breaks downstream implementations of the trait
      ref: https://doc.rust-lang.org/cargo/reference/semver.html#trait-new-item-no-default
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.38.0/src/lints/trait_method_added.ron

Failed in:
trait method hugr_core::ops::dataflow::DataflowOpTrait::substitute in file /home/runner/work/hugr/hugr/PR_BRANCH/hugr-core/src/ops/dataflow.rs:57
trait method hugr_core::ops::DataflowOpTrait::substitute in file /home/runner/work/hugr/hugr/PR_BRANCH/hugr-core/src/ops/dataflow.rs:57

--- failure trait_no_longer_object_safe: trait no longer object safe ---

Description:
Trait is no longer object safe, which breaks `dyn Trait` usage.
      ref: https://doc.rust-lang.org/stable/reference/items/traits.html#object-safety
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.38.0/src/lints/trait_no_longer_object_safe.ron

Failed in:
trait OpTrait in file /home/runner/work/hugr/hugr/PR_BRANCH/hugr-core/src/ops.rs:372

This reverts commit 703004e.

…hize

ss2165 · 2024-12-12T10:47:47Z

hugr-passes/src/monomorphize.rs

+}
+
+fn escape_dollar(str: impl AsRef<str>) -> String {
+    str.as_ref().replace("$", "\\$")


seems a bit risky to be sanitising strings ourselves, should we use a library?

There are no rules on what names functions are allowed, so sanitising is too strong a word for this.

The worst that can happen is that we end up with non-unique names, which is allowed by HUGR and the docs of monomorphize.

Surely there is room for a better, well motivated, and principled design. I think that this is the best we can do for now.

If you have specific suggestions I'm happy to change.

hugr-core/src/builder/build_traits.rs

ss2165 · 2024-12-12T10:55:58Z

hugr-core/src/ops/custom.rs

@@ -259,6 +279,14 @@ impl DataflowOpTrait for OpaqueOp {
    fn signature(&self) -> Cow<'_, Signature> {
        Cow::Borrowed(&self.signature)
    }
+
+    fn substitute(&self, subst: &crate::types::Substitution) -> Self {


I'm surprised this isn't covered, it should be right?

hugr-passes/src/monomorphize.rs

ss2165 · 2024-12-12T11:03:14Z

hugr-core/src/ops/dataflow.rs

@@ -367,6 +419,26 @@ impl DataflowOpTrait for LoadFunction {
    fn static_input(&self) -> Option<EdgeKind> {
        Some(EdgeKind::Function(self.func_sig.clone()))
    }
+


yeah there's a lot of substitute code not covered here :/

Yes, that is addressed in the commit message. We will add proptests in a future PR

hugr-passes/src/monomorphize.rs

Co-authored-by: Seyon Sivarajah <[email protected]>

hugr-passes/src/monomorphize.rs

doug-q · 2024-12-12T11:11:36Z

hugr-core/src/ops/dataflow.rs

@@ -367,6 +419,26 @@ impl DataflowOpTrait for LoadFunction {
    fn static_input(&self) -> Option<EdgeKind> {
        Some(EdgeKind::Function(self.func_sig.clone()))
    }
+


Yes, that is addressed in the commit message. We will add proptests in a future PR

hugr-core/src/builder/build_traits.rs

doug-q · 2024-12-12T11:15:46Z

hugr-passes/src/monomorphize.rs

+}
+
+fn escape_dollar(str: impl AsRef<str>) -> String {
+    str.as_ref().replace("$", "\\$")


There are no rules on what names functions are allowed, so sanitising is too strong a word for this.

The worst that can happen is that we end up with non-unique names, which is allowed by HUGR and the docs of monomorphize.

Surely there is room for a better, well motivated, and principled design. I think that this is the best we can do for now.

If you have specific suggestions I'm happy to change.

Co-authored-by: Seyon Sivarajah <[email protected]>

…hize

ss2165

thanks :)

acl-cqc added 22 commits November 28, 2024 20:24

Add {Dataflow,}OpTrait::substitute - compiles, no tests

24bff91

WIP Add Hugr::substitute

341e537

WIP monomorphize via mono_scan

6a96b2c

Use continue, match instead of if let |

7866a62

WIP start copying edges...no not so simple

871e796

Break out is_polymorphic_funcdefn, tho only used once; remove obsolet…

ac536c5

…e comment

Separate 'fn instantiate' to copy edges, add struct Instantiating; mv…

ff04f13

… fn monomorphize

Use Instantiating::poly_func, drop is_polymorphic_funcdefn; and ..**inst

e5f665b

mangle inner names, switch from impl HugrMut to &mut Hugr

6c00ad0

two-level Instantiations, flatten in instantiate()

b666dd5

Add remove_polyfuncs, using remove_subtree

5a4a0f9

comments

7de2c2b

name_mangle => mangle_name

5bb0c57

WIP tests

b04e1cf

refactor: let mut = Instantiating; use static_output_port

4aa6c79

Fixes: monomorphize Call op; read static input from old_ch

a7d6137

Test: use pair/triple not list

48a6598

Implement mangle_name

937a6d0

fixup! Test: use pair

f15f09f

fix edge handling

d5f15ff

clippy/lint, docs

ad00327

test remove_polyfuncs

c5a0f8d

acl-cqc added 5 commits December 2, 2024 17:41

Shorten test a bit

04d60b1

Factor out is_polymorphic_funcdefn, avoid root

3be790e

remove_polyfuncs if non-module-rooted; implement mangle_inner_func

4a9f7a2

Fix monomorphized func -> Call edges

8178b5d

Test flattening (and multiple calls with different typeargs from one …

2090218

…polyfunc)

Merge remote-tracking branch 'origin/main' into acl/monomorphize

6ff4c3b

doug-q added 9 commits December 11, 2024 11:37

Merge branch 'main' of gh:CQCL-DEV/hugr into acl/monomorphize

efc461a

wip

703004e

Revert "wip"

43c2f3e

This reverts commit 703004e.

Merge branch 'main' of gh:CQCL-DEV/hugr into acl/monomorphize

bc839b7

fix up merge

0adb432

Merge branch 'acl/monomorphize' of gh:CQCL-DEV/hugr into acl/monomorp…

d933d59

…hize

fmt

5845706

add array extension to signatures

6049889

fmt

144fc47

doug-q changed the title ~~feat: Monomorphize Hugrs~~ feat!: Monomorphize Hugrs Dec 12, 2024

doug-q added 2 commits December 12, 2024 08:23

lint

becc847

docs + tweak name mangling

df38d88

doug-q changed the title ~~feat!: Monomorphize Hugrs~~ feat!: Add monomorphization pass. Dec 12, 2024

doug-q changed the title ~~feat!: Add monomorphization pass.~~ feat!: Add monomorphization pass Dec 12, 2024

docs

4bde01d

ss2165 self-requested a review December 12, 2024 10:26

ss2165 reviewed Dec 12, 2024

View reviewed changes

doug-q and others added 2 commits December 12, 2024 11:11

Update hugr-passes/src/monomorphize.rs

d693c81

Co-authored-by: Seyon Sivarajah <[email protected]>

Update hugr-passes/src/monomorphize.rs

61afb2e

Co-authored-by: Seyon Sivarajah <[email protected]>

doug-q reviewed Dec 12, 2024

View reviewed changes

doug-q and others added 4 commits December 12, 2024 11:16

Update hugr-passes/src/monomorphize.rs

7618fa3

Update hugr-passes/src/monomorphize.rs

44a354c

Co-authored-by: Seyon Sivarajah <[email protected]>

remove call_indirect helper

821c68e

Merge branch 'acl/monomorphize' of gh:CQCL-DEV/hugr into acl/monomorp…

ae2b850

…hize

ss2165 approved these changes Dec 12, 2024

View reviewed changes

doug-q added this pull request to the merge queue Dec 12, 2024

Merged via the queue into main with commit b63aabc Dec 12, 2024
24 checks passed

doug-q deleted the acl/monomorphize branch December 12, 2024 12:29

hugrbot mentioned this pull request Dec 12, 2024

chore: release #1778

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: Add `monomorphization` pass #1733

feat!: Add `monomorphization` pass #1733

acl-cqc commented Dec 2, 2024 •

edited by doug-q

Loading

codecov bot commented Dec 2, 2024 •

edited

Loading

hugrbot commented Dec 2, 2024 •

edited

Loading

ss2165 Dec 12, 2024

doug-q Dec 12, 2024

ss2165 Dec 12, 2024

ss2165 Dec 12, 2024

doug-q Dec 12, 2024

doug-q Dec 12, 2024

doug-q Dec 12, 2024

ss2165 left a comment

feat!: Add monomorphization pass #1733

feat!: Add monomorphization pass #1733

Conversation

acl-cqc commented Dec 2, 2024 • edited by doug-q Loading

codecov bot commented Dec 2, 2024 • edited Loading

Codecov Report

hugrbot commented Dec 2, 2024 • edited Loading

ss2165 Dec 12, 2024

Choose a reason for hiding this comment

doug-q Dec 12, 2024

Choose a reason for hiding this comment

ss2165 Dec 12, 2024

Choose a reason for hiding this comment

ss2165 Dec 12, 2024

Choose a reason for hiding this comment

doug-q Dec 12, 2024

Choose a reason for hiding this comment

doug-q Dec 12, 2024

Choose a reason for hiding this comment

doug-q Dec 12, 2024

Choose a reason for hiding this comment

ss2165 left a comment

Choose a reason for hiding this comment

feat!: Add `monomorphization` pass #1733

feat!: Add `monomorphization` pass #1733

acl-cqc commented Dec 2, 2024 •

edited by doug-q

Loading

codecov bot commented Dec 2, 2024 •

edited

Loading

hugrbot commented Dec 2, 2024 •

edited

Loading