Opened 8 years ago

Closed 4 months ago

#1294 closed defect (wontfix)

define-record-printer messes with internal defines

Reported by: sjamaan Owned by:
Priority: major Milestone: someday
Component: expander Version: 4.11.0
Keywords: Cc:
Estimated difficulty: hard

Description (last modified by sjamaan)

As pointed out by russelw on IRC, the following program fails to compile (and doesn't work in the interpreter, either):

(module main ()
 (import chicken)
 (import scheme)

 (define (h)
  (define (f)
   (g))

  (define-record-printer (foo x port)
   #f)

  (define (g)
   (f))))

This is probably similar to #1274 and #1309.

Change History (13)

comment:1 Changed 8 years ago by sjamaan

The problem here is that define-record-printer isn't a true definition. This code is comparable to the following:

(define (h)
  (define (f)
   (g))

  (display "hello\n")

  (define (g)
   (f)))

Other Schemes will bail out on this, complaining that the definition of g is misplaced. CHICKEN accepts it, so the code from the ticket body should probably also be accepted.

comment:2 Changed 8 years ago by sjamaan

Actually, the snippet above doesn't work in CHICKEN either; the compiler bails out when wrapped in a module, and if you insert a call to (g), it will die at runtime with a "unbound variable: g".

So perhaps it simply needs better error reporting? Other Schemes complain that the define for g is invalid.

comment:3 Changed 8 years ago by sjamaan

Estimated difficulty: hard

comment:4 Changed 8 years ago by sjamaan

Description: modified (diff)

comment:5 Changed 7 years ago by sjamaan

Milestone: 4.12.05.0

This requires sweeping changes, so better not mess with this for the 4.x series.

comment:6 Changed 7 years ago by sjamaan

Milestone: 5.05.1

The reason this does not work is that each statement after a define ensures a following set of defines will start a new "letrec" block. Of course, that means forward references in earlier letrec blocks can't "see" definitions of inner letrec blocks. I'm not sure it's worthwhile or even desirable to make this work differently.

On the other hand, I can imagine a system that works more like the toplevel, where each define simply causes the definition to be made, so that any later define can refer back to it (but, that means you could get undefined variable errors at runtime, I think)

The code that deals with this stuff is extremely hairy, though, and the benefit is marginal (and could break other code, too!). Maybe a documentation change is warranted instead of trying to fix this.

comment:7 Changed 5 years ago by sjamaan

Milestone: 5.15.2

Getting ready for 5.1, moving tickets which won't make it in to 5.2.

comment:8 Changed 5 years ago by felix winkelmann

Remark: alexpander has a nice feature where (define <form>) allows an arbitrary expression <form> in any definition context to be treated as a definition, without terminating a sequence of local definitions.

comment:9 Changed 4 years ago by sjamaan

As Evan remarked:

These work fine:

define-record
define-record-type
define-values

These don't work, but they also don't really make sense outside the
toplevel (and most of them are documented as such) so I think they're
fine to ignore:

define-constant
define-external
define-foreign-type
define-foreign-variable
define-inline
define-interface
define-location

These don't work, but seem like they probably ought to:

define-compiler-syntax
define-for-syntax
define-reader-ctor
define-record-printer
define-specialization
define-syntax
define-type

comment:10 Changed 4 years ago by sjamaan

Milestone: 5.25.3

Not important enough for 5.2 IMO

comment:11 Changed 3 years ago by sjamaan

Milestone: 5.35.4

Maybe we can review the things starting with define- for 5.4. At least in 5.2 we deprecated define-record-printer in favour of set-record-printer! which takes care of the original ticket description.

comment:12 Changed 4 months ago by felix winkelmann

Milestone: 5.4someday

comment:13 Changed 4 months ago by felix winkelmann

Resolution: wontfix
Status: newclosed
Note: See TracTickets for help on using tickets.