feat: ReplaceTypes: recursively replace on types too #2442
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## acl/replace_recursive #2442 +/- ##
=======================================================
Coverage 82.59% 82.59%
=======================================================
Files 247 246 -1
Lines 45988 45879 -109
Branches 41622 41615 -7
=======================================================
- Hits 37982 37893 -89
+ Misses 5987 5967 -20
Partials 2019 2019
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -171,8 +171,10 @@ fn call<H: HugrView<Node = Node>>( | |||
| Ok(Call::try_new(func_sig, type_args)?) | |||
| } | |||
|
|
|||
| /// Options for how the replacement for an op is processed. May be specified by | |||
| /// [ReplaceTypes::replace_op_with] and [ReplaceTypes::replace_parametrized_op_with]. | |||
| // TODO also replacement consts. | |||
There was a problem hiding this comment.
At this point I am groaning and wishing that maybe I hadn't gone this way.....but, it does continue the theme of, make the framework do the work - I am thinking to put in a breaking change next that changes ReplacementOptions::default() to turn on recursion, hence, normal users won't have to turn this on and it'll just happen.
Actually, I can do that by deprecating the non-options variant, and adding a new variant with a different ReplacementOptions....even in the same PR....
Follow-up to #2435 - as the test demonstrated, if we recursively replace within ops, we need to do types as well (if the op replacements change their input/output types), due to e.g. Input/Output.