-
Notifications
You must be signed in to change notification settings - Fork 13
fix: DeadCodeElim keeps consumers of linear outputs #2560
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2560 +/- ##
==========================================
- Coverage 82.94% 82.87% -0.08%
==========================================
Files 254 254
Lines 47590 47691 +101
Branches 43101 43202 +101
==========================================
+ Hits 39475 39524 +49
- Misses 6052 6104 +52
Partials 2063 2063
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
hugr-passes/src/dead_code.rs
Outdated
| for (tgt_n, tgt_port) in h.all_linked_inputs(n) { | ||
| if h.signature(tgt_n) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Query the signature once outside the loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signature of target of edge leaving n, may be different for each edge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, done the other way. This revealed that DCE had been silently skipping any with_entrypoints not in the Hugr (!). I turned that into an assert in e5e8f4c, which required fixing a test, but (a) that's a behaviour change, so should probably be considered breaking; (b) if we do want to do that, should be an actual error, and DeadCodeElimPass has type Error = Infallible, so definitely breaking.
Hence, I've added an explicit check to skip such nodes. If you're ok with that, I'm happy to make an issue/wait-to-merge-PR to turn this into an error for the next breaking release??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ignoring invalid nodes seems like a safe solution here.
Agreed that this should be turned into an error in a breaking release.
(Make an issue and put it on the next release milestone)
… composable::test_then") This reverts commit 1458537bf197cd9248dc28c48b98ce1f36bcfe10. Skip missing entrypoints
Partly addresses #2557 - this would produce a valid Hugr, but still change the `result` semantics, so more is needed. Nonetheless this change seems correct in principle.
Add test pass runs over the circuits from #904. Some notable issues: - The constant folding pass (and DCE) fails on some cases, leaving unconnected qubit ports. CQCL/hugr#2557 Should be fixed on CQCL/hugr#2560 - (Requires Hugr `0.22.4` / `0.23` release) - The basic-block merge rewrite should define a composable pass. CQCL/hugr#2556 - We are missing a simple `InlineCFG` pass to unwrap simple linear CFGs. (Either single blocks, or it can be conbined with `merge_bbs`). CQCL/hugr#945 If we fix those, we should be able to `assert!` that some of the examples here optimize to the identity. We'll need to call pytket optimisations for the rest. - ~Requires a `hugr 0.22.3` release with this fix: CQCL/hugr#2549 - Requires #1118
…2566) At the moment (before and after #2560) it turns out they were silently dropped. This requires changing the error type of DeadCodeElimPass from Infallible, hence breakage is unavoidable. Might this be a good time to change `with_entry_points` to, oh, `with_start_points` or something that reduces confusion with `HugrView::entrypoint`?? Happy to do that renaming here although haven't done so atm. BREAKING CHANGE: DeadCodeElimPass's implementation of ComposablePass now returns an error (not Infallible)
## 🤖 New release
* `hugr-model`: 0.22.4 -> 0.23.0 (✓ API compatible changes)
* `hugr-core`: 0.22.4 -> 0.23.0 (⚠ API breaking changes)
* `hugr-llvm`: 0.22.4 -> 0.23.0 (✓ API compatible changes)
* `hugr-passes`: 0.22.4 -> 0.23.0 (⚠ API breaking changes)
* `hugr-persistent`: 0.2.3 -> 0.3.0 (✓ API compatible changes)
* `hugr`: 0.22.4 -> 0.23.0 (✓ API compatible changes)
* `hugr-cli`: 0.22.4 -> 0.23.0 (✓ API compatible changes)
### ⚠ `hugr-core` breaking changes
```text
--- 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.43.0/src/lints/enum_variant_missing.ron
Failed in:
variant PackageEncodingError::ExtensionVersion, previously in file /tmp/.tmp8eoQo1/hugr-core/src/envelope/package_json.rs:89
--- failure function_parameter_count_changed: pub fn parameter count changed ---
Description:
A publicly-visible function now takes a different number of parameters.
ref: https://doc.rust-lang.org/cargo/reference/semver.html#fn-change-arity
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.43.0/src/lints/function_parameter_count_changed.ron
Failed in:
hugr_core::import::import_package now takes 3 parameters instead of 2, in /tmp/.tmpBTXSed/hugr/hugr-core/src/import.rs:187
--- failure struct_missing: pub struct removed or renamed ---
Description:
A publicly-visible struct cannot be imported by its prior path. A `pub use` may have been removed, or the struct 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.43.0/src/lints/struct_missing.ron
Failed in:
struct hugr_core::extension::prelude::PRELUDE_REGISTRY, previously in file /tmp/.tmp8eoQo1/hugr-core/src/extension/prelude.rs:43
struct hugr_core::extension::PRELUDE_REGISTRY, previously in file /tmp/.tmp8eoQo1/hugr-core/src/extension/prelude.rs:43
struct hugr_core::std_extensions::logic::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/logic.rs:134
struct hugr_core::extension::prelude::PRELUDE, previously in file /tmp/.tmp8eoQo1/hugr-core/src/extension/prelude.rs:43
struct hugr_core::extension::PRELUDE, previously in file /tmp/.tmp8eoQo1/hugr-core/src/extension/prelude.rs:43
struct hugr_core::std_extensions::ptr::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/ptr.rs:112
struct hugr_core::std_extensions::arithmetic::int_types::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/arithmetic/int_types.rs:209
struct hugr_core::std_extensions::arithmetic::conversions::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/arithmetic/conversions.rs:174
struct hugr_core::std_extensions::collections::array::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/collections/array.rs:93
struct hugr_core::std_extensions::arithmetic::int_types::INT_TYPES, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/arithmetic/int_types.rs:49
struct hugr_core::std_extensions::arithmetic::float_types::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/arithmetic/float_types.rs:104
struct hugr_core::std_extensions::collections::static_array::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/collections/static_array.rs:142
struct hugr_core::std_extensions::collections::list::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/collections/list.rs:289
struct hugr_core::std_extensions::STD_REG, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions.rs:35
struct hugr_core::std_extensions::collections::value_array::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/collections/value_array.rs:99
struct hugr_core::std_extensions::collections::borrow_array::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/collections/borrow_array.rs:290
struct hugr_core::std_extensions::arithmetic::float_ops::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/arithmetic/float_ops.rs:115
struct hugr_core::std_extensions::arithmetic::int_ops::EXTENSION, previously in file /tmp/.tmp8eoQo1/hugr-core/src/std_extensions/arithmetic/int_ops.rs:254
```
### ⚠ `hugr-passes` 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.43.0/src/lints/enum_missing.ron
Failed in:
enum hugr_passes::non_local::NonLocalEdgesError, previously in file /tmp/.tmp8eoQo1/hugr-passes/src/non_local.rs:57
```
<details><summary><i><b>Changelog</b></i></summary><p>
## `hugr-model`
<blockquote>
##
[0.23.0](hugr-model-v0.22.4...hugr-model-v0.23.0)
- 2025-09-30
### Bug Fixes
- [**breaking**] Appease `cargo-audit` by replacing unmaintained
dependencies ([#2572](#2572))
### New Features
- Documentation and error hints
([#2523](#2523))
</blockquote>
## `hugr-core`
<blockquote>
##
[0.23.0](hugr-core-v0.22.4...hugr-core-v0.23.0)
- 2025-09-30
### Bug Fixes
- [**breaking**] Appease `cargo-audit` by replacing unmaintained
dependencies ([#2572](#2572))
- *(core)* check extension versions on model import
([#2580](#2580))
- [**breaking**] test extension version compatibility on ModelWithExts
([#2587](#2587))
- *(core)* check used extension versions against resolved extensions
([#2588](#2588))
- [**breaking**] model import loads Package extensions
([#2590](#2590))
### Miscellaneous Tasks
- [**breaking**] Cleanup deprecated definitions
([#2594](#2594))
### New Features
- add trait+funcs for linking Hugrs explicitly by Node
([#2521](#2521))
- Documentation and error hints
([#2523](#2523))
- Allow creating DFG builders from existing hugrs
([#2562](#2562))
- add_input/output for arbitrary DFGBuilders
([#2564](#2564))
- [**breaking**] Return error instead of panicking in
DFGWrapper::add_{in,out}put
([#2571](#2571))
- *(core)* inner acccesors for WithGenerator error
([#2583](#2583))
- Normalize CFGs ([#2591](#2591))
### Refactor
- [**breaking**] Replace lazy_static with std::sync::LazyLock
([#2567](#2567))
</blockquote>
## `hugr-llvm`
<blockquote>
##
[0.23.0](hugr-llvm-v0.22.4...hugr-llvm-v0.23.0)
- 2025-09-30
### Miscellaneous Tasks
- [**breaking**] Cleanup deprecated definitions
([#2594](#2594))
### Refactor
- [**breaking**] Replace lazy_static with std::sync::LazyLock
([#2567](#2567))
### Testing
- Add framework for LLVM execution tests involving panics
([#2568](#2568))
</blockquote>
## `hugr-passes`
<blockquote>
##
[0.23.0](hugr-passes-v0.22.4...hugr-passes-v0.23.0)
- 2025-09-30
### Bug Fixes
- DeadCodeElim keeps consumers of linear outputs
([#2560](#2560))
- [**breaking**] Appease `cargo-audit` by replacing unmaintained
dependencies ([#2572](#2572))
### Miscellaneous Tasks
- [**breaking**] Cleanup deprecated definitions
([#2594](#2594))
### New Features
- [**breaking**] DeadCodeElimPass reports error on non-existent
entry_points ([#2566](#2566))
- Normalize CFGs ([#2591](#2591))
### Refactor
- [**breaking**] Replace lazy_static with std::sync::LazyLock
([#2567](#2567))
</blockquote>
## `hugr-persistent`
<blockquote>
##
[0.3.0](hugr-persistent-v0.2.3...hugr-persistent-v0.3.0)
- 2025-09-30
### Miscellaneous Tasks
- [**breaking**] Cleanup deprecated definitions
([#2594](#2594))
### Refactor
- [**breaking**] Replace lazy_static with std::sync::LazyLock
([#2567](#2567))
</blockquote>
## `hugr`
<blockquote>
##
[0.23.0](hugr-v0.22.4...hugr-v0.23.0)
- 2025-09-30
### Bug Fixes
- DeadCodeElim keeps consumers of linear outputs
([#2560](#2560))
- [**breaking**] Appease `cargo-audit` by replacing unmaintained
dependencies ([#2572](#2572))
- *(core)* check extension versions on model import
([#2580](#2580))
- [**breaking**] test extension version compatibility on ModelWithExts
([#2587](#2587))
- *(core)* check used extension versions against resolved extensions
([#2588](#2588))
- [**breaking**] model import loads Package extensions
([#2590](#2590))
### Miscellaneous Tasks
- [**breaking**] Cleanup deprecated definitions
([#2594](#2594))
### New Features
- [**breaking**] DeadCodeElimPass reports error on non-existent
entry_points ([#2566](#2566))
- add trait+funcs for linking Hugrs explicitly by Node
([#2521](#2521))
- Documentation and error hints
([#2523](#2523))
- Allow creating DFG builders from existing hugrs
([#2562](#2562))
- add_input/output for arbitrary DFGBuilders
([#2564](#2564))
- [**breaking**] Return error instead of panicking in
DFGWrapper::add_{in,out}put
([#2571](#2571))
- *(core)* inner acccesors for WithGenerator error
([#2583](#2583))
- Normalize CFGs ([#2591](#2591))
### Refactor
- [**breaking**] Replace lazy_static with std::sync::LazyLock
([#2567](#2567))
</blockquote>
## `hugr-cli`
<blockquote>
##
[0.22.3](hugr-cli-v0.22.2...hugr-cli-v0.22.3)
- 2025-09-11
### New Features
- *(hugr-cli)* CliError::validate helper
([#2507](#2507))
</blockquote>
</p></details>
---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).
Partly addresses #2557 - this would produce a valid Hugr, but still change the
resultsemantics, so more is needed.Nonetheless this change seems correct in principle.