specialization-store icon indicating copy to clipboard operation
specialization-store copied to clipboard

More compile-time support.

Open commander-trashdin opened this issue 5 years ago • 21 comments

How possible it is in your opinion to add an ability for the defstore to detect at compile time, that there is no specialization give for types in the function call (let's say the types are provided in declarations or such)?

commander-trashdin avatar Aug 30 '20 20:08 commander-trashdin

To clarify the reason -- me (and I shared my idea with @digikar99) have and idea to use specialization-store for something similar to cl-generic project -- but avoiding generics to still have fast basic operations.

One of the important things so far is the equality operator (let's call it == for now). It would be really cool if we could have made it error at compile-time if types for which it is not defined are passed in (specifically the concern is to make it error if different nonnumeric types are involved, since those are rarely "equal"). I wrote an overlay that does this and only then invokes spec-store stuff, but it's a bit awkward now. Of course we would appreciate any help, including the hints where to start if you are not willing to dedicate your time modifying the project.

commander-trashdin avatar Aug 30 '20 21:08 commander-trashdin

About this, as far as I understand, the way things stand currently, Common Lisp (or SBCL?) has no support for

(declaim (ftype (or (function (integer integer) boolean)
                    (function (string string) boolean))
                ==))
;; Results in
;;   Declared type: FUNCTION

Given that == is supposed to be a function, only support from compiler macro remains; however, simply signally an (error "Ha!") from the compiler macro does indicate that the compilation failed, but -

(defun bar () t)
(define-compiler-macro bar (&whole whole)
  (error "Ha!"))
(defun foo () (bar))
; processing (DEFUN FOO ...)

; file: /tmp/slime6miWfg
; in: DEFUN FOO
;     (SS-USER::BAR)
; 
; caught WARNING:
;   Error during compiler-macroexpansion of (BAR). Use *BREAK-ON-SIGNALS* to
;   intercept.
;   
;    Ha!
; 
; compilation unit finished
;   caught 1 WARNING condition

Depending on whether foo was successfully compiled before, this results in a successful call to (foo2) if it was, even if it was fmakunbound later. Is that an SBCL bug? (If foo was not compiled successfully before, then, it results in a undefined-function error as expected.)

EDIT: Pardon the lack of context. specialization-store currently installs == as a function. Perhaps, improving its compiler macro to check for its argument types could do the job?

digikar99 avatar Aug 31 '20 05:08 digikar99

How possible it is in your opinion to add an ability for the defstore to detect at compile time, that there is no specialization give for types in the function call (let's say the types are provided in declarations or such)?

This is possible using the specialization store meta object protocol. You will have to run this using 017f616b as I found a couple of bugs whilst implementing this example.

(eval-when (:compile-toplevel :load-toplevel :execute)
  (defclass compile-only-store-class (specialization-store:standard-store-class)
    ())

  (defclass compile-only-store (specialization-store:standard-store)
    ()
    (:metaclass compile-only-store-class))

  (defmethod specialization-store:ensure-store-using-object ((store-class compile-only-store-class) store-name store-lambda-list
                                                             &rest args &key specialization-class documentation &allow-other-keys)
    (declare (ignore specialization-class documentation))
    (apply #'make-instance 'compile-only-store
           :name store-name
           :lambda-list store-lambda-list
           args)))

(defmethod specialization-store:expand-store ((store compile-only-store) form &optional environment)
  (declare (ignore environment))
  (let ((new-form (call-next-method)))
    (cond ((eql new-form form)
           (error "No specialization exists in store function ~A." (specialization-store:store-name store)))
          (t
           new-form))))

(specialization-store:defstore example (a)
  (:store-class compile-only-store))

(specialization-store:defspecialization (example :name %example/string) ((a string)) string
  (format nil "Specialization argument is: ~A." a))

(defun main/fail ()
  (example 1))

(defun main/succeed ()
  (example "here"))

markcox80 avatar Aug 31 '20 10:08 markcox80

Hmm... Few issues:

  • Is (example :name %example/string) necessary? Compiling main/succeed fails in the absence of :name.
  • Also, this seems to necessitate that type information be available at compile time. Compiling (defun main (a) (example a)) fails.

Might take a loop at speciaization-store MOP if this is possible.

digikar99 avatar Aug 31 '20 11:08 digikar99

I'm pretty sure to deal with second issue, we'll need to be using environment to see if anything was declared.

commander-trashdin avatar Aug 31 '20 12:08 commander-trashdin

The name is required as written in the documentation.

The original poster asked for "defstore to detect at compile time, that there is no specialization give for types in the function call" and "it would be really cool if we could have made it error at compile-time if types for which it is not defined". The implementation of EXPAND-STORE above has detected that and signaled an error.

I don't understand what you want to happen.

markcox80 avatar Aug 31 '20 20:08 markcox80

Well, that if types are declared, but if they are not declared then it like "who knows", so the idea is to assume it s good and defined and see what happens at run-time.

I'm already trying to tweak your suggestion to do exactly that.

commander-trashdin avatar Aug 31 '20 21:08 commander-trashdin

The default implementation tries to do compile time optimisations. If that fails, a runtime dispatch is performed.

I don't understand what you want.

markcox80 avatar Aug 31 '20 22:08 markcox80

Well, what happens if at the run time it does not find the required specialization? It erros. The point is to error at compile time if it is known (at compile time) there is no such specialization. When no types are declared, we cannot be certain about it, so there should be no error.

Example -- imagine a foo is a store specialized only these for 2 arguments -- string and fixnum.

Compiling this

(defun bar (a b)
  (declare (string a) (fixnum b))
  (foo a b))

should be fine.

Compiling this

(defun bar (a b)
  (foo a b))

should be fine.

Compiling this

(defun bar (a b)
  (declare (fixnum a) (fixnum b))
  (foo a b))

should error.

Right now the difference is that the last case doesn't error (when compiling) -- and instead errors when I run it.

I hope that makes sense.

commander-trashdin avatar Aug 31 '20 23:08 commander-trashdin

a runtime dispatch is performed

Could you point out where this is being done in the source code?


Essentially what we want to do is:

  1. If possible, detect at compile time whether a specialization exists. Signal an error if the specialization doesn't exist.
  2. If not possible to detect at compile time (because, say the types were not declared sufficiently), then compile successfully. (And signal a runtime error if the specialization does not exist, as is being done now.)

As I understand, this requires some modification to the (defmethod specialization-store:expand-store ... as defined above.

And perhaps, knowing how the runtime dispatch is performed could help us in determining what the exact modifications need to be made.

digikar99 avatar Sep 02 '20 06:09 digikar99

I have added the branch feature/compile-time-warning which implements the feature you have requested.

You don't need to use the runtime dispatch code. You need to signal a warning when type information is available and compiler macro expansion fails.

I need to think a bit more about whether or not this should be enabled by default since it requires all specializations to be inlinable or named. I will probably check this before signalling the warning.

markcox80 avatar Sep 03 '20 20:09 markcox80

I suspect enabling it by default would mean any projects that used the specializations before defining them would fail to build. A case I can think of is circular dependency of two specializations. So, may be with a (safety 3) policy or may be by some other means as you had written before.

Hmm, @commander-trashdin , how do we intend to deal with circular dependencies, or would that be a non-issue for the specific purposes of cl-generic-like project?

digikar99 avatar Sep 04 '20 11:09 digikar99

I would prefer to avoid relying on compiler policies, since that would effectively be a restriction.

I think in my specific case (unless I'm not seeing something), there aren't any circular dependencies. Most of the functions are kinda primitive and straightforward. In case something pops up, we can probably develop a workaround. But generally I agree, it is a thing to think about.

commander-trashdin avatar Sep 04 '20 13:09 commander-trashdin

Or perhaps, instead of erroring, a compiler-note could be signalled.

digikar99 avatar Sep 05 '20 21:09 digikar99

Thanks a lot for your effort! Right now this errors:

(defun foo (a b)
  (declare (string a))
                ;;(list b)
  (= a b))

(where = is a store) It would of course be preferable to assume everything is correct when there is not enough info. (I assume it thinks b is of type t and there is no spec for string and t which causes the error?)

By the way, here's https://github.com/commander-trashdin/cl-overload the subject. Heavily rewritten, so there is not much, but in couple of days I'll add way more.

commander-trashdin avatar Sep 10 '20 23:09 commander-trashdin

I would be grateful for a minimal example highlighting your issue. I don't know what specializations are defined.

markcox80 avatar Sep 13 '20 20:09 markcox80

Sure thing, so basically I have this = overloaded for many different types. However, here's the idea -- it is only defined for objects of the same type (sometimes generalized, like it is defined for any two numbers). That means it is defined for string and string, and for list and list, but not for string and list (the future user could add the definition ofc if they really want to).

It makes sense that declaring a to be a string and b to be a list causes (= a b) to compile time error -- because we asked for this feature. It also makes sense that w/o any declarations it doesn't error, because well, there's no reason to. However, decalring a to be a string and not saying anything about b still leads to compile-time(!) error, even though there is not enough information to assume anything -- maybe b would also be a string.

Can we work around that?

commander-trashdin avatar Sep 14 '20 00:09 commander-trashdin

It makes sense that declaring a to be a string and b to be a list causes (= a b) to compile time error -- because we asked for this feature. It also makes sense that w/o any declarations it doesn't error, because well, there's no reason to. However, decalring a to be a string and not saying anything about b still leads to compile-time(!) error, even though there is not enough information to assume anything -- maybe b would also be a string.

In my opinion, the functionality you describe in the text above is specific to your store function. I suggest sub-classing the standard store and then implement an EXPAND-STORE method.

I am going to merge bits of what I have done here in to master. I will be reverting all changes to the standard store implementation.

markcox80 avatar Sep 14 '20 06:09 markcox80

I have merged the bits in to master. Subclassing the standard store is arguably overkill for this application. This example shows how to do it without subclassing.

(defpackage "EXAMPLE"
  (:use "COMMON-LISP"))
(in-package "EXAMPLE")

(specialization-store:defstore object= (a b))

(specialization-store:defspecialization (object= :inline t) ((a string) (b string)) t
  (format t "Testing if strings ~S and ~S are equal." a b))

(specialization-store:defspecialization (object= :inline t) ((a number) (b number)) t
  (format t "Testing if numbers ~S and ~S are equal." a b))

(define-compiler-macro object= (&whole form a b &environment environment)
  (let* ((store (specialization-store:find-store 'object=))
         (new-form (specialization-store:expand-store store form environment)))
    (when (and (eql form new-form)
               (specialization-store:form-value-type-p a environment)
               (specialization-store:form-value-type-p b environment))
      (signal "No specialization available for form ~S." form))
    new-form))

(defun foo (a b)
  (declare (type string a))
  (object= a b))

markcox80 avatar Sep 15 '20 20:09 markcox80

Would you be adding better compile-time reporting any time? The current reporting I know if includes things like:

  • the compiler macro being incomplete and therefore not being able to optimize some cases
  • or other cases such as recursive expansion leading to an infinite loop
  • or the other issue about inlining lambdas with free variables
  • the types of variables/forms not being declared at compile time
  • no specialization available at compile time

... so, basically a superset of the above compiler macro. Would it be worth it to get it into master? (Describing object= even in the absence of the above compiler macro tells ms that it has a compiler macro. Do you think the above things are worth adding to the builtin compiler macro?) I think these seem useful even from an ordinary user's perspective, in that these are basically compiler-notes.

Also, form-value-type-p should probably check introspect-environment:variable-information to determine if the type is declared. (Erm, I forgot the case where I found doing that necessary; will update if I recall.)

digikar99 avatar Sep 19 '20 05:09 digikar99

the compiler macro being incomplete and therefore not being able to optimize some case

You will have to expand on this as I don't know what it means.

or other cases such as recursive expansion leading to an infinite loop

No. I will not be detecting this. This is a problem with macro functions in general.

or the other issue about inlining lambdas with free variables

This has been addressed in Issue #7.

the types of variables/forms not being declared at compile time

The problem I have with this is that it is not clear what reduction is used to trigger the warning. The code I added as part of this issue triggers a warning if at least one form has type information where what you wanted was a warning signaled when all forms have type information.

It isn't obvious what the default should be and I don't think it is wise to complicate defstore with options to customise this behaviour. Having said that, I am more likely to include support for triggering a warning when all forms have type information since if this much information is available then it probably indicates that you want something done with it.

no specialization available at compile time

I am reluctant to add this by default. Specialization store is not just about compile time.

The code included in this issue shows that it is easy to add this functionality to your own store functions.

Also, form-value-type-p should probably check introspect-environment:variable-information to determine if the type is declared. (Erm, I forgot the case where I found doing that necessary; will update if I recall.)

It already does. The function form-value-type-p eventually calls the function determine-form-multiple-value-type which tests the variable information.

markcox80 avatar Oct 01 '20 20:10 markcox80