fix: validation outside entrypoint, normalize_cfgs w/ nonlocal edges #2633

acl-cqc · 2025-10-23T11:17:22Z

The bug in validation was simple, validate_subtree was only called beneath the entrypoint. Change that...

Of course there were test failures - Hugrs we thought were valid but actually weren't.

Many of these were ReplaceTypes tests on DFG-entrypoint Hugrs: ReplaceTypes acted only on/beneath the entrypoint, so (the Input/Output of) the FuncDefn containing the DFG no longer matches it. I've changed these to make the FuncDefn be entrypoint, without a DFG, so all relevant nodes are ReplaceType'd.
NormalizeCFGs can move the Entry block of a CFG into a DFG outside it. If there were nonlocal Dom edges from inside that entry block, this would make the Hugr invalid. (Thus, a second bug, which we had not noticed because of the validation bug.) I've fixed this by only transforming if there are no Dom edges from the entry block.
A similar case (when the predecessor of the Exit block can be moved outside the CFG) is now also done only if there are no incoming Dom edges.

…cetype'd

codecov · 2025-10-23T11:23:17Z

Codecov Report

❌ Patch coverage is 98.03922% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 83.35%. Comparing base (39a483f) to head (51ea7cf).
⚠️ Report is 14 commits behind head on main.

Files with missing lines	Patch %	Lines
hugr-passes/src/normalize_cfgs.rs	97.82%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2633      +/-   ##
==========================================
- Coverage   83.39%   83.35%   -0.04%     
==========================================
  Files         259      259              
  Lines       50741    50628     -113     
  Branches    46264    46189      -75     
==========================================
- Hits        42313    42200     -113     
  Misses       6061     6061              
  Partials     2367     2367

Flag	Coverage Δ
python	`91.46% <ø> (-0.08%)`	⬇️
rust	`82.57% <98.03%> (-0.03%)`	⬇️

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.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

hugr-passes/src/normalize_cfgs.rs

ss2165

partial review, will do the rest after agreed upon split in to breaking and non-breakiing

hugr-passes/src/normalize_cfgs.rs

ss2165 · 2025-10-28T17:17:33Z

hugr-passes/src/normalize_cfgs.rs

+        h.output_neighbours(*pred).count() == 1
+         && // Allow only if no node in `pred` has nonlocal inputs
+            h.children(*pred)
+                .all(|ch| h.input_neighbours(ch).all(|n| ancestor_block(h, n).is_none_or(|src| src == *pred)))


this quite an involved one-liner, could be good to split up/share code with the has_nonlocals check above

split up a bit (and added dedup), but not really possible to share (much) - the two examine opposite directions and this makes Ext edges look very different

