I've started testing ASDF 3.3.1.3 with my group's code on SBCL 1.4.4
and noticed some issues with uiop:define-package (due to commit
8281e011).

First, when compiling ASDF I get 456 compilation notes
(https://pastebin.com/NnRUKGWe). I get the same notes when using
uiop:define-package in our code as well. I honestly think this is an
issue of SBCL being over aggressive. It's also odd because if the
recording of the source location is removed, then the notes aren't
produced. It appears this started happening in SBCL 1.4.1 (potentially
due to commit af5e2ed1e).

The volume of notes (especially when using package-inferred-system)
can drown out real issues. I'll likely report this as an issue to SBCL
since the keywords *should* be constant and the source transform for
apply doesn't seem to be preserving that. However, it could also be
fixed in ASDF by changing the apply to a funcall in the define-package
expansion (patch 2).

Second, the comma on (sb-c:source-location) seems to cause the source
location for the package to always point to the body of the
define-package macro. Patch 1 changes this so that form is evaluated
after the macro is expanded. I didn't test this on anything earlier
than SBCL 1.4.4, but I don't believe this behavior is version
dependent.

-Eric
From 79baf14021d8940ef2a969359fa335e72d8fad57 Mon Sep 17 00:00:00 2001
From: Eric Timmons <etimm...@mit.edu>
Date: Fri, 16 Feb 2018 23:59:49 -0500
Subject: [PATCH 1/2] Evaluate sb-c:source-location after macroexpansion

Tested on SBCL 1.4.4. If sb-c:source-location is evaluated during macro
expansion, then the source location will always point to asdf.lisp (inside the
define-package macro). If it is evaluated after macro expansion, it points to
the right place.
---
 uiop/package.lisp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/uiop/package.lisp b/uiop/package.lisp
index 4a6d38e1..97981752 100644
--- a/uiop/package.lisp
+++ b/uiop/package.lisp
@@ -735,7 +735,7 @@ (defmacro define-package (package &rest clauses)
          `(prog1
               (apply 'ensure-package ',(parse-define-package-form package clauses))
             #+sbcl (setf (sb-impl::package-source-location (find-package ',package))
-                         ,(sb-c:source-location)))))
+                         (sb-c:source-location)))))
     `(progn
        #+(or clasp ecl gcl mkcl) (defpackage ,package (:use))
        (eval-when (:compile-toplevel :load-toplevel :execute)
-- 
2.16.1

From 8d887763cab34b32f2aca0e0a655c3017f80be9c Mon Sep 17 00:00:00 2001
From: Eric Timmons <etimm...@mit.edu>
Date: Sat, 17 Feb 2018 00:04:42 -0500
Subject: [PATCH 2/2] Change apply to funcall in expansion of define-package

SBCL has been getting aggresive with checking arguments to functions. For some
reason (as of SBCL 1.4.1), setting the package source location in define-package
seems to trigger a source translation for the apply form that ends up producing
(many) compilation notes that the arguments to ensure-package in the keyword
positions are not constant, weakening keyword argument checking. We can get
around that, however, by using a funcall directly.
---
 uiop/package.lisp | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/uiop/package.lisp b/uiop/package.lisp
index 97981752..c2dd14da 100644
--- a/uiop/package.lisp
+++ b/uiop/package.lisp
@@ -700,13 +700,13 @@ (defun parse-define-package-form (package clauses)
         :and :do (setf use-p t) :else
       :when (eq kw :unintern) :append args :into unintern :else
         :do (error "unrecognized define-package keyword ~S" kw)
-      :finally (return `(,package
-                         :nicknames ,nicknames :documentation ,documentation
-                         :use ,(if use-p use '(:common-lisp))
-                         :shadow ,shadow :shadowing-import-from ,shadowing-import-from
-                         :import-from ,import-from :export ,export :intern ,intern
-                         :recycle ,(if recycle-p recycle (cons package nicknames))
-                         :mix ,mix :reexport ,reexport :unintern ,unintern)))))
+      :finally (return `(',package
+                         :nicknames ',nicknames :documentation ',documentation
+                         :use ',(if use-p use '(:common-lisp))
+                         :shadow ',shadow :shadowing-import-from ',shadowing-import-from
+                         :import-from ',import-from :export ',export :intern ',intern
+                         :recycle ',(if recycle-p recycle (cons package nicknames))
+                         :mix ',mix :reexport ',reexport :unintern ',unintern)))))
 
 (defmacro define-package (package &rest clauses)
   "DEFINE-PACKAGE takes a PACKAGE and a number of CLAUSES, of the form
@@ -733,7 +733,7 @@ (defmacro define-package (package &rest clauses)
 UNINTERN -- Remove symbols here from PACKAGE."
   (let ((ensure-form
          `(prog1
-              (apply 'ensure-package ',(parse-define-package-form package clauses))
+              (funcall 'ensure-package ,@(parse-define-package-form package clauses))
             #+sbcl (setf (sb-impl::package-source-location (find-package ',package))
                          (sb-c:source-location)))))
     `(progn
-- 
2.16.1

Reply via email to