Opened 8 years ago
Closed 8 years ago
#1309 closed defect (fixed)
CHICKEN allows misplaced internal defines, which live on outside the body
Reported by: | ai-artisan | Owned by: | |
---|---|---|---|
Priority: | critical | Milestone: | 5.0 |
Component: | core libraries | Version: | 4.11.0 |
Keywords: | record type define | Cc: | |
Estimated difficulty: | hard |
Description
(require-extension r7rs)
(let ()
(define-record-type bug
(make-bug)
bug?
)
)
(define b (make-bug))
(display (bug? b))
(newline)
;;output: #t
Attachments (1)
Change History (18)
comment:1 Changed 8 years ago by
Summary: | record types defined inside a closure lives beyond the closure → record types defined inside a body lives beyond the body |
---|
comment:2 follow-up: 3 Changed 8 years ago by
comment:4 Changed 8 years ago by
A simpler test case that triggers this bug:
(let () (##core#begin (##core#set! lala 1) (##core#begin (define (x) 1)))) (print x) ; => #<procedure (x)>
comment:5 Changed 8 years ago by
Same without the ##core
stuff, by the way. I suspect this is some internal define logic that goes wrong. Happens when compiling, too.
comment:6 Changed 8 years ago by
Milestone: | someday → 5.0 |
---|
Gauche behaves identically to CHICKEN. Racket, Guile, Gambit and Scheme48 give an error that the define is in an expression context (after adding an expression after it).
I guess technically, CHICKEN is correct here (in the sense that R5RS undefined behaviour allows this). It is confusing though, so we may want to disallow this. It's a potentially breaking change, so I think if we decide to change this, we should do so in CHICKEN 5.
comment:7 Changed 8 years ago by
Summary: | record types defined inside a body lives beyond the body → CHICKEN allows misplaced internal defines, which live on outside the body |
---|
comment:8 Changed 8 years ago by
In other words, the following program which uses only standard R5RS constructs is invalid according to R5RS:
(define lala 2) (let () (begin (set! lala 1) (begin (define (x) 1) 2))) (print x) ; => #<procedure (x)> in CHICKEN and Gauche, error in other Schemes
comment:9 Changed 8 years ago by
The set!
is just a red herring; any expression there triggers this bug
comment:10 Changed 8 years ago by
This triggers the bug:
(let () 1 (begin (define (x) 1) 2))
Removing the 1
or the superfluous begin
(which should have no effect) causes the bug to disappear.
comment:11 Changed 8 years ago by
See also #1297 (which I just closed as a duplicate of this ticket)
comment:12 Changed 8 years ago by
Estimated difficulty: | → hard |
---|
comment:13 Changed 8 years ago by
OK, this is pretty bad. We can hack in a special case for begin
, but that won't fix it in general, because something like (print (define x 1))
can also occur inside the body and won't be detected. Interestingly, this seems to be how Gauche does it, because the following program runs:
(print (define (x) 1)) (print (x)) ;; Prints #<undef> followed by 1
I had a look at how Scheme48 does it:
Their definitions are disabled completely, except at toplevel and when parsing internal defines. Any other place will trigger an error. Their definition of define
is:
(define-expander 'define (lambda (op op-node exp env) (syntax-violation 'define (if (destructure-define exp) "definition in expression context" "ill-formed definition") exp)))
When toplevel is walked, they handle define
and friends specially. This also allows them to treat define
differently from set!
.
Gambit seems to take a similar approach:
(define (##comp-define cte src tail? subexpr?) (if (or subexpr? (##not ##allow-inner-global-define?)) (##raise-expression-parsing-exception 'ill-placed-define src) ...))
In other words, ##comp-define
knows if it's being processed as a toplevel expression or as a sub-expression. There should never be a define in a subexpression (internal define
s are transformed to letrec so those are gone after processing a lambda
, for example).
Whichever approach we take, a proper fix for this will require quite a bit of rework and hardcoding all defining forms into the compiler (which we have to do already anyway, to make internal defines work).
I have to agree with the Scheme48 authors: ; The horror of internal defines
comment:14 Changed 8 years ago by
OK, this is pretty bizarre: in several places in the tests, we use functors and modules in an expression context.
(let ((foo (module blah () 1))) foo) => 1
In this regard, I'm going to say module
is like eval
in that it resets the state to a fresh "toplevel context" for the evaluation/compilation of its body. Very weird!
Changed 8 years ago by
Attachment: | 0001-Let-macros-know-if-they-run-at-toplevel.patch added |
---|
Initial attempt to teach compiler and evaluator about toplevel
comment:16 Changed 8 years ago by
Because there's no r7rs egg for CHICKEN 5 yet, here's a testcase that contains just the minimum required code for r7rs define-record-type:
(begin-for-syntax (define (macro-handler name) (cond ((assq name (##sys#macro-environment)) => caddr) (else #f))) (define (wrap-er-macro-transformer name handler) (er-macro-transformer (let ((orig (macro-handler name))) (lambda (x r c) (let ((e (##sys#current-environment))) (handler x r c (lambda (x*) (orig x* '() e))))))))) (let () (define-record-type bug (make-bug) bug?) ) (define b (make-bug)) (display (bug? b)) (newline)
comment:17 Changed 8 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
This is fixed in the chicken-5 branch, by the following set of revisions:
- 61241e5d58299264ff0a8a7318288906ff710660
- d345e514c10956a7e95267dfd027725f89394122
- c9220247dbcdf6fd39697b428cfd40068244219a
- bd0aa1c6088d865988cf5afc2f53dad36cbe2d3b
Looks similar to http://bugs.call-cc.org/ticket/445