## 🤖 New release * `hugr-model`: 0.24.0 -> 0.24.1 (✓ API compatible changes) * `hugr-core`: 0.24.0 -> 0.24.1 (✓ API compatible changes) * `hugr-llvm`: 0.24.0 -> 0.24.1 (✓ API compatible changes) * `hugr-passes`: 0.24.0 -> 0.24.1 (✓ API compatible changes) * `hugr-persistent`: 0.3.1 -> 0.3.2 (✓ API compatible changes) * `hugr`: 0.24.0 -> 0.24.1 (✓ API compatible changes) * `hugr-cli`: 0.24.0 -> 0.24.1 (✓ API compatible changes) <details><summary>Changelog</summary> ## `hugr-model` <blockquote> ## [0.24.1](hugr-model-v0.24.0...hugr-model-v0.24.1) - 2025-11-03 ### Bug Fixes - Correct conversion of `table::Term::Tuple` to `ast::Term` ([#2653](#2653)) </blockquote> ## `hugr-core` <blockquote> ## [0.24.1](hugr-core-v0.24.0...hugr-core-v0.24.1) - 2025-11-03 ### Bug Fixes - validation outside entrypoint, normalize_cfgs w/ nonlocal edges ([#2633](#2633)) - SiblingSubgraph::try_from_nodes for non-entrypoint region ([#2655](#2655)) - Correct conversion of `table::Term::Tuple` to `ast::Term` ([#2653](#2653)) ### New Features - track package descriptions when loading ([#2639](#2639)) - *(cli)* describe sub-command ([#2650](#2650)) </blockquote> ## `hugr-llvm` <blockquote> ## [0.24.0](hugr-llvm-v0.23.0...hugr-llvm-v0.24.0) - 2025-10-13 ### New Features - LLVM lowering for borrow arrays using bitmasks ([#2574](#2574)) - *(py, core, llvm)* add `is_borrowed` op for BorrowArray ([#2610](#2610)) ### Refactor - [**breaking**] consistent inout order in borrow array ([#2621](#2621)) </blockquote> ## `hugr-passes` <blockquote> ## [0.24.1](hugr-passes-v0.24.0...hugr-passes-v0.24.1) - 2025-11-03 ### Bug Fixes - validation outside entrypoint, normalize_cfgs w/ nonlocal edges ([#2633](#2633)) </blockquote> ## `hugr-persistent` <blockquote> ## [0.3.2](hugr-persistent-v0.3.1...hugr-persistent-v0.3.2) - 2025-11-03 ### New Features - *(persistent)* More efficient HugrView iterators for PersistentHugr ([#2595](#2595)) </blockquote> ## `hugr` <blockquote> ## [0.24.1](hugr-v0.24.0...hugr-v0.24.1) - 2025-11-03 ### Bug Fixes - validation outside entrypoint, normalize_cfgs w/ nonlocal edges ([#2633](#2633)) - SiblingSubgraph::try_from_nodes for non-entrypoint region ([#2655](#2655)) - Correct conversion of `table::Term::Tuple` to `ast::Term` ([#2653](#2653)) ### New Features - track package descriptions when loading ([#2639](#2639)) - *(cli)* describe sub-command ([#2650](#2650)) </blockquote> ## `hugr-cli` <blockquote> ## [0.24.1](hugr-cli-v0.24.0...hugr-cli-v0.24.1) - 2025-11-03 ### New Features - track package descriptions when loading ([#2639](#2639)) - *(cli)* describe sub-command ([#2650](#2650)) </blockquote> </details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/).

Behaviour fix after #2633, includes reverting test changes there. Behaviour of ReplaceTypes was never documented, but operating only beneath the entrypoint is clearly obstructive, so default to the whole Hugr.

## 🤖 New release * `hugr-model`: 0.24.1 -> 0.24.2 * `hugr-core`: 0.24.1 -> 0.24.2 * `hugr-llvm`: 0.24.1 -> 0.24.2 * `hugr-passes`: 0.24.1 -> 0.24.2 (✓ API compatible changes) * `hugr`: 0.24.1 -> 0.24.2 (✓ API compatible changes) * `hugr-cli`: 0.24.1 -> 0.24.2 (✓ API compatible changes) * `hugr-persistent`: 0.3.2 -> 0.3.3 <details><summary>Changelog</summary> ## `hugr-model` <blockquote> ## [0.24.1](hugr-model-v0.24.0...hugr-model-v0.24.1) - 2025-11-03 ### Bug Fixes - Correct conversion of `table::Term::Tuple` to `ast::Term` ([#2653](#2653)) </blockquote> ## `hugr-core` <blockquote> ## [0.24.1](hugr-core-v0.24.0...hugr-core-v0.24.1) - 2025-11-03 ### Bug Fixes - validation outside entrypoint, normalize_cfgs w/ nonlocal edges ([#2633](#2633)) - SiblingSubgraph::try_from_nodes for non-entrypoint region ([#2655](#2655)) - Correct conversion of `table::Term::Tuple` to `ast::Term` ([#2653](#2653)) ### New Features - track package descriptions when loading ([#2639](#2639)) - *(cli)* describe sub-command ([#2650](#2650)) </blockquote> ## `hugr-llvm` <blockquote> ## [0.24.0](hugr-llvm-v0.23.0...hugr-llvm-v0.24.0) - 2025-10-13 ### New Features - LLVM lowering for borrow arrays using bitmasks ([#2574](#2574)) - *(py, core, llvm)* add `is_borrowed` op for BorrowArray ([#2610](#2610)) ### Refactor - [**breaking**] consistent inout order in borrow array ([#2621](#2621)) </blockquote> ## `hugr-passes` <blockquote> ## [0.24.2](hugr-passes-v0.24.1...hugr-passes-v0.24.2) - 2025-11-03 ### Bug Fixes - ReplaceTypes: operate on whole Hugr, with set_regions ([#2662](#2662)) </blockquote> ## `hugr` <blockquote> ## [0.24.2](hugr-v0.24.1...hugr-v0.24.2) - 2025-11-03 ### Bug Fixes - ReplaceTypes: operate on whole Hugr, with set_regions ([#2662](#2662)) </blockquote> ## `hugr-cli` <blockquote> ## [0.24.1](hugr-cli-v0.24.0...hugr-cli-v0.24.1) - 2025-11-03 ### New Features - track package descriptions when loading ([#2639](#2639)) - *(cli)* describe sub-command ([#2650](#2650)) </blockquote> ## `hugr-persistent` <blockquote> ## [0.3.2](hugr-persistent-v0.3.1...hugr-persistent-v0.3.2) - 2025-11-03 ### New Features - *(persistent)* More efficient HugrView iterators for PersistentHugr ([#2595](#2595)) </blockquote> </details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/).

