Skip to content

feat!: Extend LowerTypes pass to linearize by inserting copy/discard #2018

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 129 commits into from
Apr 2, 2025

Conversation

acl-cqc
Copy link
Contributor

@acl-cqc acl-cqc commented Mar 21, 2025

BREAKING CHANGE: OpReplacement renamed to NodeTemplate. Note this is not a breaking change if this PR goes in the same release as #1989 (which introduced OpReplacement)

acl-cqc added 30 commits March 17, 2025 19:14
@hugrbot
Copy link
Collaborator

hugrbot commented Mar 28, 2025

This PR contains breaking changes to the public Rust API.

cargo-semver-checks summary

--- failure enum_missing: pub enum removed or renamed ---

Description:
A publicly-visible enum cannot be imported by its prior path. A `pub use` may have been removed, or the enum itself may have been renamed or removed entirely.
      ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.40.0/src/lints/enum_missing.ron

Failed in:
enum hugr_passes::replace_types::OpReplacement, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-passes/src/replace_types.rs:28

@acl-cqc acl-cqc changed the title feat: Extend LowerTypes pass to linearize by inserting copy/discard feat!: Extend LowerTypes pass to linearize by inserting copy/discard Mar 28, 2025
@acl-cqc acl-cqc requested a review from doug-q March 28, 2025 21:34
@acl-cqc
Copy link
Contributor Author

acl-cqc commented Mar 28, 2025

@doug-q ok so with the OpReplacement -> NodeTemplate rename the pressure of time kicks in, but I think I've done most of the things you suggest (good comments, thank you!) - the only thing that remains is the errors: I kinda disagree with your point about builder errors, I think those are our mistakes, but I've noted in my replies a couple of things I would be happy to make into reported errors (easy ones, too!), what do you think?

EDIT: I've also remove the linearize_x methods from ReplaceTypes and just expose the Linearizer directly. This was better in terms of giving one place in which to concentrate docs; and in the future, one can imagine giving the linearizer a run method that, without replacing types, just checks every outport of every node. (I think this can come after #1895, given that validation before Linearizer will fail! But we can keep the idea that ReplaceTypes has its own Linearizer that it uses only on nodes that it has changed.)

