Opened 10 years ago

Closed 10 years ago

Last modified 9 years ago

#368 closed defect (fixed)

File level define-setf-expander is not available for expansion unless wrapped in an eval-when

Reported by: Mark Evenson Owned by:
Priority: major Milestone: 1.3.2
Component: java Version: 1.4.0-dev
Keywords: Cc:
Parent Tickets:

Description

define-setf-expander is not available for expansion later in the file on ABCL unless we wrap it in an eval-when

https://github.com/easye/cffi/commit/03624e609cd48cc065bd15c8dcea715f287b6bbc

Slyrus ran into this while working on CFFI.

Change History (9)

comment:1 Changed 10 years ago by Mark Evenson

Summary: define-setf-expander is not available for expansion later in the file on ABCL unless we wrap it in an eval-whenFile level define-setf-expander is not available for expansion unless wrapped in an eval-when

Check compiled v. interpreted versions.

comment:2 Changed 10 years ago by Mark Evenson

The exact conditions to trigger this problem are not simple, as the following works in both interpreted and compiled modes

;;<file:///opt/local/share/doc/lisp/HyperSpec-7-0/HyperSpec/Body/m_defi_3.htm>

(defun lastguy (x) 
  (car (last x)))

(define-setf-expander lastguy (x &environment env)
  "Set the last element in a list to the given value."
  (multiple-value-bind (dummies vals newval setter getter)
      (get-setf-expansion x env)
    (let ((store (gensym)))
      (values dummies
              vals
              `(,store)
              `(progn (rplaca (last ,getter) ,store) ,store)
              `(lastguy ,getter)))))

(defun use-lastguy ()
  (let ((a `(1 2 3 4)))
    (setf (lastguy a) 42)
    a))

comment:3 in reply to:  2 ; Changed 10 years ago by Mark Evenson

Replying to mevenson:

The exact conditions to trigger this problem are not simple, as the following works […]

My current working hypothesis is that things involving macros aren't being expanded at the correct time, which seems to be the case

  (defun lastguy (x) 
  (car (last x)))

(define-setf-expander lastguy (x &environment env)
  "Set the last element in a list to the given value."
  (multiple-value-bind (dummies vals newval setter getter)
      (get-setf-expansion x env)
    (let ((store (gensym)))
      (values dummies
              vals
              `(,store)
              `(progn (rplaca (last ,getter) ,store) ,store)
              `(lastguy ,getter)))))

(defmacro use-lastguy-macro ()
  (let ((a `(1 2 3 4)))
    (setf (lastguy a) 42)
    a))

fails at

The function (SETF LASTGUY) is undefined.
   [Condition of type UNDEFINED-FUNCTION]

Restarts:
 0: [RETRY] Retry SLIME REPL evaluation request.
 1: [*ABORT] Return to SLIME's top level.
 2: [ABORT] Abort thread.

Backtrace:
  0: (#<FUNCTION {2290F71D}> #<UNDEFINED-FUNCTION {7F80EF61}> #<FUNCTION {2290F71D}>)
  1: (APPLY #<FUNCTION {2290F71D}> (#<UNDEFINED-FUNCTION {7F80EF61}> #<FUNCTION {2290F71D}>))
  2: (SYSTEM::RUN-HOOK SYSTEM::*INVOKE-DEBUGGER-HOOK* #<UNDEFINED-FUNCTION {7F80EF61}> #<FUNCTION {2290F71D}>)
  3: (INVOKE-DEBUGGER #<UNDEFINED-FUNCTION {7F80EF61}>)
  4: org.armedbear.lisp.Lisp.error(Lisp.java:382)
  5: org.armedbear.lisp.Symbol.getSymbolSetfFunctionOrDie(Symbol.java:426)
  6: org.armedbear.lisp.ticket_368_3.execute(ticket-368.lisp:75)
  7: org.armedbear.lisp.LispThread.execute(LispThread.java:832)
  8: org.armedbear.lisp.Primitives$pf_funcall.execute(Primitives.java:2702)
  9: ((MACRO-FUNCTION USE-LASTGUY-MACRO) (USE-LASTGUY-MACRO) #<ENVIRONMENT {301F1AAE}>)
Last edited 10 years ago by Mark Evenson (previous) (diff)

comment:4 in reply to:  3 Changed 10 years ago by Mark Evenson

Replying to mevenson:

Triaged back to failure in abcl-1.1.1, so this is not the result of recent damage.

comment:5 Changed 10 years ago by charmon

This is also true for define-modify-macro. Both issues are fixed by the most recent 2 commits on my setf-and-modify-eval-when branch: https://github.com/slyrus/abcl/commits/setf-and-modify-eval-when

Any chance of getting this committed soon? Then we can revert the corresponding CFFI patch and get the rest of the ABCL support for CFFI committed.

comment:6 Changed 10 years ago by Mark Evenson

In testing your patch, ran into an issue that may or may not have been a result of not doing a complete recompilation. I'll get back to this later today.

Unfortunately for CFFI, until we get a release of abcl-1.4.0 or (abcl-1.3.2) with this patch, I wouldn't recommend changing the CFFI patch, as almost no users will have had a chance to update. I don't know where the compromise lies: since we have to appease the CFFI maintainers, and the problems with DEFINE-SETF-MODIFY affect the code shared with other Lisp implementation, I don't expect them to want to "dirty" the code with the necessary hack for the broken ABCL. But on the other hand, if CFFI doesn't use the ugly version of the DEFINE-SETF-MODIFY code, the usage of CFFI will break for all ABCL users not running from abcl-1.4.0-dev sources. This week is busy for me, so I would realistically guess that I could make a release of abcl sometime in the next two weeks.

comment:7 in reply to:  6 Changed 10 years ago by Mark Evenson

Replying to mevenson:

In testing your patch, ran into an issue that may or may not have been a result of not doing a complete recompilation. I'll get back to this later today.

After a recompilation of ABCL from scratch, this patch does indeed seem to resolve the underlying issues. I'm running more of the test suite to ensure we haven't broken anything else, afterwards I'll commit Cyrus' patch. Thanks!

comment:8 Changed 10 years ago by Mark Evenson

Resolution: fixed
Status: newclosed

Committed Cyrus's patch as <http://abcl.org/trac/changeset/14727>.

comment:9 Changed 9 years ago by Mark Evenson

Milestone: 1.4.01.3.2
Note: See TracTickets for help on using tickets.