fix: Loading custom polymorphic function defs as values #260
Conversation
| # Find the module node by walking up the hierarchy | ||
| module: Node = dfg.node |
There was a problem hiding this comment.
We can put the FunctionDef anywhere, no need to walk up to the module
|
|
||
| # Finally, load the function into the local DFG | ||
| return graph.add_load_constant(def_node.out_port(0), dfg.node).out_port(0) | ||
| return graph.add_load_function(def_node.out_port(0), [], dfg.node).out_port(0) |
There was a problem hiding this comment.
We already monomorphised, so we can load with empty type args
There was a problem hiding this comment.
I think this comment should be included! Maybe lower the ty = ... line below you're We create a .. comment, and add "instantiate the monomorphised type of the function" to that comment
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #260 +/- ##
=======================================
Coverage ? 91.05%
=======================================
Files ? 44
Lines ? 4973
Branches ? 0
=======================================
Hits ? 4528
Misses ? 445
Partials ? 0 ☔ View full report in Codecov by Sentry. |
mark-koch
force-pushed
the
fix/higher-order-poly
branch
from
9126527
to
8b4ad23
Compare
guppylang/definition/custom.py
Outdated
| "Encountered node that is not contained in a module." | ||
| ) | ||
| module = module.parent | ||
| ty = self.ty.instantiate(type_args) |
There was a problem hiding this comment.
| ty = self.ty.instantiate(type_args) | |
| func_ty = self.ty.instantiate(type_args) |
There was a problem hiding this comment.
There are lots of types flying around, I think renaming this would make it easier to understand.
|
|
||
| # Finally, load the function into the local DFG | ||
| return graph.add_load_constant(def_node.out_port(0), dfg.node).out_port(0) | ||
| return graph.add_load_function(def_node.out_port(0), [], dfg.node).out_port(0) |
There was a problem hiding this comment.
I think this comment should be included! Maybe lower the ty = ... line below you're We create a .. comment, and add "instantiate the monomorphised type of the function" to that comment
| module = GuppyModule("test") | ||
| T = guppy.type_var(module, "T") | ||
|
|
||
| @guppy.custom(module, CustomCompiler()) |
There was a problem hiding this comment.
Are users expected to write a CustomCompiler? Needs much better docs and examples, and probably more helper functions. Happy to approve without, but recommend you add an issue to flesh this out if you haven't already.
There was a problem hiding this comment.
At the moment custom compilers are an escape hatch to build Hugr stuff that can't be directly expressed in Guppy. Not sure how much we want to expose that to users in the future?
I created issue #269 for discussion
🤖 I have created a release *beep* *boop* --- ## [0.6.0](v0.5.2...v0.6.0) (2024-07-02) ### Features * Add array type ([#258](#258)) ([041c621](041c621)) * Add nat type ([#254](#254)) ([a461a9d](a461a9d)) * Add result function ([#271](#271)) ([792fb87](792fb87)), closes [#270](#270) * Allow constant nats as type args ([#255](#255)) ([d706735](d706735)) * Generate constructor methods for structs ([#262](#262)) ([f68d0af](f68d0af)), closes [#261](#261) * Return already-compiled hugrs from `GuppyModule.compile` ([#247](#247)) ([9d01eae](9d01eae)) * Turn int and float into core types ([#225](#225)) ([99217dc](99217dc)) ### Bug Fixes * Add missing test file ([#266](#266)) ([75231fe](75231fe)) * Loading custom polymorphic function defs as values ([#260](#260)) ([d15b2f5](d15b2f5)), closes [#259](#259) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Fixes #259