-
-
Notifications
You must be signed in to change notification settings - Fork 104
Automated Resyntax fixes #1438
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
Automated Resyntax fixes #1438
Changes from all commits
21784fd
63a7fe1
60a1399
da8c553
583a1e3
f8d6c9d
84dc345
c6f5a28
5f1257e
57862b3
651bd68
3575e85
3f21958
f2601c5
c7858d9
c9d12e3
a51e0f3
df62579
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -143,8 +143,7 @@ | |
[#:for-each (f) (for-each f ps)] | ||
[#:custom-constructor/contract | ||
(-> (listof (or/c TypeProp? NotTypeProp? LeqProp?)) OrProp?) | ||
(let ([ps (sort ps (λ (p q) (unsafe-fx<= (eq-hash-code p) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jackfirth my guess is that this change is slower There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it is, that seems like a missed optimization opportunity in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's really an issue of inlining. |
||
(eq-hash-code q))))]) | ||
(let ([ps (sort ps unsafe-fx<= #:key eq-hash-code)]) | ||
(intern-single-ref! | ||
orprop-intern-table | ||
ps | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,14 +24,12 @@ | |
(pt-seq-map f (combinator-args v)) | ||
(void)) | ||
(define (sc->contract v f) | ||
(match v | ||
[(prompt-tag-combinator (pt-seq vals call-cc)) | ||
(with-syntax ([(vals-stx ...) (map f vals)] | ||
[(call-cc-stx ...) | ||
(if call-cc | ||
#`(#:call/cc (values #,@(map f call-cc))) | ||
empty)]) | ||
#'(prompt-tag/c vals-stx ... call-cc-stx ...))])) | ||
(match-define (prompt-tag-combinator (pt-seq vals call-cc)) v) | ||
(with-syntax ([(vals-stx ...) (map f vals)] | ||
[(call-cc-stx ...) (if call-cc | ||
#`(#:call/cc (values #,@(map f call-cc))) | ||
empty)]) | ||
#'(prompt-tag/c vals-stx ... call-cc-stx ...))) | ||
(define (sc->constraints v f) | ||
(merge-restricts* 'chaperone (map f (pt-seq->list (combinator-args v)))))]) | ||
|
||
|
@@ -52,16 +50,11 @@ | |
|
||
|
||
(define (pt-seq-map f seq) | ||
(match seq | ||
[(pt-seq vals call-cc) | ||
(define (f* a) (f a 'invariant)) | ||
(pt-seq | ||
(map f* vals) | ||
(and call-cc (map f* call-cc)))])) | ||
(match-define (pt-seq vals call-cc) seq) | ||
(define (f* a) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sorawee why is this on two lines? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a "community"'s preference to always format function definition this way. It used to support one-line format, but IIRC, either @jackfirth or @Metaxal gathered supporters to NOT do the one-line format, and I made the change accordingly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would be interested to find that discussion; I strongly disagree. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it was me, I don't remember having a strong opinion about this. Perhaps it was more of a pragmatic choice at that time(?). But again, perhaps it wasn't me 😁 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's @Metaxal. I'm very lucky that I wrote your name in the commit message 😂 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Haha indeed! Though "Suggestion by" sounds a bit different from "gathered supporters to NOT do" ;) Again, I don't feel particularly strongly about this, although I do have a preference for two lines so that the function header stands out. I'm not going to die on this hill though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was also me. I was (and still am) of the strong opinion that function definitions should never be on a single line. Otherwise it's hard to tell them apart from variable definitions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Functions are just another kind of variable. ;) |
||
(f a 'invariant)) | ||
(pt-seq (map f* vals) (and call-cc (map f* call-cc)))) | ||
|
||
(define (pt-seq->list seq) | ||
(match seq | ||
[(pt-seq vals call-cc) | ||
(append | ||
vals | ||
(or call-cc empty))])) | ||
(match-define (pt-seq vals call-cc) seq) | ||
(append vals (or call-cc empty))) |
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.
@jackfirth this could use
format-symbol
fromracket/syntax
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.
Added a new rule for this in jackfirth/resyntax#442.