Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpaqueValue should keep a Weak<Extension> pointer #1742

Closed
aborgna-q opened this issue Dec 4, 2024 · 1 comment · Fixed by #1780
Closed

OpaqueValue should keep a Weak<Extension> pointer #1742

aborgna-q opened this issue Dec 4, 2024 · 1 comment · Fixed by #1780

Comments

@aborgna-q
Copy link
Collaborator

Similarly to what we did with ExtensionOps and CustomTypes, OpaqueValue/CustomConst should keep a weak pointer to their extension and provide a used_extensions call.

@doug-q
Copy link
Collaborator

doug-q commented Dec 12, 2024

This issue buries the lede, CustomConsts are not currently registered via Extensions. Surely that must happen first for this to be meaningful.

See #1225, and also #1224

@aborgna-q aborgna-q added this to the hugr-rs 0.14 / hugr-py 0.10 milestone Dec 12, 2024
github-merge-queue bot pushed a commit that referenced this issue Dec 12, 2024
Collects and resolves extensions in the types stored inside a `Value`.
This includes the specified type of a `Sum` and other cached types.

Since we expect `CustomConst`'s `get_type` method to always return
signatures computed by a binary definition, I'd say we close #1742 as
not needed.

For some reason there's a random test on `hugr-py` that starts failing
when we enable this:
```
=================================== FAILURES ===================================
______________________________ test_higher_order _______________________________
Error parsing package: Error resolving opaque operation: Error in signature of operation 'prelude.Noop' in Node(3): Type arguments of node did not match params declared by definition: Wrong number of type arguments: 0 vs expected 1 declared type parameters
```

The fix for #1774 that I'm submitting immediately after this PR fixes it
so I'm skipping the test in this PR 🤷
I'll open an issue to investigate further after we make the release

---------

Co-authored-by: Seyon Sivarajah <[email protected]>
github-merge-queue bot pushed a commit that referenced this issue Dec 12, 2024
Closes #1742.

Adds a (default defined) `update_extensions` call to `CustomConst` so it
can update its internal extension `Weak` pointers after loading a
serialized hugr.
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 a pull request may close this issue.

2 participants