Re: SWANK defines defstructs in the keyword package

Martin Simmons <[email protected]> Mon, 3 Jul 2017 18:41:00 +0100
Newsgroups gmane.lisp.slime.devel
Message-ID <[email protected]>
Thanks, I've attached a patch that works for LispWorks, but I think only SBCL
uses the readers.

>>>>> On Mon, 3 Jul 2017 20:04:46 +0300, Stas Boukarev said:
> 
> I'd apply a patch that performs this.
> 
> On Mon, Jul 3, 2017 at 7:52 PM, Martin Simmons <[email protected]> wrote:
> > Hi,
> >
> > swank/backend.lisp defines some defstructs in the keyword package:
> >
> > (defstruct (:location (:type list) :named
> >                       (:constructor make-location
> >                                     (buffer position &optional hints)))
> >   buffer position
> >   ;; Hints is a property list optionally containing:
> >   ;;   :snippet SOURCE-TEXT
> >   ;;     This is a snippet of the actual source text at the start of
> >   ;;     the definition, which could be used in a text search.
> >   hints)
> >
> > (defstruct (:error (:type list) :named (:constructor)) message)
> >
> > ;;; Valid content for BUFFER slot
> > (defstruct (:file       (:type list) :named (:constructor)) name)
> > (defstruct (:buffer     (:type list) :named (:constructor)) name)
> > (defstruct (:etags-file (:type list) :named (:constructor)) filename)
> >
> > ;;; Valid content for POSITION slot
> > (defstruct (:position (:type list) :named (:constructor)) pos)
> > (defstruct (:tag      (:type list) :named (:constructor)) tag1 tag2)
> >
> >
> > This is generally a bad idea because it can lead to clashes in the namespace
> > of structure names (as used by the :include option).
> >
> > AFICS, except for "location", these definitions are never used because they
> > are always constructed using backquote or list.
> >
> > Would you consider removing the defstructs and adding defuns for make-location
> > and its readers?
> >
> > --
> > Martin Simmons
> > LispWorks Ltd
> > http://www.lispworks.com/
> >
> 
> 
> 
> -- 
> With best regards, Stas.
>
remove-keyword-defstructs.patch (text/plain, 1.7 KB)
diff --git a/swank/backend.lisp b/swank/backend.lisp
index 66c104e..e4c536d 100644
--- a/swank/backend.lisp
+++ b/swank/backend.lisp
@@ -994,26 +994,26 @@ returns.")
 
 ;;;; Definition finding
 
-(defstruct (:location (:type list) :named
-                      (:constructor make-location
-                                    (buffer position &optional hints)))
-  buffer position
+(defun make-location (buffer position &optional hints)
+  ;; Possible content for BUFFER:
+  ;;   (:file name)
+  ;;   (:buffer name)
+  ;;   (:etags-file filename)
+  ;; Possible content for POSITION:
+  ;;   (:position pos)
+  ;;   (:tag tag1 tag2)
   ;; Hints is a property list optionally containing:
   ;;   :snippet SOURCE-TEXT
   ;;     This is a snippet of the actual source text at the start of
   ;;     the definition, which could be used in a text search.
-  hints)
-
-(defstruct (:error (:type list) :named (:constructor)) message)
-
-;;; Valid content for BUFFER slot
-(defstruct (:file       (:type list) :named (:constructor)) name)
-(defstruct (:buffer     (:type list) :named (:constructor)) name)
-(defstruct (:etags-file (:type list) :named (:constructor)) filename)
-
-;;; Valid content for POSITION slot
-(defstruct (:position (:type list) :named (:constructor)) pos)
-(defstruct (:tag      (:type list) :named (:constructor)) tag1 tag2)
+  `(:location ,buffer ,position ,hints))
+
+(defun location-buffer (location)
+  (nth 1 location))
+(defun location-position (location)
+  (nth 2 location))
+(defun location-hints (location)
+  (nth 3 location))
 
 (defmacro converting-errors-to-error-location (&body body)
   "Catches errors during BODY and converts them to an error location."