Opened 7 years ago

Last modified 5 months ago

#1355 new defect

define strips hygiene from defined identifiers at top-level

Reported by: Alex Shinn Owned by:
Priority: major Milestone: 6.0.0
Component: expander Version: 4.12.0
Keywords: hygiene Cc:
Estimated difficulty: hard

Description

The following program with tests passes in chibi and scheme48, but fails in chicken. The expander handles inserted "tmp" identifiers and disambiguates correctly for readers and predicates, but it appears define-record-type treats all the setters the same.

(cond-expand
 (chibi (import (scheme base) (chibi test)))
 (chicken (use test)))

;; other (,open srfi-9 srfi-23 in scheme48)
;; (define (read-string k in)
;;   (let lp ((i k) (res '()))
;;     (if (zero? i)
;;         (list->string (reverse res))
;;         (lp (- i 1) (cons (read-char in) res)))))
;; (define (test-begin) #f) (define (test-end) #f)
;; (define (test e x)
;;   (if (not (equal? e x))
;;       (error "test failed" e '!= x)))

;; utility primitive type consisting of a predicate, reader and writer

(define-syntax define-binary-type
  (syntax-rules ()
    ((define-binary-type name pred reader writer)
     (define-syntax name
       (syntax-rules (pred: read: write:)
         ((name pred: args) (pred args))
         ((name read: args) (reader args))
         ((name write: args) (writer args)))))))

;; a fixed-length string

(define-binary-type fixed-string
  (lambda (args)
    (let ((len (car args)))
      (lambda (x) (and (string? x) (= (string-length x) len)))))
  (lambda (args)
    (let ((len (car args)))
      (lambda (in)
        (read-string len in))))
  (lambda (args)
    (lambda (str out)
      (write-string str out))))

;; wrapper around define-record-type to provide type checking and
;; (de)serialization

(define-syntax defrec
  (syntax-rules ()
    ;; all fields processed: expand record, reader and setters
    ((defrec name (make . make-fields) pred reader ()
       (field (type . args) read-field get %set set) ...)
     (begin
       (define-record-type name (make . make-fields) pred
         (field get %set) ...)
       (define set
         (let ((field? (type pred: 'args)))
           (lambda (x val)
             (if (not (field? val))
                 (error "invalid field" '(type . args) val))
             (%set x val))))
       ...
       (define reader
         (let ((read-field (type read: 'args)) ...)
           (lambda (in)
             (let ((field (read-field in)) ...)
               (make . make-fields)))))))
    ;; step: insert read-field and %set bindings
    ((defrec name make pred reader
       ((field (type . args) get set) . rest)
       fields ...)
     (defrec name make pred reader rest fields ...
       (field (type . args) read-field get %set set)))))

(define-syntax define-binary-record-type
  (syntax-rules ()
    ((define-binary-record-type name make pred reader . fields)
     (defrec name make pred reader fields))))

;; example

(define-binary-record-type stuff (make-stuff foo bar) stuff?
  read-stuff
  (foo (fixed-string 2) stuff-foo stuff-foo-set!)
  (bar (fixed-string 3) stuff-bar stuff-bar-set!))

(test-begin)
(let ((x (make-stuff "ab" "bar")))
  (test "ab" (stuff-foo x))
  (test "bar" (stuff-bar x))
  (stuff-foo-set! x "xy")    ;; <----- this checks the right field type
                             ;;        but uses the sets the wrong %set
  (test "xy" (stuff-foo x))
  (test "bar" (stuff-bar x)))
(test-end)

Change History (8)

comment:1 Changed 7 years ago by Alex Shinn

A stripped down version:

(use test)

(define-syntax %defrec
  (syntax-rules ()
    ((%defrec name make pred ((field get %set set) ...) ())
     (begin
       (define-record-type name make pred
         (field get %set) ...)
       (define (set x v)
         (display "mutation is evil!\n")
         (%set x v))
       ...))
    ((%defrec name make pred (fields ...) ((field get set) . rest))
     (%defrec name make pred (fields ... (field get %set set)) rest))))

(define-syntax defrec
  (syntax-rules ()
    ((defrec name make pred . fields)
     (%defrec name make pred () fields))))

(defrec stuff (make-stuff foo bar) stuff?
  (foo stuff-foo stuff-foo-set!)
  (bar stuff-bar stuff-bar-set!))

(test-begin)
(let ((x (make-stuff "ab" "bar")))
  (test "ab" (stuff-foo x))
  (test "bar" (stuff-bar x))
  (stuff-foo-set! x "xy")    ;; <----- this uses the wrong %set
  (test "xy" (stuff-foo x))
  (test "bar" (stuff-bar x)))
(test-end)

comment:2 Changed 7 years ago by Alex Shinn

Note debugging with ##core#syntax shows the expansion is correct:

(begin151
 (define-record-type152 stuff (make-stuff foo bar) stuff?
   (foo stuff-foo %set148)
   (bar stuff-bar %set150))
 (define153 (stuff-foo-set! x154 v155)
   (display156 "mutation is evil!\n")
   (%set148 x154 v155))
 (define153 (stuff-bar-set! x154 v155)
   (display156 "mutation is evil!\n")
   (%set150 x154 v155)))

Also, debugging the define-record-type macro show it's output also looks correct:

(##core#begin
 (define221 (make-stuff foo bar)
   (##sys#make-structure (##core#quote stuff) foo bar))
 (define221 (stuff? x223)
   (##sys#structure? x223 (##core#quote stuff)))
 (define221 stuff-foo
   (##core#lambda (x223)
     (##core#check (##sys#check-structure x223 (##core#quote stuff) (##core#quote stuff-foo)))
     (##sys#block-ref x223 1)))
 (define221 %set212
   (##core#lambda (x223 y224)
     (##core#check (##sys#check-structure x223 (##core#quote stuff) (##core#quote #f)))
     (##sys#block-set! x223 1 y224)))
 (define221 stuff-bar
   (##core#lambda (x223)
     (##core#check (##sys#check-structure x223 (##core#quote stuff) (##core#quote stuff-bar)))
     (##sys#block-ref x223 2)))
 (define221 %set214
   (##core#lambda (x223 y224)
     (##core#check (##sys#check-structure x223 (##core#quote stuff) (##core#quote #f)))
     (##sys#block-set! x223 2 y224))))

however, it seems that hygiene is stripped from all defined identifiers, so the renamed sets all revert to %set.

comment:3 Changed 7 years ago by Alex Shinn

Keywords: define-record-type removed
Summary: make define-record-type setters hygienicdefine strips hygiene from defined identifiers

comment:4 Changed 7 years ago by Alex Shinn

Summary: define strips hygiene from defined identifiersdefine strips hygiene from defined identifiers at top-level

comment:5 Changed 7 years ago by Alex Shinn

The simplest way to reproduce the core bug is:

#;1> (define-syntax deffoo (syntax-rules () ((deffoo val) (define foo 42))))
#;2> (deffoo 42)
#;3> foo
42

foo should have been hygienically renamed, making it effectively invisible. Likely this is by design, and changing it would break existing code, but it also makes many types of macros impossible by making it impossible to insert temp bindings.

Note this only affects the top-level: (let () (deffoo 42) foo) throws an error as expected.

comment:6 Changed 7 years ago by sjamaan

Component: core librariesexpander
Estimated difficulty: hard
Milestone: someday5.0

You're right, this is by design. I don't know if we can handle any more super-invasive changes like this for CHICKEN 5, but considering fixing this would be backwards incompatible, I guess 5.0 would be the time to fix it.

comment:7 Changed 6 years ago by felix winkelmann

Milestone: 5.0someday

comment:8 Changed 5 months ago by felix winkelmann

Milestone: someday6.0.0
Note: See TracTickets for help on using tickets.