## 🤖 New release * `hugr-model`: 0.24.2 -> 0.24.3 (✓ API compatible changes) * `hugr-core`: 0.24.2 -> 0.24.3 * `hugr-llvm`: 0.24.2 -> 0.24.3 (✓ API compatible changes) * `hugr-passes`: 0.24.2 -> 0.24.3 (✓ API compatible changes) * `hugr`: 0.24.2 -> 0.24.3 (✓ API compatible changes) * `hugr-cli`: 0.24.2 -> 0.24.3 (✓ API compatible changes) * `hugr-persistent`: 0.3.3 -> 0.3.4 <details><summary>Changelog</summary> ## `hugr-model` <blockquote> ## [0.24.1](hugr-model-v0.24.0...hugr-model-v0.24.1) - 2025-11-03 ### Bug Fixes - Correct conversion of `table::Term::Tuple` to `ast::Term` ([#2653](#2653)) </blockquote> ## `hugr-core` <blockquote> ## [0.24.1](hugr-core-v0.24.0...hugr-core-v0.24.1) - 2025-11-03 ### Bug Fixes - validation outside entrypoint, normalize_cfgs w/ nonlocal edges ([#2633](#2633)) - SiblingSubgraph::try_from_nodes for non-entrypoint region ([#2655](#2655)) - Correct conversion of `table::Term::Tuple` to `ast::Term` ([#2653](#2653)) ### New Features - track package descriptions when loading ([#2639](#2639)) - *(cli)* describe sub-command ([#2650](#2650)) </blockquote> ## `hugr-llvm` <blockquote> ## [0.24.3](hugr-llvm-v0.24.2...hugr-llvm-v0.24.3) - 2025-11-06 ### Bug Fixes - BorrowArray discard handler allows elements to be borrowed ([#2666](#2666)) </blockquote> ## `hugr-passes` <blockquote> ## [0.24.3](hugr-passes-v0.24.2...hugr-passes-v0.24.3) - 2025-11-06 ### Bug Fixes - BorrowArray discard handler allows elements to be borrowed ([#2666](#2666)) </blockquote> ## `hugr` <blockquote> ## [0.24.3](hugr-v0.24.2...hugr-v0.24.3) - 2025-11-06 ### Bug Fixes - BorrowArray discard handler allows elements to be borrowed ([#2666](#2666)) </blockquote> ## `hugr-cli` <blockquote> ## [0.24.1](hugr-cli-v0.24.0...hugr-cli-v0.24.1) - 2025-11-03 ### New Features - track package descriptions when loading ([#2639](#2639)) - *(cli)* describe sub-command ([#2650](#2650)) </blockquote> ## `hugr-persistent` <blockquote> ## [0.3.2](hugr-persistent-v0.3.1...hugr-persistent-v0.3.2) - 2025-11-03 ### New Features - *(persistent)* More efficient HugrView iterators for PersistentHugr ([#2595](#2595)) </blockquote> </details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/).

Breaking followup to #2633. * normalize_cfgs moves the entry block outside the CFG *even if there are outgoing `Dom` edges* by inlining the DFG (so the entry block children become siblings of the CFG), as this makes said edges into valid `Ext` edges. Sadly, I don't see a good/corresponding treatment for the exit dfg; it doesn't help that the source block of the `Dom` edges must dominate the exit (and hence we could break the CFG just before that source block and start another CFG), because even then there could be backedges to the source (new CFG entry) block, so we can't necessarily lift it outside. Options thus seem to be: 1. thread the values from the source block through the whole CFG 2. add extra outputs to the CFG, and thus to each predecessor of the new exit block - but that requires either finding the corresponding Tag, or also adding new inputs to any other successors of those predecessors (and any other predecessors of those, and so on) ...and I've not tried either of those here. BREAKING CHANGE: NormalizeCFGResult specifies entry_nodes_moved not entry_dfg (as no DFG is inserted).

acl-cqc added 3 commits October 23, 2025 11:34

Fix validation...breaking tests

2c647ca

Fix ReplaceTypes tests - Function containing DFG entrypoint not repla…

15b92f7

…cetype'd

normalize_cfgs: handle nonlocal edges from Entry. TODO into exit-pred...

d087d4d

acl-cqc added 2 commits October 23, 2025 15:20

fix test

0d048e2

Outline exit predecessor only if no op inside has nonlocal inputs

485adbd

acl-cqc changed the title ~~fix!: validate outside entry; and normalize_cfgs w/ nonlocal edges~~ fix!: validation outside entrypoint, and normalize_cfgs w/ nonlocal edges Oct 23, 2025

acl-cqc commented Oct 23, 2025

View reviewed changes

hugr-passes/src/normalize_cfgs.rs Outdated Show resolved Hide resolved

acl-cqc marked this pull request as ready for review October 23, 2025 15:06

acl-cqc requested a review from a team as a code owner October 23, 2025 15:06

acl-cqc requested a review from ss2165 October 23, 2025 15:06

ss2165 reviewed Oct 28, 2025

View reviewed changes

hugr-passes/src/normalize_cfgs.rs Outdated Show resolved Hide resolved

don't panic if there are ext edges into exit-predecessor

0908342

acl-cqc force-pushed the acl/validate_outside_entry branch from 5dfa6db to f4a4924 Compare October 28, 2025 15:19

Convert entry to DFG or leave alone if outgoing nonlocals

483b4ba

acl-cqc force-pushed the acl/validate_outside_entry branch from f4a4924 to 483b4ba Compare October 28, 2025 15:36

acl-cqc changed the title ~~fix!: validation outside entrypoint, and normalize_cfgs w/ nonlocal edges~~ fix: validation outside entrypoint, normalize_cfgs w/ nonlocal edges Oct 28, 2025

acl-cqc requested a review from ss2165 October 28, 2025 15:41

acl-cqc mentioned this pull request Oct 28, 2025

feat(hugr-passes)!: normalize_cfgs inlines entry DFG #2649

Merged

ss2165 approved these changes Oct 28, 2025

View reviewed changes

acl-cqc added 2 commits October 28, 2025 18:16

clarify checks

7cdd73c

and more

51ea7cf

acl-cqc enabled auto-merge October 28, 2025 18:41

acl-cqc added this pull request to the merge queue Oct 28, 2025

Merged via the queue into main with commit 01e7ac0 Oct 28, 2025
32 checks passed

acl-cqc deleted the acl/validate_outside_entry branch October 28, 2025 18:50

hugrbot mentioned this pull request Oct 28, 2025

chore: release #2625

Merged

hugrbot mentioned this pull request Nov 3, 2025

chore: release #2661

Merged

acl-cqc mentioned this pull request Nov 3, 2025

fix: ReplaceTypes: operate on whole Hugr, with set_regions #2662

Merged

This was referenced Nov 5, 2025

chore: release #2667

Closed

chore: release #2669

Merged

hugrbot mentioned this pull request Nov 10, 2025

chore: release #2674

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: validation outside entrypoint, normalize_cfgs w/ nonlocal edges #2633

fix: validation outside entrypoint, normalize_cfgs w/ nonlocal edges #2633

Uh oh!

acl-cqc commented Oct 23, 2025 •

edited

Loading

Uh oh!

codecov bot commented Oct 23, 2025 •

edited

Loading

Uh oh!

Uh oh!

ss2165 left a comment

Uh oh!

Uh oh!

Uh oh!

ss2165 Oct 28, 2025

Uh oh!

acl-cqc Oct 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix: validation outside entrypoint, normalize_cfgs w/ nonlocal edges #2633

fix: validation outside entrypoint, normalize_cfgs w/ nonlocal edges #2633

Uh oh!

Conversation

acl-cqc commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

ss2165 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ss2165 Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

acl-cqc Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

acl-cqc commented Oct 23, 2025 •

edited

Loading

codecov bot commented Oct 23, 2025 •

edited

Loading