Repository navigation
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3282 +/- ##
==========================================
+ Coverage 82.05% 82.17% +0.11%
==========================================
Files 245 245
Lines 47553 47847 +294
Branches 41144 41438 +294
==========================================
+ Hits 39020 39316 +296
+ Misses 6629 6620 -9
- Partials 1904 1911 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
This PR contains breaking changes to the public Rust API. cargo-semver-checks summary |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The ownership contract, generated declarations, builders, documentation, and tests are internally consistent.
Review effort: Balanced
Findings: None
What changed in this PR
Makes ptr<T> a linear handle with explicit ownership and support for linear stored values.
Changes:
- Adds
Swap,Dup,Free, and row-polymorphicMap. - Updates Rust builders, signatures, ownership documentation, and tests.
- Regenerates Rust/Python declarations as
ptr0.2.0.
| File | Description |
|---|---|
specification/stdlib.md |
Documents pointer ownership and operation semantics. |
resources/std_extensions/ptr.json |
Defines the 0.2.0 extension schema. |
hugr-py/tests/test_ptr.py |
Tests linear bounds and operation availability. |
hugr-py/src/hugr/std/ptr.py |
Updates pointer documentation. |
hugr-py/src/hugr/std/_json_defs/ptr.json |
Updates Python’s bundled extension definition. |
hugr-core/src/std_extensions/ptr.rs |
Implements operations, builders, signatures, and tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| //! passing the pointer returned by one operation into the next makes the next | ||
| //! operation wait for the first to finish. After [`PtrOpDef::Dup`], operations on | ||
| //! the separate handles have unspecified order unless another dependency orders | ||
| //! them. Sharing a cell does not itself establish an execution order. |
There was a problem hiding this comment.
Is this last sentence just repeating the previous sentence or is it saying something different?
There was a problem hiding this comment.
It's just repeating the previous sentance. for "clarity". Better to remove?
There was a problem hiding this comment.
Yes, I think it's better to remove, to keep the docs succinct.
| //! Pointer handles are threaded through operations to establish their order: | ||
| //! passing the pointer returned by one operation into the next makes the next | ||
| //! operation wait for the first to finish. After [`PtrOpDef::Dup`], operations on |
There was a problem hiding this comment.
Is this different from the sematics of value edges in general, i.e. are we putting stricter requirements on the ordering of operations involving Ptr<T> than we are on other operations? If so, how will we enforce this?
I remember some discussion with @acl-cqc about whether operations with child graphs were always atomic, but I don't remember where we landed.
There was a problem hiding this comment.
No it's not different from value edges in general. This just states the hugr semantics specialised to this extension.
There was a problem hiding this comment.
Again I would say remove this explanation then, since it is just general hugr semantics.
Make
ptr<T>a linear handle to shared mutable storage, including storage of linear values.ReadandWriterequire copyable values and return the pointer;Swap,Dup,Free, and row-polymorphicMappreserve explicit ownership. The lastFreereturns the stored value.Add
Eq<T>: (ptr<T>, ptr<T>) -> (ptr<T>, ptr<T>, bool). It compares cell identity, returns both linear handles in input order, and neither reads nor compares the stored values. Both inputs have the same element type, which may be linear.Provide concrete Rust operations and builders for all eight operations, document ownership and pointer dependency ordering, and regenerate both Rust/Python declaration copies as
ptr0.2.0. Runtime implementations must implement the new operation contract.Latest local validation: 718 core unit tests passed (4 ignored), including signatures, extension round trips, mixed-type rejection, and rejection of copied/dropped Eq outputs; 4 Python pointer tests passed; all-target core Clippy, all-feature core rustdoc with warnings denied, workspace formatting, locked Ruff 0.6.9, and extension-version checks passed. The pinned Nix shell and lockfiles were unchanged. Remote CI supplies the broader cross-language and semver checks.
BREAKING CHANGE:
ptr<T>is now linear. Thread the pointer returned byRead/Write, useDupfor shared handles, and release every handle withFree, handling its optional value. Rust callers must consume the updated builder results and construct operations withPtrOpDef::with_typeorPtrOp::mapinstead of the old two-fieldPtrOpliteral. Runtime implementations must support theptr0.2.0 signatures and operations.