Opened 2 months ago

Last modified 2 months ago

#1355 new defect

define strips hygiene from defined identifiers at top-level

Reported by: ashinn Owned by:
Priority: major Milestone: 5.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 (6)

comment:1 Changed 2 months ago by ashinn

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 2 months ago by ashinn

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 2 months ago by ashinn

  • Keywords define-record-type removed
  • Summary changed from make define-record-type setters hygienic to define strips hygiene from defined identifiers

comment:4 Changed 2 months ago by ashinn

  • Summary changed from define strips hygiene from defined identifiers to define strips hygiene from defined identifiers at top-level

comment:5 Changed 2 months ago by ashinn

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 2 months ago by sjamaan

  • Component changed from core libraries to expander
  • Estimated difficulty set to hard
  • Milestone changed from someday to 5.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.

Note: See TracTickets for help on using tickets.