Opened 7 years ago

Last modified 10 months ago

#421 new enhancement

Improve recording of source information for most kinds of definitions

Reported by: Mark Evenson Owned by:
Priority: major Milestone: 1.9.3
Component: interpreter Version: 1.5.0-dev
Keywords: github-issue Cc:
Parent Tickets:

Description

Alan proposes in <https://github.com/armedbear/abcl/pull/5>:

Improve recording of source information for most kinds of definitions
e.g.

(describe 'split-at-char)
SPLIT-AT-CHAR is an internal symbol in the COMMON-LISP-USER package.
Its function binding is #<SPLIT-AT-CHAR {4D969FD4}>.
The function's lambda list is:

(STRING CHAR)

The symbol's property list contains these indicator/value pairs:

SYSTEM::%SOURCE-BY-TYPE ((:COMPILER-MACRO "/Users/alanr/repos/lsw2/util/string.lisp" 1039)

((:FUNCTION SPLIT-AT-CHAR) "/Users/alanr/repos/lsw2/util/string.lisp" 687))

SYSTEM::%SOURCE (#P"/Users/lori/repos/lsw2/util/string.lisp" . 687)

I followed the source types in slime's swank/sbcl.lisp

This is preliminary to adding support for using this information in slime.

What should be recorded:

packages
classes
functions
macros
compiler-macros
setf-expanders
methods
conditions
structures
types
source-transforms
variables
constants

Implementation

fdefinition.lisp has the function record-source-information-for-type For interactive evaluation it does the work. For file compiling there are additions to compile-file.lisp that add forms after the prologue that add the information when fasl loading, during which recording by record-source-information-for-type is disabled, so it doesn't record the source positions in the fasl header.

As you see, I haven't got rid of the now redundant sys:%source property. Once slime has been adjusted it might be worth doing so.

Right now the absolute pathnames are recorded, but it might make sense to use logical pathnames if there were ones set up, though I expect the slime support with do some dwimming in any case.

Change History (15)

comment:1 Changed 7 years ago by Mark Evenson

@alanruttenberg: As I understand your intention after this patch is finished, we will have two properties on symbols with associated source:

Symbol Contents
SYSTEM::%SOURCE (PATHNAME . SOURCE-LINE)
SYSTEM::%SOURCE-BY-TYPE ((KEYWORD-FOR-TYPE-OR-FUNCTION-INFO PATHNAME SOURCE-LINE) …))

where KEYWORD-FOR-TYPE-OR-FUNCTION-INFO would be one of :CONDITION, :VARIABLE, :KEYWORD, :MACRO, :COMPILER-MACRO, :PACKAGE, :DEFSTRUCT, :SETF-EXPANDER, :CONSTANT, :SOURCE-TRANSFORM and for both functions and generic functions a list of the form (:FUNCTION name-of-function)

Once the code works well enough for SLIME, we would no longer need SYSTEM::%SOURCE (assuming that no one else consumes such symbol properties, which is a bit of a stretch, so I would go for an announced deprecation route for abcl-1.6.0 or something).

Shouldn't we take the opportunity to export and normalize the property list key we are using?

Instead of SYSTEM::%SOURCE-BY-TYPE let's move to SYSTEM:SOURCE, which will be the standard interface going forwards.

Uncomment code for arglist?

Why does your code comment out the call to SET-ARGLIST in the precompiler? Does this need additional work, or an oversight on your part?

Inconsistent KEYWORD-FOR-TYPE with DEFMETHOD

Why do functions need to record their names with a cons in place of a simple keyword (probably something basic than I am not getting)?

Shouldn't we distinguish between DEFUN, DEFMETHOD, and DEFGENERIC definitions for functions?

i.e. shouldn't we just replace the (:FUNCTION MOP:ADD-DIRECT-METHOD) with :GENERIC-FUNCTION?

(get 'mop:add-direct-method 'system::%source-by-type)
(((:FUNCTION MOP:ADD-DIRECT-METHOD)

"/Users/evenson/work/abcl.git/src/org/armedbear/lisp/clos.lisp"
100215))

If you agree on the symbol property key being 'SYSTEM:SOURCE', and can explain the bit about function source location using a cons to record its type, I'd go for merging this as a work in progress.

comment:2 Changed 7 years ago by Mark Evenson

Alan replies:

I had some trouble building with the arglist being set there. I think it is set elsewhere. Try getting arglists in the new build.

mop:add-direct-method is at least defined with defun at that location. It is later defined using atomic-defgeneric and looking at the expansion of that I can see that it is doing some funny business. It will need to be special-cased. It defines the generic function on a gensym.

;;; To be redefined as generic functions later
(defun add-direct-method (specializer method) ...

(describe 'print-object) to see methods and generic-functions recorded

  ((:METHOD PRINT-OBJECT NIL (STANDARD-OBJECT T))
   "/Users/lori/repos/abcl/src/org/armedbear/lisp/print-object.lisp" 1878)
  ((:METHOD PRINT-OBJECT NIL (T T))
   "/Users/lori/repos/abcl/src/org/armedbear/lisp/print-object.lisp" 1715)
  ((:GENERIC-FUNCTION PRINT-OBJECT)
   "/Users/lori/repos/abcl/src/org/armedbear/lisp/print-object.lisp" 1672))

Functions are defined as cons because sometimes they are the symbol and sometimes they are the (setf xxx). In the latter case the source information is also kept on xxx.

e.g.

(defun (setf jfield) (newvalue class-ref-or-field field-or-instance
              &optional (instance nil instance-supplied-p) unused-value)
  (declare (ignore unused-value))
  (if instance-supplied-p
      (jfield class-ref-or-field field-or-instance instance newvalue)
      (jfield class-ref-or-field field-or-instance nil newvalue)))

:defstruct should be :structure. I'm trying to follow this bit in swank/sbcl.lisp. Will fix.

(defparameter *definition-types*
  '(:variable defvar
    :constant defconstant
    :type deftype
    :symbol-macro define-symbol-macro
    :macro defmacro
    :compiler-macro define-compiler-macro
    :function defun
    :generic-function defgeneric
    :method defmethod
    :setf-expander define-setf-expander
    :structure defstruct
    :condition define-condition
    :class defclass
    :method-combination define-method-combination
    :package defpackage
    :transform :deftransform
    :optimizer :defoptimizer
    :vop :define-vop
    :source-transform :define-source-transform
    :ir1-convert :def-ir1-translator
    :declaration declaim
    :alien-type :define-alien-type)
  "Map SB-INTROSPECT definition type names to Slime-friendly forms")

comment:3 Changed 7 years ago by Mark Evenson

Arglist initialization wasn't working, so the call to attempt to record arglist <https://github.com/armedbear/abcl/pull/5/files#diff-f5d0489130c637f85b74ad605012e18eR1183> documentation was left commented out in the resolution of the issue, to be added TODO.

Last edited 7 years ago by Mark Evenson (previous) (diff)

comment:4 Changed 7 years ago by Mark Evenson

Milestone: 1.5.01.6.0

Ticket retargeted after milestone closed

comment:5 Changed 4 years ago by Mark Evenson

Milestone: 1.6.01.6.1

Ticket retargeted after milestone closed

comment:6 Changed 4 years ago by Mark Evenson

Milestone: 1.6.11.6.2

Ticket retargeted after milestone closed

comment:7 Changed 4 years ago by Mark Evenson

Milestone: 1.6.21.7.0

comment:8 Changed 4 years ago by Mark Evenson

Milestone: 1.7.01.7.1

Ticket retargeted after milestone closed

comment:9 Changed 4 years ago by Mark Evenson

Milestone: 1.7.11.7.2

Ticket retargeted after milestone closed

comment:10 Changed 4 years ago by Mark Evenson

Milestone: 1.7.21.8.0

Milestone renamed

comment:11 Changed 3 years ago by Mark Evenson

Milestone: 1.8.01.8.1

Ticket retargeted after milestone closed

comment:12 Changed 2 years ago by Mark Evenson

Milestone: 1.8.11.9.0

comment:13 Changed 15 months ago by Mark Evenson

Milestone: 1.9.01.9.1

comment:14 Changed 14 months ago by Mark Evenson

Milestone: 1.9.11.9.2

comment:15 Changed 10 months ago by Mark Evenson

Milestone: 1.9.21.9.3
Note: See TracTickets for help on using tickets.