Summary
CHICKEN allows misplaced internal defines, which live on outside the body
Metadata
- Id: dd714e4ef7ecfec3ddd3e3b66011b0c8195c5adf
- Trac id: 1309
- Type: defect
- Reporter: ai-artisan
- Owner:
- Cc:
- Status: closed
- Component: core libraries
- Estimated difficulty: hard
- Resolution: fixed
- Priority: critical
- Milestone: 5.0
- Version: 4.11.0
- Changetime: 2017-03-22 11:58:11 UTC
- Created: 2016-07-25 09:52:46 UTC
- Keywords: record type define
Attachments
- dd714e4ef7ecfec3ddd3e3b66011b0c8195c5adf/attachments/0001-Let-macros-know-if-they-run-at-toplevel.patch
Description
(require-extension r7rs)
(let ()
(define-record-type bug (make-bug) bug? )
)
(define b (make-bug)) (display (bug? b)) (newline)
- ;output
- #t
Changes and comments
[2016-07-25 09:53:56 UTC] ai-artisan changed summary
[2016-07-25 09:58:28 UTC] mario wrote:
Looks similar to http://bugs.call-cc.org/ticket/445
[2016-07-25 10:06:59 UTC] ai-artisan wrote:
yes but the bug still exists at 4.11.0
[2016-08-02 20:10:53 UTC] sjamaan wrote:
A simpler test case that triggers this bug:
#!scm (let () (##core#begin (##core#set! lala 1) (##core#begin (define (x) 1)))) (print x) ; => #<procedure (x)>
[2016-08-02 20:12:13 UTC] sjamaan wrote:
Same without the ##core stuff, by the way. I suspect this is some internal define logic that goes wrong. Happens when compiling, too.
[2016-08-02 20:20:42 UTC] sjamaan changed milestone from someday to 5.0
[2016-08-02 20:20:42 UTC] sjamaan wrote:
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.
[2016-08-02 20:21:14 UTC] sjamaan changed summary
[2016-08-02 20:22:26 UTC] sjamaan wrote:
In other words, the following program which uses only standard R5RS constructs is invalid according to R5RS:
#!scm
(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
[2016-08-02 20:22:26 UTC] sjamaan wrote:
1470162156647727
[2016-08-02 20:23:43 UTC] sjamaan wrote:
The set! is just a red herring; any expression there triggers this bug
[2016-08-02 20:26:18 UTC] sjamaan wrote:
This triggers the bug:
#!scm
(let ()
1
(begin
(define (x) 1)
2))
(print x) ; => #<procedure (x)> in CHICKEN and Gauche, error in other Schemes
Removing the 1 or the superfluous begin (which should have no effect) causes the bug to disappear.
[2016-08-02 20:26:18 UTC] sjamaan wrote:
1470162415620959
[2016-08-02 20:36:12 UTC] sjamaan wrote:
See also #1297 (which I just closed as a duplicate of this ticket)
[2016-08-25 20:41:15 UTC] sjamaan set difficulty to hard
[2016-11-24 20:58:11 UTC] sjamaan wrote:
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
[2016-12-08 21:39:34 UTC] sjamaan wrote:
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!
[2016-12-08 21:45:30 UTC] sjamaan attached 0001-Let-macros-know-if-they-run-at-toplevel.patch (description=Initial attempt to teach compiler and evaluator about toplevel)
[2016-12-08 21:45:45 UTC] sjamaan wrote:
Now all that remains is to add some tests.
[2017-02-26 17:37:03 UTC] sjamaan wrote:
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)
[2017-03-22 11:58:11 UTC] sjamaan changed status from new to closed
[2017-03-22 11:58:11 UTC] sjamaan set resolution to fixed
[2017-03-22 11:58:11 UTC] sjamaan wrote:
This is fixed in the chicken-5 branch, by the following set of revisions:
- 61241e5d58299264ff0a8a7318288906ff710660
- d345e514c10956a7e95267dfd027725f89394122
- c9220247dbcdf6fd39697b428cfd40068244219a
- bd0aa1c6088d865988cf5afc2f53dad36cbe2d3b