-
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
refactor: remove MutableIO #1866
base: 1819-dfbs-need-add_inoutput-like-the-functionbuilder
Are you sure you want to change the base?
refactor: remove MutableIO #1866
Conversation
This reverts commit bdc709b.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 1819-dfbs-need-add_inoutput-like-the-functionbuilder #1866 +/- ##
=======================================================================================
Coverage ? 86.58%
=======================================================================================
Files ? 194
Lines ? 35249
Branches ? 32062
=======================================================================================
Hits ? 30521
Misses ? 2965
Partials ? 1763
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -331,6 +352,46 @@ impl<T> HugrBuilder for DFGWrapper<Hugr, T> { | |||
} | |||
} | |||
|
|||
/// TODO perhaps these should be on HugrMut? |
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.
Yes, it should probably go there.
I'd add a single function though, parametric on the direction. (And add specializations that cast the output type if needed)
Also, docs :)
// | ||
// Replaces the DFG under construction with new_optype and mutates the | ||
// Output node by appending `output_type``. | ||
pub(crate) fn add_output_impl(&mut self, new_optype: impl Into<OpType>, output_type: Type) { |
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.
This is the same as add_input_impl
bar the returned wire, which can be computed in add_input
directly.
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.
It's not the same. They look at output
/input
respectively in the signature, and they call different "insert port" functions. You want the insert port functions to be combined, after which combining these makes more sense. Nice, will do.
No description provided.