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

Instantiation fails for ArrayOps with non-prelude element type #1774

Closed
aborgna-q opened this issue Dec 12, 2024 · 0 comments · Fixed by #1784
Closed

Instantiation fails for ArrayOps with non-prelude element type #1774

aborgna-q opened this issue Dec 12, 2024 · 0 comments · Fixed by #1784
Assignees
Labels
bug Something isn't working

Comments

@aborgna-q
Copy link
Collaborator

Failing test case, similar to array_op::tests::test_get.

    #[test]
    /// Initialize an array operation where the element type is not from the prelude.
    fn test_non_prelude_op() {
        let size = 2;
        let element_ty = float64_type();
        let op = ArrayOpDef::get.to_concrete(element_ty.clone(), size);

        let optype: OpType = op.into();

        let sig = optype.dataflow_signature().unwrap();

        assert_eq!(
            sig.io(),
            (
                &vec![array_type(size, element_ty.clone()), usize_t()].into(),
                &vec![option_type(element_ty.clone()).into()].into()
            )
        );
    }

This should get fixed if we remove the extension registries from validation.

@aborgna-q aborgna-q added the bug Something isn't working label Dec 12, 2024
@aborgna-q aborgna-q self-assigned this Dec 12, 2024
@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 16, 2024
Now that we keep references to the extensions used around the hugr, we
no longer need to pass `ExtensionRegistry`es when calling `validate` on
ops, types, etc. This PR removes most uses of the registries.

drive-by: Fix `Noop` not declaring its type argument in `hugr-py`.

Closes #1774; now that the type carries its extension we can instantiate
`Array`s with any element type.

BREAKING CHANGE: Removed the extension registry argument from `validate`
calls.
BREAKING CHANGE: Removed the extension registry argument from operation
instantiation methods.
BREAKING CHANGE: Removed most extension-specific test registries. Use
`EMPTY_REG`, `PRELUDE_REGISTRY`, or `STD_REG` instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant