-
Notifications
You must be signed in to change notification settings - Fork 7
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
fix: Resolve types in Value
s and custom consts
#1779
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1779 +/- ##
==========================================
- Coverage 86.73% 86.69% -0.05%
==========================================
Files 186 186
Lines 33990 34035 +45
Branches 30865 30910 +45
==========================================
+ Hits 29481 29506 +25
- Misses 2840 2857 +17
- Partials 1669 1672 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the nice comments :)
OpType::Const(c) => { | ||
let typ = c.get_type(); | ||
collect_type_exts(&typ, &mut used, &mut missing); | ||
collect_value_exts(&c.value, &mut used, &mut missing); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: looks like the braces are no longer necessary
Co-authored-by: Seyon Sivarajah <[email protected]>
Value
s and custom contsValue
s and custom consts
Collects and resolves extensions in the types stored inside a
Value
. This includes the specified type of aSum
and other cached types.Since we expect
CustomConst
'sget_type
method to always return signatures computed by a binary definition.For some reason there's a random test on
hugr-py
that starts failing when we enable this: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