feat: Define pass application scopes #2772

aborgna-q · 2025-12-19T17:19:54Z

Defines PassScope configurations in both hugr-passes and hugr-py that let users define set the parts of the hugr that a pass should optimize.

Since each pass must be carefully modified to support the new config, this PR only adds the definitions (and a ComposablePass::with_scope method), and states that passes are not required to follow the configuration (for now).
This will let us make the required changes incrementally without blocking the hugr-rs 0.25.0 release. I opened an issue to track the implementations. #2771

Closes #2748. See discussion about pass properties there. We left the NamedFunctions variant to be defined later.

I'll add some python tests later.

codecov · 2025-12-19T17:23:44Z

Codecov Report

❌ Patch coverage is 57.62712% with 75 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.55%. Comparing base (b3cdc4e) to head (11a643e).
⚠️ Report is 48 commits behind head on main.

Files with missing lines	Patch %	Lines
hugr-py/src/hugr/passes/_scope.py	26.47%	50 Missing ⚠️
hugr-passes/src/composable.rs	0.00%	24 Missing ⚠️
hugr-py/src/hugr/passes/_composable_pass.py	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2772      +/-   ##
==========================================
- Coverage   83.69%   83.55%   -0.14%     
==========================================
  Files         265      263       -2     
  Lines       52893    53019     +126     
  Branches    47651    47480     -171     
==========================================
+ Hits        44270    44302      +32     
- Misses       6242     6328      +86     
- Partials     2381     2389       +8

Flag	Coverage Δ
python	`87.21% <28.16%> (-1.63%)`	⬇️
rust	`83.13% <77.35%> (+<0.01%)`	⬆️

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.

acl-cqc

This is pretty good, I particularly like your implementation of "scope" :)