/// as part of an [NodeTemplate::CompoundOp].
///
/// Note that [Self::register] takes precedence when the `src` types overlap.
pub fn register_parametric(
Copy link
Contributor Author

@acl-cqc acl-cqc Mar 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So perhaps that this one takes a callback, supporting arbitrary N, whereas register has only 0-ary discard and 2-ary copy, might be the bigger difference; both can be used for monomorphic types. It might be better to rename, thus

  • this one to register_callback or register_with_callback or even just register?
  • register maybe to register_binary / register_2way / register_(dup/copy)_discard / register_simple ??

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like register_simple/register_callback best, but do not feel strongly about it..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM, done

@acl-cqc
Copy link
Contributor Author

acl-cqc commented Mar 31, 2025

Ok @doug-q fire away. I tried using &dyn Linearizer rather than the rather-odd-seeming-on-its-own CallbackHandler....yes dyn is inefficient but that seemed a lot cleaner...however it required making Linearizer object-safe and other changes (but you can see cc4fd68)

Copy link
Collaborator

@doug-q doug-q left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks Alan. I will fix CI.

@acl-cqc acl-cqc enabled auto-merge April 2, 2025 09:19
@acl-cqc acl-cqc added this pull request to the merge queue Apr 2, 2025
Merged via the queue into main with commit 9ad9e6d Apr 2, 2025
25 checks passed
@acl-cqc acl-cqc deleted the acl/lower_types_linearize branch April 2, 2025 09:34
@hugrbot hugrbot mentioned this pull request Apr 2, 2025
github-merge-queue bot pushed a commit that referenced this pull request Apr 2, 2025
## 🤖 New release

* `hugr-model`: 0.18.1 -> 0.19.0 (⚠ API breaking changes)
* `hugr-core`: 0.15.2 -> 0.15.3 ~(⚠ API breaking changes)~
* `hugr-llvm`: 0.15.2 -> 0.15.3 (✓ API compatible changes)
* `hugr-passes`: 0.15.2 -> 0.15.3 (✓ API compatible changes)
* `hugr`: 0.15.2 -> 0.15.3 (✓ API compatible changes)
* `hugr-cli`: 0.15.2 -> 0.15.3 (✓ API compatible changes)

### ⚠ `hugr-model` breaking changes

```text
--- failure enum_missing: pub enum removed or renamed ---

Description:
A publicly-visible enum cannot be imported by its prior path. A `pub use` may have been removed, or the enum itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.40.0/src/lints/enum_missing.ron

Failed in:
  enum hugr_model::v0::table::ExtSetPart, previously in file /tmp/.tmpDqJ3X7/hugr-model/src/v0/table/mod.rs:352

--- failure enum_variant_added: enum variant added on exhaustive enum ---

Description:
A publicly-visible enum without #[non_exhaustive] has a new variant.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#enum-variant-new
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.40.0/src/lints/enum_variant_added.ron

Failed in:
  variant Term:Func in /tmp/.tmpIpZPlW/hugr/hugr-model/src/v0/table/mod.rs:341

--- failure enum_variant_missing: pub enum variant removed or renamed ---

Description:
A publicly-visible enum has at least one variant that is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.40.0/src/lints/enum_variant_missing.ron

Failed in:
  variant Term::ExtSet, previously in file /tmp/.tmpDqJ3X7/hugr-model/src/v0/table/mod.rs:316
  variant Term::ConstFunc, previously in file /tmp/.tmpDqJ3X7/hugr-model/src/v0/table/mod.rs:321
  variant Term::ExtSet, previously in file /tmp/.tmpDqJ3X7/hugr-model/src/v0/ast/mod.rs:237

--- failure pub_module_level_const_missing: pub module-level const is missing ---

Description:
A public const is missing or renamed
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.40.0/src/lints/pub_module_level_const_missing.ron

Failed in:
  CORE_EXT_SET in file /tmp/.tmpDqJ3X7/hugr-model/src/v0/mod.rs:177
```

### ⚠ `hugr-core` breaking changes (⚠️ ignored ⚠️ )

```text
--- failure enum_variant_added: enum variant added on exhaustive enum ---

Description:
A publicly-visible enum without #[non_exhaustive] has a new variant.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#enum-variant-new
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.40.0/src/lints/enum_variant_added.ron

Failed in:
  variant ImportError:OrderHint in /tmp/.tmpIpZPlW/hugr/hugr-core/src/import.rs:73
```

<details><summary><i><b>Changelog</b></i></summary><p>

## `hugr-model`

<blockquote>

##
[0.19.0](hugr-model-v0.18.1...hugr-model-v0.19.0)
- 2025-04-02

### New Features

- Python bindings for `hugr-model`.
([#1959](#1959))
- Remove extension sets from `hugr-model`.
([#2031](#2031))
- Packages in `hugr-model` and envelope support.
([#2026](#2026))
- Represent order edges in `hugr-model` as metadata.
([#2027](#2027))
</blockquote>

## `hugr-core`

<blockquote>

##
[0.16.0](hugr-core-v0.15.2...hugr-core-v0.16.0)
- 2025-04-02

### Documentation

- Provide docs for array ops, fix bad doc for HugrView::poly_func_type
([#2021](#2021))

### New Features

- Expand SimpleReplacement API
([#1920](#1920))
- Python bindings for `hugr-model`.
([#1959](#1959))
- ReplaceTypes pass allows replacing extension types and ops
([#1989](#1989))
- Remove extension sets from `hugr-model`.
([#2031](#2031))
- Packages in `hugr-model` and envelope support.
([#2026](#2026))
- Represent order edges in `hugr-model` as metadata.
([#2027](#2027))
- add `build_expect_sum` to allow specific error messages
([#2032](#2032))
</blockquote>

## `hugr-llvm`

<blockquote>

##
[0.16.0](hugr-llvm-v0.15.2...hugr-llvm-v0.16.0)
- 2025-04-02

### New Features

- *(hugr-llvm)* Add llvm codegen for `arithmetic.float.fpow`
([#2042](#2042))
- *(hugr-llvm)* Emit divmod and mod operations
([#2025](#2025))
</blockquote>

## `hugr-passes`

<blockquote>

##
[0.16.0](hugr-passes-v0.15.2...hugr-passes-v0.16.0)
- 2025-04-02

### New Features

- ReplaceTypes pass allows replacing extension types and ops
([#1989](#1989))
- MakeTuple->UnpackTuple elision pass
([#2012](#2012))
- [**breaking**] Extend LowerTypes pass to linearize by inserting
copy/discard ([#2018](#2018))
</blockquote>

## `hugr`

<blockquote>

##
[0.16.0](hugr-v0.15.2...hugr-v0.16.0)
- 2025-04-02

### Documentation

- Provide docs for array ops, fix bad doc for HugrView::poly_func_type
([#2021](#2021))

### New Features

- MakeTuple->UnpackTuple elision pass
([#2012](#2012))
- [**breaking**] Extend LowerTypes pass to linearize by inserting
copy/discard ([#2018](#2018))
- Expand SimpleReplacement API
([#1920](#1920))
- Python bindings for `hugr-model`.
([#1959](#1959))
- ReplaceTypes pass allows replacing extension types and ops
([#1989](#1989))
- Remove extension sets from `hugr-model`.
([#2031](#2031))
- Represent order edges in `hugr-model` as metadata.
([#2027](#2027))
- add `build_expect_sum` to allow specific error messages
([#2032](#2032))
- Packages in `hugr-model` and envelope support.
([#2026](#2026))
</blockquote>

## `hugr-cli`

<blockquote>

##
[0.16.0](hugr-cli-v0.15.2...hugr-cli-v0.16.0)
- 2025-04-02

### Documentation

- Add usage info to hugr-cli's rustdocs
([#2044](#2044))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).

---------

Co-authored-by: Agustín Borgna <[email protected]>
This was referenced Apr 3, 2025
@hugrbot hugrbot mentioned this pull request Apr 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants