-
Notifications
You must be signed in to change notification settings - Fork 160
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
Interop seamlessly with racket #161
base: master
Are you sure you want to change the base?
Conversation
9bcda91
to
d143e7e
Compare
The description at http://arclanguage.org/item?id=21163 is super useful to understanding the intent of this PR. Thanks! |
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 have some extensive feedback at various levels of severity. Most importantly, see the "this seems incorrect" comment where I talk about how Anarki's existing $
and unquote
approach already addresses these needs with what I think of as a better design.
@@ -1,10 +1,8 @@ | |||
#lang racket/base | |||
#lang racket/load |
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.
This makes it impossible to provide
things which can be require
d by other Racket modules. This makes it hard to load more than one instance of the Anarki runtime in a Racket program; the only way to load this code more than once is by using something dynamic like eval
or dynamic-require
. And if that's the approach you think we should take, then you should keep in mind this would currently cause us to have a different ar-atomic-sema
semaphore and a different ar-tagged
structure type in each Anarki runtime.
|
||
(read-accept-bar-quote #f) |
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.
This side effect potentially clobbers other uses of read
in a larger Racket program that happens to load Anarki.
@@ -41,8 +41,112 @@ | |||
(define main-namespace | |||
(namespace-anchor->namespace main-namespace-anchor)) | |||
|
|||
(define lang* (make-parameter 'arc)) |
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.
Why make this a parameter? Why not pass it as an explicit argument of ac
?
(let ((x (assoc s ac-global-names))) | ||
(if ;#f | ||
x | ||
(cadr x) (string->symbol (string-append "" (symbol->string s))))) |
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.
If this is the way you intended to indent this if
call, all right, but I want to call attention to it in case it was an accident.
(and (symbol? x) (> (string-length (symbol->string x)) 0) (eqv? #\| (string-ref (symbol->string x) 0)))) | ||
|
||
(define (id-literal x) | ||
(let ((s (substring (symbol->string x) 1 (- (string-length (symbol->string x)) 1)))) |
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.
This file uses square brackets for let
bindings and cond
clauses, as per the usual Racket style. If you want to change that, why, and why not change it everywhere else in the file too?
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 notice you're shaving off the last character of the identifier here, probably because you're assuming it's a |
character, but you never actually check for that; you only check for |
at the beginning (in id-literal?
).
Why use the syntax |foo|
anyway? It looks like you may have been somehow inspired by the existing meaning of |foo|
, but the fact this syntax clobbers that one is more of a downside than an upside.
@@ -933,7 +1071,8 @@ | |||
((async-channel? x) 'channel) | |||
((evt? x) 'event) | |||
[(keyword? x) 'keyword] | |||
[#t (err "Type: unknown type" x)])) | |||
; [#t (err "Type: unknown type" x)])) | |||
[#t (vector-ref (struct->vector x) 0)])) |
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.
Fair implementation for treating Racket as though it's Arc, but for as inconsistent as Arc is about type
-related behavior, struct->vector
is possibly even more unreliable in Racket. I recently brought up racket/racket#2454 and while they were fixing it, they tweaked several struct->vector
results across the language.
I think if you want a catch-all case for exotic Racket values, returning 'unknown
would probably be better. Anarki doesn't promise future-compatibility, but it's easy for programmers to overlook that and expect future-compatibility anyway. If a programmer interactively discovers the type of something is 'unknown
, I bet that'll raise their awareness of the instability, and they might be able to prepare themselves better for it to become "known" as something else in a future Anarki release.
@@ -1603,10 +1742,9 @@ Arc 3.1 documentation: https://arclanguage.github.io/ref. | |||
val)) | |||
|
|||
(define (bound? arcname) | |||
(with-handlers ([exn:fail:syntax? (lambda (e) #t)] | |||
(with-handlers ([exn:fail:syntax? (lambda (e) (if (eqv? arcname '_) #f '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.
I've had trouble with the _
syntax in Racket interacting with Arc too, lol. :)
I think a more seamless approach to bound?
here (which isn't to say a very easy or accessible one) might first involve running all Arc code during a Racket syntax transformer, so that we could call syntax-local-value
to determine whether something was bound to Racket syntax. Then, to make the interaction with _
make sense, we would refactor ac
so that instead of recursively compiling a whole expression, it would expand to a Racket expression that contained nested occurrences of a Racket macro that expanded its body as Arc code again. That way, when _
is used as a local variable, the Racket macroexpander will understand this, and syntax-local-variable
won't look up the module-level meaning anymore.
Relying on Racket's macroexpander like that would railroad the development of the Anarki macro system; I suspect it couldn't do certain things until a new Racket release allowed it to. For all the things Racket's macroexpander can do that ac
can't, I think of it as a downgrade in the long term. Racket's macroexpander design is full of dynamic scoping (e.g. the fact syntax-local-value
can only be used within the dynamic extent of a macro call) and leaky abstractions (e.g. the fact that Racket macros return s-expressions which their caller can pull apart) and ac
is closer to a design path that avoids these things.
[exn:fail:contract:variable? (lambda (e) #f)]) | ||
(namespace-variable-value (ac-global-name arcname)) | ||
#t)) | ||
(namespace-variable-value (ac-global-name arcname)))) |
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.
If you change bound?
to return something other than a Boolean, I think it will conform to Racket conventions better to remove the question mark from the name. If you're trying to keep it the way it is for backwards compatibility, then I recommend making a second procedure and leaving this one as a predicate.
(if acons.l | ||
(do (f car.l) (recur cdr.l)) | ||
(generator? l) (let x (l) (unless (void? x) (f x) (recur l))) | ||
(sequence? l) (walk (sequence->generator l) f)))) |
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.
The indentation on this seems inconsistent. The shortest case is split across two lines while the longer ones are crammed onto one. At first glance I thought this was an if
with four subforms, when it really has six subforms.
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.
Oh, right, isn't the point of walk
that it's a stub for people to defextend
? I think if you're going to extend it for generators and sequences, those would be better placed in defextend
declarations.
`(,(ac fn env) ,@(map (lambda (x) (ac x env)) args)))] | ||
((and (id-literal? fn) | ||
(void? (id-literal fn))) | ||
(map (lambda (x) (ac x env)) args)) |
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.
This seems incorrect. The point of ac
is to compile an Arc expression into a Racket expression. When the thing we're calling is a Racket macro, not all the subforms are necessarily supposed to be expressions; some are patterns or variable bindings or require
specifications, etc.
By treating them as expressions, you're changing the meaning of ac
so that sometimes it processes things that aren't really expressions. So ac
ends up having to be a compiler and an s-expression template DSL at the same time, in different contexts.
I think Anarki's existing approach, using $
to get into Racket and unquote
to return to Arc, is already a better s-expression template DSL design than this is.
Here's a link to the Arc Forum thread since some discussion has happened there: http://arclanguage.org/item?id=21162 |
This PR illustrates a way to interop seamlessly between racket and arc. More details. Most of the tests pass.
(Not really intended to be merged. I'm doing something similar in https://github.com/laarc/laarc so I wanted to show how it could be done in anarki.)