-
Notifications
You must be signed in to change notification settings - Fork 13
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
[☯]: Inferring the names of procedures created by ☯. #73
base: main
Are you sure you want to change the base?
Conversation
qi-lib/flow.rkt
Outdated
[(_ onex) | ||
#:with name (syntax-local-name) | ||
(quasisyntax/loc stx | ||
(let ([flowed #,((compose compile-flow expand-flow) #'onex)]) |
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.
"flowed" suggests past tense and would be confusing as the name here. flow
on its own as the name may also be confusing since it could suggest that it is simply a function named "flow", as these two cases would have the same output in the REPL:
(define (flow x) x)
flow
and
(define-flow f (-< add1 add1))
f
How about using compiled-flow
as the name?
I wonder whether in the future it could make sense to do this at a more granular level, so that something like (object-name (flow (-< ...))) ;=> compiled-tee-flow
and (object-name (flow (>< ...)) ;=> compiled-amp-flow
-- but only if it could be done without complicating the implementation / incurring performance overhead since otherwise it wouldn't be worth it.
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.
I wonder whether in the future it could make sense to do this at a more granular level, so that something like (object-name (flow (-< ...))) ;=> compiled-tee-flow and (object-name (flow (>< ...)) ;=> compiled-amp-flow -- but only if it could be done without complicating the implementation / incurring performance overhead since otherwise it wouldn't be worth it.
Looks good to me! I've named these forms here.
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.
Would it be possible to put it in a separate PR from the identity values one? I haven't had a chance to consider the implications of that yet and I'm doubtful that I'll be able to get to it in the near future due to competing priorities. I think we'd be able to merge it sooner if the changes are kept separate (or even included in the present PR). Wdyt?
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.
Would it be possible to put it in a separate PR from the identity values one? I haven't had a chance to consider the implications of that yet and I'm doubtful that I'll be able to get to it in the near future due to competing priorities. I think we'd be able to merge it sooner if the changes are kept separate (or even included in the present PR). Wdyt?
The main job of that PR is to make some forms return the fixed functions in special cases.
Before:
Welcome to Racket v8.6 [cs].
> (require qi)
> (☯ (-<))
#<procedure:...b/flow/compiler.rkt:108:7>
> (eq? (☯ (-<)) (☯ (-<)))
#f
> (eq? (☯ (-<)) (☯ (gen)))
#f
> (eq? (☯ (-<)) (☯ ⏚))
#f
> (eq? (☯ (-<)) (☯ (select)))
#f
> (☯ (==*))
==*: undefined;
cannot reference an identifier before its definition
in module: top-level
[,bt for context]
> (☯ (switch))
#<procedure:values>
> (☯ (fanout 0))
#<procedure:...b/flow/compiler.rkt:494:9>
> (☯ (fanout 1))
#<procedure:...b/flow/compiler.rkt:494:9>
After:
Welcome to Racket v8.6 [cs].
> (require qi)
> (☯ (-<))
#<procedure:*->1>
> (eq? (☯ (-<)) (☯ (-<)))
#t
> (eq? (☯ (-<)) (☯ (gen)))
#t
> (eq? (☯ (-<)) (☯ ⏚))
#t
> (eq? (☯ (-<)) (☯ (select)))
#t
> (☯ (==*))
#<procedure:1->1>
> (☯ (switch))
#<procedure:values>
> (☯ (fanout 0))
#<procedure:*->1>
> (☯ (fanout 1))
#<procedure:values>
Because naming the procedures returned by these forms, and letting these forms return the specific procedures in special cases, are both optimizations to the returned procedures, I thought it would be more convenient to complete these two tasks in one PR. Of course implementing the naming as a PR alone makes sense to me. Wdyt?
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.
OK on this. Btw, I will be traveling starting next week but I'll try to get to these before then. I've added a Calendar to the wiki to have some visibility into the availability of contributors, as well as other general events of interest that are related to the project (e.g. Michael's upcoming talk at RacketCon is on there, and so is my upcoming talk at EmacsConf).
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.
I noticed that the name of (flow (~> + add1))
on its own is composed
. That might be because compose
internally ends up naming the result composed
, and "names closer to an expression take precedence". Do you think this case would be worth handling and renaming to compiled-flow
?
[(_ onex) | ||
#:with name (syntax-local-name) | ||
(quasisyntax/loc stx | ||
(let ([compiled-flow #,((compose compile-flow expand-flow) #'onex)]) |
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: it would be nice to add a comment here explaining the reason for this block of code, i.e. that it allows the name of the function to be inferred by the compiler and supports helpful error messages.
I forgot to mention some issues with this PR a while ago. Currently I feel that renaming via For example: Welcome to Racket v8.6 [cs].
> (struct doc-procedure (doc f)
#:property prop:procedure (struct-field-index f))
> (define f (doc-procedure "f" (lambda () 123)))
> (doc-procedure? f)
#t
> (doc-procedure? (procedure-rename f 'g))
#f
|
In what way would they not work? Could you give an example -- would it just be a naming issue or would it affect the functionality as well? In general, I wonder if that should be considered a bug in |
Also, I'm assuming the |
Yeah, I've reported it (racket/racket#4455).
For example, I redefined (struct coprocedure (coarity result-coarity))
(struct composed coprocedure (f)
#:property prop:procedure (struct-field-index f))
(define (make-composed . f*) ...)
(define-qi-syntax-rule (~> flo ...)
(esc (make-composed (☯ flo) ...))) If we rename the procedure made by
#70 currently only renames some curried functions (they are not structs), so I guess it should have no effect. |
Summary of Changes
Inferring the names of procedures created by
☯
.Public Domain Dedication
(Why: The freely released, copyright-free work in this repository represents an investment in a better way of doing things called attribution-based economics. Attribution-based economics is based on the simple idea that we gain more by giving more, not by holding on to things that, truly, we could only create because we, in our turn, received from others. As it turns out, an economic system based on attribution -- where those who give more are more empowered -- is significantly more efficient than capitalism while also being stable and fair (unlike capitalism, on both counts), giving it transformative power to elevate the human condition and address the problems that face us today along with a host of others that have been intractable since the beginning. You can help make this a reality by releasing your work in the same way -- freely into the public domain in the simple hope of providing value. Learn more about attribution-based economics at drym.org, tell your friends, do your part.)