Opened 9 years ago
Closed 9 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 9 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 9 years ago by
comment:4 Changed 9 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 9 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 9 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 9 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 9 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 9 years ago by
The set! is just a red herring; any expression there triggers this bug
comment:10 Changed 9 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 9 years ago by
See also #1297 (which I just closed as a duplicate of this ticket)
comment:12 Changed 9 years ago by
| Estimated difficulty: | → hard |
|---|
comment:13 Changed 9 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 defines 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 9 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 9 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 9 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 9 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