-
-
Notifications
You must be signed in to change notification settings - Fork 319
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
WIP discussion of function values to resources #716
base: master
Are you sure you want to change the base?
Conversation
Hey @gelisam I opened this PR (it's based of #704) to discuss passing function values to resources. In playing around first I hit an expected bug in the Into() function. This function pushes a bunch of mcl values INTO a golang struct (the resource). Of course the function part wasn't implemented. So I started moving things around. And I have the import cycle of Txn <-> Types since I thought FancyFunc should maybe be part of the types package. Although it's a bit different because it has the unusual Txn aspect in there. Before proceeding with a more complex refactor, I figured I'd check with you if you think this all makes sense or if you had a thought about how instead we'd pass in a simpler FuncValue to resources instead? (I don't expect they'll need the Txn, that's not the kind of functions we'd be passing to the resource I don't think.) Cheers! |
Notes from today's meeting:
|
I propose a new static analysis phase after type checking and before |
Since you opened this PR, I guess this time I'll be the one pushing to a auxiliary branch and you'll be the one updating the official MR branch to incorporate my changes! I have pushed to |
Could you post the examples and counter-examples of what counts as timeless, both as a reminder of what the goal is and so we can use them as tests? |
I'll fix and throw away this branch. But if you'd prefer to have the PR branch, lmk.
In short: timeless:
timeful:
|
Here are the notes I took (may contain errors)
|
99e204e
to
ab7eb0a
Compare
@gelisam Just wanted to double-check if this patch with interface and one implementation is basically what you were expecting? If so, LMK and I can complete it for other functions too. |
Looks good, yes! |
Next steps:
|
|
fe2313c
to
271a94e
Compare
64fd35b
to
73dfec6
Compare
Not the priority, but I was playing with this code and implemented a few extra for fun. I also pushed in your commits to have them here. An interesting realization I had is that we should eventually once all have been implemented, be able to remove each mostly identical custom Stream() implementation with a single one that is common to all functions. Or most functions! |
Add a new interface for callable functions.
Add an initial implementation for the `if` function.
Add an initial implementation for the `const` function.
Add initial implementations for these wrapper functions.
73dfec6
to
3eb3a96
Compare
Add an initial implementation for this wrapper function.
3eb3a96
to
d401577
Compare
b8072b2
to
380004b
Compare
No description provided.