However there are issues about addition of new nodes?
Wrt. recursive and flat regions....so let's think about the two sorts of passes:
- those that operate on flat regions. These can additionally recurse on sub-containers, kinda separately, and this is roughly what you get by calling regions. Note this means that when operating on a region, such passes must not change any container, as they're gonna recurse on it later. (I guess that also means making sure they continue to feed equivalent inputs to the container - so every container/subregion-parent is a black box.) Note that recursive isn't really a question that such a pass would ask, it just looks at regions.
- those that operate on subtrees. In which case these are gonna call roots and then consider all/some descendants. (Note special case when roots includes a function and the entrypoint is inside it, I'm not sure what we can do about that.) This is the case when recursive is required. Perhaps instead roots should also return a bool ?

Finally, with respect to the breakage....

This is breaking now (we're adding a non-default method to the trait). Ok, this isn't so bad given we're about to make a breaking release, but it seems particularly galling that we still leave unsolved #2771. Well, fair, that is a bunch of work - so I we'll roll out "fixes" to individual passes as non-breaking releases, fine :). But then "breaking" 0.26.0 will just change the doc that passes "must" follow the trait method doc? So downstream crates will "upgrade" to a newer hugr-passes in order to assert that they themselves follow that doc?

This means that consumers get the pain now (and let's say they open issues to "properly implement with_scope before 0.26.0, just like we do here). Then at the point when implementing that method and solving the issue is actually required....there's no noise, just a change to a doc; we rely on them to track that from the first issue.

The alternative, rather than adding 10 do-nothing methods, is a trait default method - so this is non-breaking. We could roll this out later (after 0.25.0), maybe even with the first batch of passes using it, say. Then breaking change (0.26.0) removes the default - at which point downstream crates will actually break if they haven't implemented with_scope themselves.

I guess downstream crates would get less warning that they need to do something, so it might be longer before they update to 0.26.0. But feels to me that (if we assume downstream crates follow our example) we'll get a stronger guarantee that they have actually done the right thing for 0.26.0.

Or.... Leave ComposablePass as per 0.24.0, and define a new trait ComposablePassWithScope that's the same but with the new method. Blanket-implement CPWithScope for any ComposablePass, with the do-nothing method, and deprecate ComposablePass, for removal in 0.26.0. I guess that's "the right way"; if this is a big enough concern that we wanna (1) get the new method out there ASAP, (2) give a large window of opportunity for implementation, (3) have a breaking change when it becomes required....is it worth it?

acl-cqc · 2025-12-22T10:07:10Z

hugr-passes/src/composable/scope.rs

@@ -0,0 +1,384 @@
+//! Scope configuration for a pass.
+//!
+//! This defines the parts of the HUGR that a pass should be applied to, and


I reckon you could probably do the module doc with just a one-liner that says "see [PassScope]". I'm not sure whether "Scope configuration" really tells people anything (recognition of a term once they've seen it before), either, as we haven't really defined what "Scope" is, so you could drop that.

acl-cqc · 2025-12-22T11:15:43Z

hugr-passes/src/composable/scope.rs

+
+/// Scope configuration for a pass.
+///
+/// The scope of a pass defines which parts of a HUGR it is allowed to modify.


and the goals/targets of optimization? I think this is still short enough to be the one-line summary

acl-cqc · 2025-12-22T11:17:38Z

hugr-passes/src/composable.rs

+    /// In `hugr 0.25.*`, this configuration is only a guidance, and may be
+    /// ignored by the pass.
+    ///
+    /// From `hugr >=0.26.0`, passes must respect the scope configuration.


Yeah, I'm really unsure about this, see main discussion/reply

acl-cqc · 2025-12-22T11:21:07Z

hugr-passes/src/composable/scope.rs

+/// The scope of a pass defines which parts of a HUGR it is allowed to modify.
+///
+/// Each variant defines the following properties:
+/// - `roots` is a set of **regions** in the HUGR that the pass should be


Consider "flat regions" rather than just "regions"?

Has the term "regions" been precisely defined somewhere else? If not, we should do so here. I believe the correct term from the spec is "Dataflow sibling graph"...

Also, some optimizations like nesting CFGs (perhaps even simpler ones like CFG normalization) cannot necessarily be expressed as operations on flat regions.

Given only EntrypointFlat really depends on the notion of regions, perhaps this should not be a main/defining property of a PassScope but just a helper method for passes that care.

Yeah, having written main PR comment, and noting you have both/separate fn regions and fn roots - I think the latter is the correct approach, whereas the doc here defines roots as being the regions.

acl-cqc · 2025-12-22T11:22:38Z

hugr-passes/src/composable/scope.rs

+/// - `roots` is a set of **regions** in the HUGR that the pass should be
+///   applied to.
+///   - The pass **MUST NOT** modify the container nodes of the regions defined
+///     in `roots`. This includes the optype and ports of the node.


Agreed on ports. I'm reasonably happy with optype, but there would seem to be cases where we can turn a CFG or TailLoop into a DFG, say, without affecting anything outside it. (Code expecting to manipulate the internals will be confused, of course.) Do we want to rule those out?

I.e. is there some use case where it's particularly good to? My intuition is that the root node optype (as long as dataflow) is inside the subtree/region defined by the root node, whereas it's ports are not inside (they are the boundary)

acl-cqc · 2025-12-22T11:44:06Z

hugr-passes/src/composable/scope.rs

+    ) -> impl Iterator<Item = H::Node> {
+        match self {
+            Self::EntrypointFlat | Self::EntrypointRecursive => {
+                // Include the entrypoint if it is not the module root.


We want to include like this for All and AllPublic too, no?

acl-cqc · 2025-12-22T11:44:29Z

hugr-passes/src/composable/scope.rs

+                Either::Left(entrypoint)
+            }
+            Self::All | Self::AllPublic => {
+                // Decide which public functions to include.


Suggested change

// Decide which public functions to include.

// Decide which functions to include.

acl-cqc · 2025-12-22T11:45:03Z

hugr-passes/src/composable/scope.rs

+                    let Some(fn_op) = hugr.get_optype(node).as_func_defn() else {
+                        return false;
+                    };
+                    let public = fn_op.visibility() == &Visibility::Public;


nit: either call this is_public or just inline at its single point of use

acl-cqc · 2025-12-22T11:49:07Z

hugr-passes/src/composable/scope.rs

+                if node_parent != hugr.module_root() {
+                    return true;
+                }
+                // For module children, only private functions


Maybe "Additionally allow removing/modifying private functions" ?

acl-cqc · 2025-12-22T11:51:15Z

hugr-passes/src/composable/scope.rs

+/// - `scope` is a set of **nodes** in the HUGR that the pass **MAY** modify.
+///   - This set is closed under descendants, meaning that all the descendants
+///     of a node in `scope` are also in scope.
+///   - Nodes that are not in `scope` **MUST** remain unchanged.


both in terms of their signature/optype and their behaviour...or clarify scope is the nodes which may be modified both as ops and as the behaviour defined by their descendants

acl-cqc · 2025-12-22T12:45:39Z

hugr-passes/src/composable/scope.rs

+    ///
+    /// Nodes in `root` are never in scope, but their children will be.
+    ///
+    /// If [PassScope::recursive] is `true`, then all descendants of nodes in


This suggests that if recursive is false, then descendants of nodes in root are not in scope, i.e. MUST NOT be modified? (EDIT: Yes, that is consistent with elsewhere, so ok.)

acl-cqc

Hmmm. IIUC, you can't modify anything outside of roots. So if you run with AllPublic you can't modify the private functions?? EDIT sorry yes, the private functions are still in_scope even though not in roots.

EDIT2 So roots is really more about goals/targets than scope. How about renames...
scope -> may_modify and/or may_modify_children? (/may_modify_descendants - perhaps one might even make that false if the thing contains the entrypoint, but true for all non-entrypoint descendants, although "modify_children" might mean, "ok to add children")
roots -> target_roots

acl-cqc · 2026-01-19T18:06:21Z

Urgh. Tried to use this for RemoveDeadFuncsPass....ok, that is an awkward one, maybe should have tried something simpler.

Only PassScope::AllPublic allows removing any dead functions; All, EntrypointFlat and EntrypointRecursive (regardless of the actual entrypoint) prevent DFE from doing anything.
It'd be good to have a mode where scope is "the whole hugr, except the entrypoint node" (e.g. if we are optimizing an executable and the entrypoint is where we'll start running). That could be AllEntrypoint. (Or you could say All should be called GlobalKeepAll, AllPublic is GlobalKeepPublic and this would be GlobalKeepEntrypoint.) Maybe a different approach would be to have two fields: a single "root" (can touch everything from there down) and a set of "preserve" (so "scope" can be computed as all descendants of "root" minus "preserve", and "regions" from "root" and the "recursive" hint) ?
Minor but super-annoying, I tried:

RemoveDeadFuncsPass::default()
            .with_scope(&scope)
            .run(&mut hugr)
            .unwrap();

and got this error:

error[E0283]: type annotations needed
   --> hugr-passes/src/dead_funcs.rs:275:14
    |
275 |             .with_scope(&scope)
    |              ^^^^^^^^^^
    |
    = note: cannot satisfy `_: hugr_core::hugr::hugrmut::HugrMut`
    = help: the following types implement trait `hugr_core::hugr::hugrmut::HugrMut`:
              &mut T
              Cow<'_, T>
              Rerooted<H>
              hugr_core::Hugr
              std::boxed::Box<T>
note: required by a bound in `composable::ComposablePass::with_scope`
   --> hugr-passes/src/composable.rs:15:29
    |
15  | pub trait ComposablePass<H: HugrMut>: Sized {
    |                             ^^^^^^^ required by this bound in `ComposablePass::with_scope`
...
34  |     fn with_scope(self, scope: &PassScope) -> Self {
    |        ---------- required by a bound in this associated function
help: try using a fully qualified path to specify the expected types
    |
274 -         RemoveDeadFuncsPass::default()
274 +         <RemoveDeadFuncsPass as composable::ComposablePass<H>>::with_scope(RemoveDeadFuncsPass::default(), &scope)
    |

which sounds a bit like it isn't using the call to run to determine the type of the receiver of with_scope....(which does make sense; that the argument to run does completely determine the receiver-type of with_scope is only because of our self -> Self method).

You can see this in 6ed9ee6

acl-cqc · 2026-01-21T12:22:27Z

Ok how about:

enum PassScope {
    /// Pass runs on the entrypoint region only (may not modify outside).
    /// Rerunning the pass with the entrypoint set to a descendant of the current entrypoint,
    /// may do further optimization.
    EntrypointFlat,
    /// Pass runs on the entrypoint region only (may not modify outside).
    /// Rerunning the pass with the entrypoint set to a descendant of the current entrypoint,
    /// must have no effect (needs to have been done already).
    EntrypointRecursive,
    /// Pass may mutate the whole Hugr, but must preserve semantics of all functions (private+public)
    /// ?? TODO ?? ignores entrypoint?? or entrypoint as well?
    PreserveAll,
    /// Pass may mutate the whole Hugr, but must preserve semantics of public functions (?? TODO ?? +entrypoint??)
    PreservePublic,
    /// Pass may mutate the whole Hugr, but must preserve semantics of entrypoint (only)
    PreserveEntrypoint,
}

acl-cqc · 2026-01-21T20:37:55Z

Ok how about: ...

I note this is only really one enum variant more than we have now, i.e. very similar! But I think this allows to summarize:

EntrypointFlat/Recursive do nothing if the entrypoint is the module-root (rather than acting, everywhere). This seems the only reasonable interpretation for EntrypointFlat; for EntrypointRecursive, we could take it is being, the whole Hugr, but then we would have to decide what to preserve. (Perhaps PreserveAll would be reasonable?)
Question as to whether PreserveAll/PreservePublic (All/AllPublic) also preserve the entrypoint. I guess they should; if there is no entrypoint that you want to preserve, set the entrypoint to the module root (or a funcdefn that's being preserved anyway), right?
roots. Currently this is all functions for (Preserve)All but public functions only for (All/Preserve)Public. I think the latter can also include all functions - we can optimize the private ones (like PreserveAll), but do not even have to preserve the behaviour of the private ones - the requirement for (All/Preserve)Public is just that we have to preserve the public funcdefn nodes themselves. (Similarly for proposed PreserveEntrypoint.) That means roots is:
- just the entrypoint (for EntrypointFlat/EntrypointRecursive)
- all the funcdefns (for everything else)
  ....which leads me to think that we should just have a single root(&self) -> Node being either the entrypoint or the module-root.
regions is then either just the root, for EntrypointFlat; or all FuncDefns/Dataflow-containers/maybe-CFG-containers-too that are nonstrict descendants of the root, for any other PassScope)
in_scope are the nodes beneath the root and not to be preserved (a small list of exceptions)....
- However I'm pro being able to turn CFG->DFG, even for the entrypoint (which is preserved): OpType seems internal/implementation, whereas ports and behaviour/execution semantics, are the interface; it seems fair game to mutate the implementation but preserve the interface. (Obvs transforming BasicBlock<->DFG<->FuncDefn is not allowed! I think the ports do a good job here: BasicBlock and FuncDefn have different ports to any Dataflow node, right?)
- So it might be better to invert in_scope to preserve_interface (listing only nodes beneath the root)??? Not quite sure here.
- Much as you have it, for (Preserve)All, both public and private funcdefns/decls are out of scope / to-be-preserved; for (Preserve/All)Public, public funcdefns/decls are out of scope, but private ones are in scope (which includes deletion)

acl-cqc · 2026-01-22T19:44:49Z

/// Pass may mutate the whole Hugr, but must preserve semantics of entrypoint (only)
PreserveEntrypoint,

If the entrypoint is the module-root, there are a few possibilities for "preserve semantics of only the entrypoint"....

Let's not say, delete everything. I mean, that does seem the literal interpretation, but it's not good....
Do nothing? (i.e. nothing in scope)
Same as PreserveAll?

aborgna-q added 3 commits December 19, 2025 16:38

feat: Define pass application scopes

1f05f19

Stub implementation for all passes

aba6477

Python definition for PassScope

3e0c6ac

aborgna-q requested a review from acl-cqc December 19, 2025 17:19

aborgna-q requested a review from a team as a code owner December 19, 2025 17:19

aborgna-q changed the title ~~feat: Define pass application scopes~~ feat!: Define pass application scopes Dec 19, 2025

aborgna-q mentioned this pull request Dec 19, 2025

Migrate passes to use the new PassScope #2771

Open

10 tasks

acl-cqc reviewed Dec 22, 2025

View reviewed changes

Add default with_scope impl, make this non-breaking

02d2886

aborgna-q changed the title ~~feat!: Define pass application scopes~~ feat: Define pass application scopes Dec 22, 2025

Move scope to a submodule of ::composable

e3a63b4

acl-cqc added 3 commits January 23, 2026 13:34

wip

0b730bf

All=>Preserve (make global), add PreserveEntrypoint, enum InScope

e71125c

docs, re-export InScope

11a643e

acl-cqc mentioned this pull request Jan 23, 2026

WIP: update passes to use PassScope #2836

Draft

	// Decide which public functions to include.
	// Decide which functions to include.

feat: Define pass application scopes #2772

Are you sure you want to change the base?

feat: Define pass application scopes #2772

Uh oh!

Conversation

aborgna-q commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

acl-cqc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

acl-cqc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

acl-cqc commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

acl-cqc commented Jan 21, 2026

Uh oh!

acl-cqc commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

acl-cqc commented Jan 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aborgna-q commented Dec 19, 2025 •

edited

Loading

codecov bot commented Dec 19, 2025 •

edited

Loading

acl-cqc left a comment •

edited

Loading

acl-cqc Dec 22, 2025 •

edited

Loading

acl-cqc left a comment •

edited

Loading

acl-cqc commented Jan 19, 2026 •

edited

Loading

acl-cqc commented Jan 21, 2026 •

edited

Loading