CHICKEN allows misplaced internal defines, which live on outside the body

[ai-artisan]

(require-extension r7rs)

(let ()

(define-record-type bug

(make-bug)
bug?

)

)

(define b (make-bug))
(display (bug? b))
(newline)

;;output: #t

[Mario Domenech Goulart]

Looks similar to ​http://bugs.call-cc.org/ticket/445

[ai-artisan]

yes but the bug still exists at 4.11.0

[sjamaan]

A simpler test case that triggers this bug:

(let () (##core#begin (##core#set! lala 1) (##core#begin (define (x) 1))))
(print x) ; => #<procedure (x)>

[sjamaan]

Same without the ##core stuff, by the way. I suspect this is some internal define logic that goes wrong. Happens when compiling, too.

[sjamaan]

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.

[sjamaan]

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

[sjamaan]

The set! is just a red herring; any expression there triggers this bug

[sjamaan]

This triggers the bug:

(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.

[sjamaan]

See also #1297 (which I just closed as a duplicate of this ticket)

[sjamaan]

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

[sjamaan]

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!

[sjamaan]

Initial attempt to teach compiler and evaluator about toplevel

[sjamaan]

Now all that remains is to add some tests.

[sjamaan]

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)

[sjamaan]

This is fixed in the chicken-5 branch, by the following set of revisions:

• 61241e5d58299264ff0a8a7318288906ff710660
• d345e514c10956a7e95267dfd027725f89394122
• c9220247dbcdf6fd39697b428cfd40068244219a
• bd0aa1c6088d865988cf5afc2f53dad36cbe2d3b