0.9.18.12: valid/already-dumped confusion in the file compiler/

 * A constant is not already dumped just because it is in the valid
   table.
 * Fopcompiler was validating the wrong object occasionally.
   Unfortunately the *DUMP-ONLY-VALID-STRUCTURES* binding still
   needed. Couple of FIXME's pertaining to that added.
 * Useless WHEN in EMIT-MAKE-LOAD-FORM deleted.
 * Record a bug.

diff --git a/BUGS b/BUGS
index 070a9a3..963ff68 100644 (file)
--- a/BUGS
+++ b/BUGS
@@ -1713,3 +1713,13 @@ WORKAROUND:
   causes a TYPE-ERROR 
     The value NIL is not of type SB-C::PHYSENV.
   in MERGE-LETS.
+
+406: functional has external references -- failed aver
+ Given the following food in a single file
+  (eval-when (:compile-toplevel :load-toplevel :execute)
+    (defstruct foo3))
+  (defstruct bar
+    (foo #.(make-foo3)))
+ as of 0.9.18.11 the file compiler breaks on it:
+  failed AVER: "(NOT (FUNCTIONAL-HAS-EXTERNAL-REFERENCES-P CLAMBDA))"
+ Defining the missing MAKE-LOAD-FORM method makes the error go away.

diff --git a/NEWS b/NEWS
index b1ccac8..98e4471 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -5,7 +5,9 @@ changes in sbcl-0.9.19 (1.0.0?) relative to sbcl-0.9.18:
   * improvement: GET-INTERNAL-REAL-TIME now reports the time since
     startup, not time since first call to GET-INTERNAL-REAL-TIME.
   * bug fix: compiler bug triggered by a (non-standard) VALUES
-    declaration in a LET* was fixed.
+    declaration in a LET* was fixed. (reported by Kaersten Poeck)
+  * bug fix: file compiler no longer confuses validated and already
+    dumped structurres. (reported by Kaersten Poeck)
   * improvements to the Windows port:
     ** floating point exceptions are now reported correctly.
     ** stack exhaustion detection works partially.

diff --git a/src/compiler/dump.lisp b/src/compiler/dump.lisp
index 4d136f3..a296451 100644 (file)
--- a/src/compiler/dump.lisp
+++ b/src/compiler/dump.lisp
@@ -498,13 +498,15 @@
     (dump-byte 0 file))
   (dump-pop file))
 
-;;; Return T iff CONSTANT has not already been dumped. It's been
-;;; dumped if it's in the EQ table.
+;;; Return T iff CONSTANT has already been dumped. It's been dumped if
+;;; it's in the EQ table.
+;;;
+;;; Note: historically (1) the above comment was "T iff ... has not been dumped",
+;;; (2) the test was  was also true if the constant had been validated / was in
+;;; the valid objects table. This led to substructures occasionally skipping the
+;;; validation, and hence failing the "must have been validated" test.
 (defun fasl-constant-already-dumped-p (constant file)
-  (if (or (gethash constant (fasl-output-eq-table file))
-          (gethash constant (fasl-output-valid-structures file)))
-      t
-      nil))
+  (and (gethash constant (fasl-output-eq-table file)) t))
 
 ;;; Use HANDLE whenever we try to dump CONSTANT. HANDLE should have been
 ;;; returned earlier by FASL-DUMP-LOAD-TIME-VALUE-LAMBDA.

diff --git a/src/compiler/fopcompile.lisp b/src/compiler/fopcompile.lisp
index c0dfd12..7f18013 100644 (file)
--- a/src/compiler/fopcompile.lisp
+++ b/src/compiler/fopcompile.lisp
@@ -196,7 +196,11 @@
                       (declare (ignore init-form))
                       (case creation-form
                         (:sb-just-dump-it-normally
-                         (fasl-validate-structure constant *compile-object*)
+                         ;; FIXME: Why is this needed? If the constant
+                         ;; is deemed fopcompilable, then when we dump
+                         ;; it we bind *dump-only-valid-structures* to
+                         ;; NIL.
+                         (fasl-validate-structure value *compile-object*)
                          (dotimes (i (- (%instance-length value)
                                         (layout-n-untagged-slots
                                          (%instance-ref value 0))))
@@ -374,5 +378,9 @@
 
 (defun fopcompile-constant (form for-value-p)
   (when for-value-p
+    ;; FIXME: Without this binding the dumper chokes on unvalidated
+    ;; structures: CONSTANT-FOPCOMPILABLE-P validates the structure
+    ;; about to be dumped, not its load-form. Compare and contrast
+    ;; with EMIT-MAKE-LOAD-FORM.
     (let ((sb!fasl::*dump-only-valid-structures* nil))
       (dump-object form *compile-object*))))

diff --git a/src/compiler/main.lisp b/src/compiler/main.lisp
index 8e4e154..2fe6b17 100644 (file)
--- a/src/compiler/main.lisp
+++ b/src/compiler/main.lisp
@@ -1795,8 +1795,6 @@ SPEED and COMPILATION-SPEED optimization values, and the
         (:ignore-it
          nil)
         (t
-         (when (fasl-constant-already-dumped-p constant *compile-object*)
-           (return-from emit-make-load-form nil))
          (let* ((name (write-to-string constant :level 1 :length 2))
                 (info (if init-form
                           (list constant name init-form)

diff --git a/tests/compiler.test.sh b/tests/compiler.test.sh
index daa3fff..f7c9218 100644 (file)
--- a/tests/compiler.test.sh
+++ b/tests/compiler.test.sh
@@ -355,6 +355,22 @@ cat > $tmpfilename <<EOF
 EOF
 expect_clean_compile $tmpfilename
 
+cat > $tmpfilename <<EOF
+(defstruct foo
+  (bar #p"/tmp/"))
+EOF
+expect_clean_compile $tmpfilename
+
+cat > $tmpfilename <<EOF
+(eval-when (:compile-toplevel :load-toplevel :execute)
+  (defstruct foox)
+  (defmethod make-load-form ((foo foox) &optional env)
+    `(make-foox)))
+(defstruct bar
+  (foo #.(make-foox)))
+EOF
+expect_clean_compile $tmpfilename
+
 rm $tmpfilename
 rm $compiled_tmpfilename
 

diff --git a/version.lisp-expr b/version.lisp-expr
index 8bc6cc2..56957d8 100644 (file)
--- a/version.lisp-expr
+++ b/version.lisp-expr
@@ -17,4 +17,4 @@
 ;;; checkins which aren't released. (And occasionally for internal
 ;;; versions, especially for internal versions off the main CVS
 ;;; branch, it gets hairier, e.g. "0.pre7.14.flaky4.13".)
-"0.9.18.11"
+"0.9.18.12"