[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Another EQ problem

    Date: 26 May 88 11:01 EST
    From:     STERRITT%SDEVAX.decnet@ge-crd.arpa

            I think that the following is a bug that should be changed:
            I have some code that does surgery (setf's of nth's) on lists in
    a Defstruct.  Since some of the fields are identical, the lists are eq.
    This means (obviously, once you know about it) that if you change slot A,
    slot B is smashed too!
    (defstruct foo
            (a '(0.0 0.0))
            (b '(0.0 0.0)))
    ;;compile, then:
    (setf bar (make-foo))
    (setf (nth 0 (foo-a bar)) 4.5)
    (foo-a bar)
    -> (4.5 0.0)
    (foo-b bar)
    -> (4.5 0.0)

            I can understand doing constant folding on let-bound vars, but I'm
    not so sure about struct slots.  The usual fix of changing it to:
    (defstruct foo
            (a (list 0.0 0.0))
            (b (list 0.0 0.0)))
    fixes the symptoms -- but I do think it's a bug.

1IF0 you agree that the semantics of a LISP program is that it's made up of LISP
   objects (rather than the printed representations thereof) which are
   interpreted by 1eval0; and compiling preserves those semantics, but may also
   happen to make the program faster
        Semantics Summary:
        I have a function, 1eval0, and I'm passing it a piece of list structure.
1        eval0 uses the list structure to decide what to do.  It does it.
0  Destructively modifying a quoted list is, in effect, modifying the list
  structure which makes up the source of your program.

I've rarely seen valid uses of self-modifying code.  It makes error recovery,
debugging, and restarting 2very0 difficult.  It also makes it harder to allow the
compiler to do things like put constants [including the compiled code itself!]
into read-only or shared memory.

Students in LISP courses we teach often use quoted constants where they mean
to CONS up a fresh list every time.  One class wrote a hangman program, where
a gallows was represented as a list of body parts.  They wrote:

        (defun make-gallows ()
          '(*header* head body arms legs))

later, they wrote:

        (defun remove-part-from-gallows (gallows)
          (setf (cdr gallows) (cddr gallows)))

The program did:
        (let ((gallows (make-gallows)))
          (remove-part-from-gallows gallows)

The bug they discovered was, 2the second time they ran the program0,
the new gallows was missing parts.  Because in effect, the SETF actually
changed the source code of 1make-gallows0.

By the way, notice that in the above example, the quoted lists are part of the
source code for the 1defstruct0.  2Regardless0 of whether the lists are collapsed,
when you destructively modify one, it modifies the 1defstruct0 form.

Watch what happens if you make a second FOO:

Command: (setf bar (make-foo))
#S(FOO :A (0.0 0.0)
       :B (0.0 0.0))
Command: (setf (nth 0 (foo-a bar)) 4.5)
Command: (foo-a bar)
(4.5 0.0)
Command: (foo-b bar)
(4.5 0.0)
Command: (setf baz (make-foo))
1#S(FOO :A (4.5 0.0)
       :B (4.5 0.0))
0Command: [Abort]


I think of this more as an issue of understanding that a quoted list is,
actually, part of the source code of your program.  Modifying one is like
changing the program text on the fly.  Which, for debuggability, readability,
etc., is a dangerous thing to do.

[Imagine the above example in production code.  You're suddenly creating FOOs
whose initial contents seem only marginally related to the initial values that
the 1defstruct0 says they should be.  And the contents certainly don't seem
related to the SETF you did on some instance, ages ago.]

Whether the compiler has license to collapse constants in code is, to me, a
secondary issue.  [One which a compiler option like (DECLARE
DONT-COLLAPSE-CONSTANTS) would take care of].

- Stephen

P.S.  For those of you running release 7.2 or later, a diagram may
      help...  Evaluating (and remember, compilation preseves evaluation

                (defun make-foo ()
                  '(1 2))

      Is actually passing this to 1eval0: