Skip to content

Conversation

@aborgna-q
Copy link
Collaborator

Closes #2077

BREAKING CHANGE: Removed RootTagged trait. Now RootChecked is a non-hugrview wrapper only used to verify inputs where appropriate.

@aborgna-q aborgna-q requested a review from a team as a code owner April 28, 2025 12:00
@aborgna-q aborgna-q requested a review from doug-q April 28, 2025 12:00
@codecov
Copy link

codecov bot commented Apr 28, 2025

Codecov Report

Attention: Patch coverage is 86.48649% with 15 lines in your changes missing coverage. Please review.

Project coverage is 83.44%. Comparing base (a1cd051) to head (60d97cf).
Report is 2 commits behind head on release-rs-v0.16.0.

Files with missing lines Patch % Lines
hugr-core/src/hugr/views/sibling.rs 0.00% 5 Missing ⚠️
hugr-core/src/hugr/views/root_checked.rs 91.42% 2 Missing and 1 partial ⚠️
hugr-core/src/hugr/views/sibling_subgraph.rs 82.35% 1 Missing and 2 partials ⚠️
hugr-core/src/hugr/rewrite.rs 0.00% 1 Missing ⚠️
hugr-core/src/hugr/views/descendants.rs 0.00% 0 Missing and 1 partial ⚠️
hugr-core/src/hugr/views/impls.rs 0.00% 1 Missing ⚠️
hugr-llvm/src/utils/inline_constant_functions.rs 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@                  Coverage Diff                   @@
##           release-rs-v0.16.0    #2122      +/-   ##
======================================================
- Coverage               83.49%   83.44%   -0.05%     
======================================================
  Files                     219      219              
  Lines                   42188    42149      -39     
  Branches                38290    38251      -39     
======================================================
- Hits                    35225    35172      -53     
- Misses                   5150     5163      +13     
- Partials                 1813     1814       +1     
Flag Coverage Δ
python 85.73% <ø> (ø)
rust 83.21% <86.48%> (-0.06%) ⬇️

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 Apr 28, 2025

This PR contains breaking changes to the public Rust API.

cargo-semver-checks summary

--- failure trait_missing: pub trait removed or renamed ---

Description:
A publicly-visible trait cannot be imported by its prior path. A `pub use` may have been removed, or the trait 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.41.0/src/lints/trait_missing.ron

Failed in:
trait hugr_core::hugr::views::RootTagged, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/views.rs:483
trait hugr_core::hugr::RootTagged, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/hugr/views.rs:483

--- failure trait_removed_supertrait: supertrait removed or renamed ---

Description:
A supertrait was removed from a trait. Users of the trait can no longer assume it can also be used like its supertrait.
      ref: https://doc.rust-lang.org/reference/items/traits.html#supertraits
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.41.0/src/lints/trait_removed_supertrait.ron

Failed in:
supertrait hugr_core::hugr::views::RootTagged of trait HugrMutInternals in file /home/runner/work/hugr/hugr/PR_BRANCH/hugr-core/src/hugr/internal.rs:110
supertrait hugr_core::hugr::RootTagged of trait HugrMutInternals in file /home/runner/work/hugr/hugr/PR_BRANCH/hugr-core/src/hugr/internal.rs:110
supertrait hugr_core::hugr::views::RootTagged of trait HierarchyView in file /home/runner/work/hugr/hugr/PR_BRANCH/hugr-core/src/hugr/views.rs:482
supertrait hugr_core::hugr::RootTagged of trait HierarchyView in file /home/runner/work/hugr/hugr/PR_BRANCH/hugr-core/src/hugr/views.rs:482

--- failure function_requires_different_generic_type_params: function now requires a different number of generic type parameters ---

Description:
A function now requires a different number of generic type parameters than it used to. Uses of this function that supplied the previous number of generic types (e.g. via turbofish syntax) will be broken.
      ref: https://doc.rust-lang.org/reference/items/generics.html
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.41.0/src/lints/function_requires_different_generic_type_params.ron

Failed in:
function merge_basic_blocks (0 -> 1 generic types) in /home/runner/work/hugr/hugr/PR_BRANCH/hugr-passes/src/merge_bbs.rs:19

Copy link
Collaborator

@doug-q doug-q left a comment

Choose a reason for hiding this comment

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

Thank you

index.into()
}

#[inline]
Copy link
Collaborator

Choose a reason for hiding this comment

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

why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This got filtered in from the big trait rework changes. I removed it from this patch

fn replace_op(&mut self, node: Node, op: impl Into<OpType>) -> Result<OpType, HugrError> {
fn replace_op(&mut self, node: Node, op: impl Into<OpType>) -> OpType {
panic_invalid_node(self, node);
// We know RootHandle=Node here so no need to check
Copy link
Collaborator

Choose a reason for hiding this comment

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

delete this line?

) -> Result<crate::ops::OpType, crate::hugr::HugrError> {
) -> crate::ops::OpType {
let op = op.into();
// Note: This wrapper will be removed in a subsequent PR, so we just panic here for now.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is "This wrapper" SiblingMut?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep!

"display",
"error",
"from",
"into",
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not clear what this is for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same here. Removed from this patch

@aborgna-q aborgna-q added this pull request to the merge queue Apr 28, 2025
Merged via the queue into release-rs-v0.16.0 with commit 6ca1258 Apr 28, 2025
25 checks passed
@aborgna-q aborgna-q deleted the ab/drop-root-tagged branch April 28, 2025 13:16
aborgna-q added a commit that referenced this pull request May 7, 2025
Closes #2077

BREAKING CHANGE: Removed `RootTagged` trait. Now `RootChecked` is a
non-hugrview wrapper only used to verify inputs where appropriate.
This was referenced May 7, 2025
@hugrbot hugrbot mentioned this pull request May 14, 2025
This was referenced May 29, 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.

4 participants