diff options
| -rw-r--r-- | consfigurator.asd | 4 | ||||
| -rw-r--r-- | debian/control | 4 | ||||
| -rw-r--r-- | doc/introduction.rst | 7 | ||||
| -rw-r--r-- | doc/pitfalls.rst | 31 | ||||
| -rw-r--r-- | src/property.lisp | 14 | ||||
| -rw-r--r-- | src/propspec.lisp | 163 |
6 files changed, 106 insertions, 117 deletions
diff --git a/consfigurator.asd b/consfigurator.asd index e518632..87f4951 100644 --- a/consfigurator.asd +++ b/consfigurator.asd @@ -14,8 +14,8 @@ #:cffi (:feature :sbcl (:require #:sb-posix)) #:closer-mop - #:trivial-backtrace - #:trivial-macroexpand-all) + #:agnostic-lizard + #:trivial-backtrace) :components ((:file "src/package") (:file "src/reader") (:file "src/util") diff --git a/debian/control b/debian/control index e5fae5d..ca26690 100644 --- a/debian/control +++ b/debian/control @@ -12,7 +12,7 @@ Build-Depends: cl-ppcre, cl-closer-mop, cl-trivial-backtrace, - cl-trivial-macroexpand-all, + cl-agnostic-lizard, debhelper-compat (= 13), dh-elpa, python3-sphinx, @@ -37,7 +37,7 @@ Depends: cl-ppcre, cl-closer-mop, cl-trivial-backtrace, - cl-trivial-macroexpand-all, + cl-agnostic-lizard, emacsen-common, ${misc:Depends}, Recommends: diff --git a/doc/introduction.rst b/doc/introduction.rst index 656e7eb..4275996 100644 --- a/doc/introduction.rst +++ b/doc/introduction.rst @@ -290,10 +290,9 @@ Portability and stability - All of the code in the core library should be portable ANSI Common Lisp, though optional packages providing properties and connection types might use - implementation-specific functionality. There is one exception: we require - an implementation of ``MACROEXPAND-ALL``, but most Lisps in use today - provide this. Little to no testing is done by the author on implementations - other than SBCL, so testing and portability patches are welcome. + implementation-specific functionality. Little to no testing is done by the + author on implementations other than SBCL, so testing and portability + patches are welcome. - Little attempt is made by the author to support systems other than Debian GNU/Linux, but again, portability patches are welcome, and the design of diff --git a/doc/pitfalls.rst b/doc/pitfalls.rst index 7a7bbae..b0610eb 100644 --- a/doc/pitfalls.rst +++ b/doc/pitfalls.rst @@ -48,3 +48,34 @@ serialisable, so you can't pass anonymous functions or objects containing those. You can work around the latter restriction by defining a new property which passes in the desired anonymous function, and then adding the new property to your property application specification. + +Code-walking limitations +------------------------ + +The preprocessing of propspecs, and the conversion of unevaluated propspecs +into propspecs, both require code walking. Consfigurator's implementation of +this is in the function ``MAP-PROPSPEC-PROPAPPS``. However, due to +limitations in the Common Lisp standard, it is not possible to implement the +work of that function in a way which is both always correct and fully +portable. I have not found a general purpose code walker which hooks into +implementation-specific functionality and that is currently maintained, and so +at present we use a best-effort portable code walker, Agnostic Lizard. + +This will almost always generate the correct expansions, but if you have +particularly advanced macro property combinators then it is possible that +``MAP-PROPSPEC-PROPAPPS`` will return incorrectly expanded forms. For full +details see Michael Raskin. 2017. "Writing a best-effort portable code +walker in Common Lisp." In *Proceedings of 10th European Lisp Symposium*, +Vrije Universiteit Brussel, Belgium, April 2017 (ELS2017). DOI: +10.5281/zenodo.3254669. + +It is possible to implement the work of ``MAP-PROPSPEC-PROPAPPS`` in terms of +``MACROEXPAND-ALL``, whose semantics are conventionally well-understood and +for which fully correct implementations are available in most implementations +of Common Lisp (the trivial-macroexpand-all library can be used to get at +these). However, note that we cannot just call ``MACROEXPAND-ALL`` on +propspecs because unquoted lists appearing as arguments to properties in +atomic property applications will look like invalid function calls to the code +walker. Avoiding this would seem to require wrapping the propspec in one +macrolet for each known property, and this makes ``MACROEXPAND-ALL`` too slow, +even if the macrolet forms are precomputed. diff --git a/src/property.lisp b/src/property.lisp index 8b38db9..c6512cd 100644 --- a/src/property.lisp +++ b/src/property.lisp @@ -133,22 +133,10 @@ (defvar *known-properties* nil "All properties whose definitions have been loaded.") -(defvar *known-property-macrolets* nil - "Macro definitions for all known properties as used in MAP-PROPSPEC-PROPAPPS. - -This variable exists just to avoid consing these forms over and over again; -see MAP-PROPSPEC-PROPAPPS for how they are used.") - (defun record-known-property (psym) (unless (get psym 'isprop) (setf (get psym 'isprop) t) - (push psym *known-properties*) - (push `(,psym (&rest args) - (let ((gensym (gensym))) - (push (list* gensym ',psym args) - *replaced-propapps*) - gensym)) - *known-property-macrolets*))) + (push psym *known-properties*))) (defun dump-properties-for-emacs (from to) (let ((put-forms diff --git a/src/propspec.lisp b/src/propspec.lisp index 92a73e8..81e50a2 100644 --- a/src/propspec.lisp +++ b/src/propspec.lisp @@ -20,32 +20,31 @@ ;;;; Property application specifications -(define-condition ambiguous-propspec (undefined-function) ()) - -(define-condition invalid-or-ambiguous-propspec (error) - ((original-error :initarg :error :reader original-error) - (broken-propspec :initarg :propspec :reader broken-propspec)) +(define-condition ambiguous-propspec (undefined-function) () (:report (lambda (condition stream) (format stream -"MACROEXPAND-ALL could not process the following propspec. This can happen -because the propspec is invalid, or because it contains references to -properties whose definitions have not been loaded. + "The function, property or property combinator ~A is undefined. Ensure that all functions, properties and property combinators used in a -propspec are defined before that propspec is processed by Consfigurator. +propspec are defined before that propspec is processed by Consfigurator." + (cell-error-name condition))))) -~S" (broken-propspec condition)) +(define-condition invalid-propspec (error) + ((original-error :initarg :error :reader original-error) + (broken-propspec :initarg :propspec :reader broken-propspec)) + (:report + (lambda (condition stream) + (format + stream + "The code walker could not process the following propspec.~%~%~S" + (broken-propspec condition)) (when (slot-boundp condition 'original-error) (format stream "~&~%The error from the code walker was:~%~%~A" (original-error condition)))))) -(defvar *replaced-propapps* nil - "Internal dynamic variable used in MAP-PROPSPEC-PROPAPPS.") - -(defun map-propspec-propapps - (function propspec &optional reconstruct env &aux *replaced-propapps*) +(defun map-propspec-propapps (function propspec &optional reconstruct env) "Map FUNCTION over each propapp occurring in PROPSPEC after macroexpansion. FUNCTION designates a pure function from propapps to propapps. PROPSPEC is a property application specification expression. @@ -54,87 +53,59 @@ RECONSTRUCT is a boolean flag indicating whether to return code which will evaluate to the resultant propspec rather than that propspec itself; if t, FUNCTION too should return code which will evaluate to propapps rather than propapps themselves. This is useful for when this function is called by -macros. ENV is the ENV argument to be passed along to MACROEXPAND-ALL. - -Note that this implementation will fail to map propapps appearing within the -arguments to properties in propapps, but that should not be needed." - ;; The work of this function cannot be implemented fully portably. See - ;; - ;; Michael Raskin. 2017. Writing a best-effort portable code walker in - ;; Common Lisp. In Proceedings of 10th European Lisp Symposium, Vrije - ;; Universiteit Brussel, Belgium, April 2017 (ELS2017). - ;; DOI: 10.5281/zenodo.3254669 - ;; - ;; for why. However, it can be implemented in terms of MACROEXPAND-ALL, - ;; whose semantics are conventionally well-understood and which is available - ;; in most implementations of Common Lisp (we use the - ;; trivial-macroexpand-all library to get at these implementations). - (labels - ((macrolet-and-expand (macrolets form) - (multiple-value-bind (expanded supported env-supported) - (trivial-macroexpand-all:macroexpand-all - `(macrolet ,macrolets ,form) env) - (unless supported - (error "Don't know how to MACROEXPAND-ALL in this Lisp.")) - (when (and env (not env-supported)) - (error "Don't know how to MACROEXPAND-ALL with env in this Lisp.")) - ;; At least SB-CLTL2:MACROEXPAND-ALL leaves the MACROLET in, so use - ;; CADDR to remove it again -- if that turns out to be - ;; implementation-specific, we can look for what we added and - ;; remove it. - ;; - ;; This is not just to avoid leaking our implementation to our - ;; callers -- if we call this function more than once with old - ;; calls to MACROLET left in, we can get stuck in infinite macro - ;; expansion loops. - (caddr expanded))) - (walk (tree) - (if (atom tree) - (if-let ((propapp (gethash tree *replaced-propapps*))) - (funcall function propapp) - (if (and reconstruct (symbolp tree)) `',tree tree)) - (let ((walked (mapcar #'walk tree))) - (if reconstruct (cons 'list walked) walked))))) - ;; First we need to find all the propapps, after macro expansion. - ;; Propapps contain the arguments to be passed to properties rather than - ;; expressions which will evaluate to those arguments, and some of these - ;; might be lists, which will look like invalid function calls to the code - ;; walker. So we macrolet every known property so that the code walker - ;; does not assume these arguments are to be evaluated as arguments to - ;; ordinary functions are. - ;; - ;; We can't just set up the macrolets to map FUNCTION over the propapp and - ;; return the result because if FUNCTION returns a propapp whose car is - ;; the same (as indeed it often will be) then we would get stuck in an - ;; infinite macro expansion. So we substitute back and forth for gensyms. - (let ((expanded - (handler-case - (macrolet-and-expand *known-property-macrolets* propspec) - (error (condition) - (error 'invalid-or-ambiguous-propspec :error condition - :propspec propspec))))) - ;; Now we use a dummy macro expansion pass to find any symbols without - ;; function or property definitions occurring in function call - ;; positions. These could potentially be properties whose definitions - ;; have not been loaded -- especially since we get called at compile - ;; time by PROPS -- and if so, we would return an incorrect result - ;; because the previous step will not have identified all the propapps - ;; in the propspec. So error out if we detect that situation. - (macrolet-and-expand - (loop for leaf in (delete-duplicates (flatten expanded)) - if (and (symbolp leaf) (not (isprop leaf))) - collect `(,leaf (&rest args) - (unless (or (fboundp ',leaf) (isprop ',leaf)) - (error 'ambiguous-propspec :name ',leaf)) - ;; return something which looks like an - ;; ordinary function call to the code walker, - ;; so that it will recurse into ARGS - (cons (gensym) args))) - expanded) - ;; Finally, substitute the mapped propapps back in to the propspec. - (let ((*replaced-propapps* - (alist-hash-table *replaced-propapps* :test 'eq))) - (walk expanded))))) +macros. ENV is passed along to AGNOSTIC-LIZARD:WALK-FORM. + +This implementation will fail to map propapps appearing within the arguments +to properties in propapps, but that should not be needed. It can very +occasionally give incorrect results due to limitations of the Common Lisp +standard with respect to code walking; see \"Pitfalls\" in the Consfigurator +manual." + (let* (replaced-propapps + ;; First we need to find all the propapps, after macro expansion. + ;; Propapps contain the arguments to be passed to properties rather + ;; than expressions which will evaluate to those arguments, and some + ;; of these might be lists, which will look like invalid function + ;; calls to the code walker. So we replace every known property so + ;; that the code walker does not assume these arguments are to be + ;; evaluated as arguments to ordinary functions are. + (expanded + (handler-case + (agnostic-lizard:walk-form + propspec env + :on-macroexpanded-form + (lambda (form env &aux (c (and (listp form) (car form)))) + (declare (ignore env)) + (cond ((and c (isprop c)) + (let ((gensym (gensym))) + (push (cons gensym form) replaced-propapps) + gensym)) + ;; We also look for any symbols without function or + ;; property definitions occurring in function call + ;; positions. These could potentially be properties + ;; whose definitions have not been loaded -- + ;; especially since we get called at compile time by + ;; PROPS -- and if so, we would return an incorrect + ;; result because the previous branch will not have + ;; identified all the propapps in the propspec. So + ;; error out if we detect that situation. + ((and c (not (fboundp c))) + (error 'ambiguous-propspec :name c)) + (t + form)))) + (ambiguous-propspec (c) (error c)) + (error (condition) + (error 'invalid-propspec :error condition :propspec propspec)))) + (replaced-propapps + (alist-hash-table replaced-propapps :test 'eq))) + ;; Finally, substitute the mapped propapps back in to the propspec. + (labels ((walk (tree) + (if (atom tree) + (if-let ((propapp (gethash tree replaced-propapps))) + (funcall function propapp) + (if (and reconstruct (symbolp tree)) `',tree tree)) + (let ((walked (mapcar #'walk tree))) + (if reconstruct (cons 'list walked) walked))))) + (walk expanded)))) (defmacro in-consfig (systems) "Sets the variable *CONSFIG* in the current package to SYSTEMS, or (SYSTEMS) |