Opened 8 years ago
Last modified 16 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 8 years ago by
comment:2 Changed 8 years ago by
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 8 years ago by
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.
comment:7 Changed 4 years ago by
Milestone: | 1.6.2 → 1.7.0 |
---|
comment:12 Changed 2 years ago by
Milestone: | 1.8.1 → 1.9.0 |
---|
comment:13 Changed 20 months ago by
Milestone: | 1.9.0 → 1.9.1 |
---|
comment:14 Changed 19 months ago by
Milestone: | 1.9.1 → 1.9.2 |
---|
comment:15 Changed 16 months ago by
Milestone: | 1.9.2 → 1.9.3 |
---|
@alanruttenberg: As I understand your intention after this patch is finished, we will have two properties on symbols with associated source:
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).
Instead of SYSTEM::%SOURCE-BY-TYPE let's move to SYSTEM:SOURCE, which will be the standard interface going forwards.
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?
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)
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.