Skip to content

Conversation

acl-cqc
Copy link
Contributor

@acl-cqc acl-cqc commented Aug 26, 2025

In the short term, this is likely wanted for an upcoming PR on linking.
In the (only-slightly-)longer term, we're likely to want this to include static function type-arguments, where a Call/LoadFunction can refer to multiple functions (e.g. foo<bar>(runtime_args)), which will probably mean adding variants to enum CallGraphEdge. So....

  • There is a possibility I could derive #[non_exhaustive] enum CallGraphEdge now, making that (anticipated, hypothetical) future change less breaking. (Probably still breaking, though!)
  • This has the cost of making this change breaking now, which it presently is not (because of deprecated re-export in hugr-passes - I think rust-semver checks is wrong here).
  • However, rather than re-export, I could duplicate call_graph.rs, with the hugr-core version having non_exhaustive but hugr-passes not.
  • I'm not sure this is worth it: static function type-args are likely to be breaking anyway, and the non-exhaustive just delays the point of user-code needing an update until, most likely, it sees a new instance at runtime rather than compile-time....
  • Thoughts please?

Copy link

codecov bot commented Aug 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.68%. Comparing base (daf6cfe) to head (4660ac4).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2531      +/-   ##
==========================================
+ Coverage   82.66%   82.68%   +0.01%     
==========================================
  Files         252      252              
  Lines       46781    46927     +146     
  Branches    42297    42443     +146     
==========================================
+ Hits        38673    38800     +127     
- Misses       6058     6066       +8     
- Partials     2050     2061      +11     
Flag Coverage Δ
python 91.41% <ø> (ø)
rust 81.75% <ø> (+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.

@hugrbot
Copy link
Collaborator

hugrbot commented Aug 26, 2025

This PR contains breaking changes to the public Rust API.
Please deprecate the old API instead (if possible), or mark the PR with a ! to indicate a breaking change.

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.43.0/src/lints/enum_missing.ron

Failed in:
enum hugr_passes::call_graph::CallGraphNode, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-passes/src/call_graph.rs:17
enum hugr_passes::call_graph::CallGraphEdge, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-passes/src/call_graph.rs:9

--- failure module_missing: pub module removed or renamed ---

Description:
A publicly-visible module cannot be imported by its prior path. A `pub use` may have been removed, or the module may have been renamed, removed, or made non-public.
      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/module_missing.ron

Failed in:
mod hugr_passes::call_graph, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-passes/src/call_graph.rs:1

--- 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_passes::call_graph::CallGraph, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-passes/src/call_graph.rs:42

//! Compilation passes acting on the HUGR program representation.
pub mod call_graph;
#[deprecated(note = "Re-exported from hugr-core", since = "0.22.3")]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The since here commits me to making a patch release, ho hum....

Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine, if 0.22.3 never happens, people will be smart enough to infer this means 0.23 :)

@acl-cqc acl-cqc marked this pull request as ready for review August 26, 2025 15:17
@acl-cqc acl-cqc requested a review from a team as a code owner August 26, 2025 15:17
@acl-cqc acl-cqc requested a review from lmondada August 26, 2025 15:17
Copy link
Contributor

@lmondada lmondada left a comment

Choose a reason for hiding this comment

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

thanks Alan!

//! Compilation passes acting on the HUGR program representation.
pub mod call_graph;
#[deprecated(note = "Re-exported from hugr-core", since = "0.22.3")]
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine, if 0.22.3 never happens, people will be smart enough to infer this means 0.23 :)

@lmondada
Copy link
Contributor

lmondada commented Aug 26, 2025

I'm confused - why does semver complain on this change?

EDIT: this is a known issue -> obi1kenobi/cargo-semver-checks#355. We can ignore and override it.

@acl-cqc
Copy link
Contributor Author

acl-cqc commented Sep 16, 2025

This is following something of a different path in #2555 (I had to include constants!) so I'm gonna close this, and then do it again with hugr-core "StaticGraph" (or some name like that) and add the #[non_exhaustive]s for good measure...

@acl-cqc acl-cqc closed this Sep 16, 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