Lesson learned: never modify literals, quote lists only for constant data

2011-02-01

While writing a scopa card game in Common Lisp both as a pet project and a way to practice a little lisp, I had a hard time trying to understand what was going on here:

(defun prime-points-of-card (card)
    (let ((prime-points '((7 21) (6 18) (1 16) (5 15) (4 14) (3 13) (2 12) (10 10) (9 10) (8 10))))
    (cadr (assoc (card-rank card) prime-points))))

(defun prime-points (deck)
    (let ((scores '((denari 0) (coppe 0) (bastoni 0) (spade 0))))
    (loop for card across deck do
            (let ((suit (card-suit card)))
            (setf (cadr (assoc suit scores)) (max (cadr (assoc suit scores))
                                                    (prime-points-of-card card)))))
    (apply #'+ (mapcar #'cadr scores))))

I was expecting prime-points to return the prime points of a sequence of cards, but it kept returning what seemed to me just meaningless numbers.

I added a couple of prints to inspect what was going on:
(yeah I still need to learn how to debug effectively with lisp)

(defun prime-points (deck)
  (let ((scores '((denari 0) (coppe 0) (bastoni 0) (spade 0))))
    (print scores)
    (loop for card across deck do
      (let ((suit (card-suit card)))
        (setf (cadr (assoc suit scores)) (max (cadr (assoc suit scores))
                                         (prime-points-of-card card)))))
    (print scores)
    (apply #'+ (mapcar #'cadr scores))))

and this is what I found:

[3]> (prime-points #())

((DENARI 0) (COPPE 0) (BASTONI 0) (SPADE 0))
((DENARI 0) (COPPE 0) (BASTONI 0) (SPADE 0))
0
[4]> (prime-points #())

((DENARI 0) (COPPE 0) (BASTONI 0) (SPADE 0))
((DENARI 0) (COPPE 0) (BASTONI 0) (SPADE 0))
0
[5]> (prime-points (deck 'stock))

((DENARI 0) (COPPE 0) (BASTONI 0) (SPADE 0))
((DENARI 21) (COPPE 21) (BASTONI 21) (SPADE 21))
84
[6]> (prime-points #())

((DENARI 21) (COPPE 21) (BASTONI 21) (SPADE 21))
((DENARI 21) (COPPE 21) (BASTONI 21) (SPADE 21))
84

scores was keeping its value at each call to prime-points!
Wasn’t let supposed to create a new lexical scope?
Yes, it is and in fact it is what let does.
The problem is to understand that

(quote (foo bar baz))

and its shortcut

'(foo bar baz)

are just list literals the same way as

#(1 2 3) ; Literal array
"literal string"

This means that

(quote (foo bar baz))

is not equivalent to

(list 'foo 'bar 'baz)

In the first case, the list is literal, in the second it is generated by code.
When the list is literal, the compiler could copy it into write-protected memory or share it among more functions (or more calls to the same function) hence the consequences are undefined if it is modified.

As HyperSpec’s QUOTE page states:

Special Operator QUOTE

Syntax: quote object => object

Arguments and Values: object—an object; not evaluated.

Description: The quote special operator just returns object.

The consequences are undefined if literal objects (including quoted objects) are destructively modified.

Ok. This may seem trivial for anybody else, but it was not for me.

This is the same thing that happens in other languages.

#include <stdio.h>

void foo() {
    char string[] = "Fresh new at every call";

    printf("%s\n", string);
    string[0]++;
    printf("%s\n", string);
}

void bar() {
    char* string = "I could be in read-only memory";

    printf("%s\n", string);
    string[0]++; // Possible segmentation fault
    printf("%s\n", string);
}

int main(int argc, char **argv) {
    foo();
    foo();
    foo();

    bar();
    bar();
    bar();

    return 0;
}

This is what happens with gcc:

dom@dom-laptop:~/Progetti/c$ gcc -o test test.c && ./test
Fresh new at every call
Gresh new at every call
Fresh new at every call
Gresh new at every call
Fresh new at every call
Gresh new at every call
I could be in read-only memory
Errore di segmentazione
dom@dom-laptop:~/Progetti/c$

So if you need to define a data structure the way I did, use list or just copy-tree the literal data:

(defun prime-points (deck)
  (let ((scores (copy-tree '((denari 0) (coppe 0) (bastoni 0) (spade 0)))))
    (loop for card across deck do
      (let ((suit (card-suit card)))
        (setf (cadr (assoc suit scores)) (max (cadr (assoc suit scores))
                                              (prime-points-of-card card)))))
(apply #'+ (mapcar #'cadr scores))))

I need to thank people at comp.lang.lisp for always being friendly and very didactic to lurk at.