-
-
Notifications
You must be signed in to change notification settings - Fork 104
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
Patched require/untyped-contract to accept a language spec. #1330
base: master
Are you sure you want to change the base?
Conversation
Added cases to allow optional language spec, and removed redundant format-symbol
Hi Raol, sorry I am so slow. Just want you to know that I know this PR exists and will take a look soon! |
EDIT: nevermind, the problem was that I used Tried running this on the example, and I'm getting an error about an
|
'require)] | ||
[*typed/racket (datum->syntax #'from-module-spec (syntax-e #'language-spec))] | ||
[*require (datum->syntax #'from-module-spec 'require)] | ||
[*language-spec (datum->syntax #'racket/base (syntax-e #'language-spec))] |
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 was typed/racket
before. I think it needs to be the language-spec.
EDIT: oh, I misremembered the argument order. This could just be #'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.
@bennn Thanks for the feedback. I've changed the datum->syntax
context to #'_
. I'm not really up to good coding style conventions for Racket, so I hope I've done the right thing.
EDIT:
As for the error about an (only-in quote)
, I agree the message could be improved. From a cursory glance at the source, I think the most feasible way of doing it is to wrap require/untyped-contract
in a with-handlers
form and providing a custom message. Letting the exception bubble up to handle it more generally immediately brings us to core racket collections that I'd rather not touch. Thoughts ?
Is there anything else I can do to improve this PR ?
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.
how about changing the syntax from typed/racket/shallow
to #:lang typed/racket/shallow
--- from :id
to keyword +:expr
? that should improve the error and make this easier to use later.
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.
Resyntax analyzed 1 file in this pull request and found no issues.
@@ -29,46 +29,52 @@ | |||
(stx-map (lambda (id) ((make-syntax-introducer) id)) ids)) | |||
|
|||
(define-syntax (require/untyped-contract stx) | |||
(syntax-parse stx #:literals (begin) | |||
[(_ (begin form ...) from-module-spec:expr [name:id T:expr] ...) |
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.
keep the indentation like it was
Hi @bennn ,
This is a very modest proposal for #1286. I did not patch the docs yet, but I will do so if there is interest in this pull request. It seems to work with the following test code (changed language to typed/racket/no-check):
server.rkt
main.rkt
Cheers!