feat: insert_link_hugr adds entrypoint subtree and links, with reachability
#2555
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Remove unused fn directive
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## acl/link_hugr #2555 +/- ##
=================================================
+ Coverage 83.61% 83.63% +0.01%
=================================================
Files 266 266
Lines 51951 52054 +103
Branches 47370 47520 +150
=================================================
+ Hits 43441 43533 +92
- Misses 6133 6140 +7
- Partials 2377 2381 +4
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:
|
acl-cqc
commented
Sep 16, 2025
acl-cqc
changed the title
insert_link_hugr to add entrypoint subtree AND linkinsert_link_hugr to add entrypoint subtree AND link
acl-cqc
commented
Nov 17, 2025
| /// [`OnMultiDefn::NewFunc`] of [`OnNewFunc::RaiseError`] | ||
| /// | ||
| /// [`FuncDefn`]: crate::ops::FuncDefn | ||
| /// [`on_new_names`]: NameLinkingPolicy::get_on_new_names |
Contributor
Author
There was a problem hiding this comment.
I guess I should either change the links to be called `get_on_new_names` or write `on_new_names` has been set to ...
acl-cqc
commented
Nov 17, 2025
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 18, 2025
Respin of #2531, marking the node/edge enums in the new StaticGraph as `non-exhaustive` and adding Consts + LoadConstants. (Lukas originally planned to make something like this to reflect references from Terms to functions, and non-exhaustive allows extending this StaticGraph for that purpose - if we do go that way, however, I have a different plan, can describe if relevant. Rather I think non-exhaustive is just a reasonable allowance for future extension to new use-cases hopefully without damaging existing use-cases.) This is taken from #2555, i.e. which uses it (although there it is still called CallGraph in hugr-core). Naming: could be ModuleGraph ?? Other ideas welcome. I will add a test with some Consts + use of `out_edges` (tho will be supplied in #2555 !)
acl-cqc
changed the title
insert_link_hugr to add entrypoint subtree AND linkinsert_link_hugr adds entrypoint subtree and links, with reachability
acl-cqc
force-pushed
the
acl/insert_link_callgraph
branch
from
4ce1890 to
46b988c
Compare
acl-cqc
force-pushed
the
acl/insert_link_callgraph
branch
from
46b988c to
d0de271
Compare
ss2165
reviewed
Nov 28, 2025
| /// | ||
| /// # Errors | ||
| /// | ||
| /// If other's entrypoint is its module-root (recommend using [Self::link_module] instead) |
Member
There was a problem hiding this comment.
should this line be above # Errors ?
| /// If other's entrypoint is its module-root (recommend using [Self::link_module] instead) | ||
| /// | ||
| /// If other's entrypoint calls (perhaps transitively) the function containing said entrypoint | ||
| /// (an exception is made if the called+containing function is public and is being replaced |
Member
There was a problem hiding this comment.
nit: remove parentheses for readability
| /// | ||
| /// If other's entrypoint calls a public [`FuncDefn`] in `other` which has the same name | ||
| /// and signature as a public [`FuncDefn`] in `self` and [`on_multi_defn`] is | ||
| /// [`OnMultiDefn::NewFunc`] of [`OnNewFunc::RaiseError`] |
Member
There was a problem hiding this comment.
Suggested change
| /// [`OnMultiDefn::NewFunc`] of [`OnNewFunc::RaiseError`] | |
| /// [`OnMultiDefn::NewFunc`] or [`OnNewFunc::RaiseError`] |
hugr-core/src/hugr/linking.rs
Outdated
| /// [`on_new_names`]: NameLinkingPolicy::get_on_new_names | ||
| /// [`on_multi_defn`]: NameLinkingPolicy::get_on_multiple_defn | ||
| /// [`on_sig_conflict`]: NameLinkingPolicy::get_signature_conflict | ||
| #[allow(clippy::type_complexity)] |
Member
There was a problem hiding this comment.
a where block might let you remove this?
Contributor
Author
There was a problem hiding this comment.
Not quite - error: equality constraints are not yet supported in where clauses rust-lang/rust#20041
Ah, but parametrizing by HugrNode type rather than HugrView type does! Done
Comment on lines
+1332
to
+1335
| let reached = mb.declare("foo", endo_sig(usize_t()).into()).unwrap(); | ||
| // This would conflict signature, but is not reached: | ||
| let unreached = mb | ||
| .declare("bar", inout_sig(vec![], usize_t()).into()) |
Member
There was a problem hiding this comment.
nit
Suggested change
| let reached = mb.declare("foo", endo_sig(usize_t()).into()).unwrap(); | |
| // This would conflict signature, but is not reached: | |
| let unreached = mb | |
| .declare("bar", inout_sig(vec![], usize_t()).into()) | |
| let reached = mb.declare("reached", endo_sig(usize_t()).into()).unwrap(); | |
| // This would conflict signature, but is not reached: | |
| let unreached = mb | |
| .declare("unreached", inout_sig(vec![], usize_t()).into()) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follows #2529, which is approved, but will merge both without any intervening release.
insert_link_hugrandinsert_link_viewtaking a node under which to add the entrypointinsert_link_xxxcloses #2517
Questions remain from previous PR about factory methods for NameLinkingPolicy and whether tests need to be more systematic about covering all the policy variants.