Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No adequate specialization of MAKE-LOAD-FORM for an object of type GLOBAL-BYTECODE-SIMPLE-FUN #1517

Open
bohonghuang opened this issue Nov 26, 2023 · 4 comments
Labels

Comments

@bohonghuang
Copy link

Describe the bug
In a Lisp source file, an error occurs when assigning the macro-function of a macro to a constant, and then using that constant to create an alias for the macro using (setf macro-function).

Code at issue

  • setf-macro-function.lisp
    (defmacro test-macro ())
    
    (defconstant +test-macro-function+ (macro-function 'test-macro))
    
    (setf (macro-function 'test-macro-2) +test-macro-function+)
CL-USER> (compile-file #P"/tmp/setf-macro-function.lisp")
; Compiling file: /tmp/setf-macro-function.lisp
;   (DEFMACRO TEST-MACRO ...)
;   (DEFCONSTANT +TEST-MACRO-FUNCTION+ ...)
;   (SETF (MACRO-FUNCTION 'TEST-MACRO-2) ...)
; caught ERROR:
;   No adequate specialization of MAKE-LOAD-FORM for an object of type GLOBAL-BYTECODE-SIMPLE-FUN
;     at /tmp/setf-macro-function.lisp 5:0
; 
; 
; compilation unit finished
;   caught 1 ERROR condition
;                               
NIL
T
T

Context

(LISP-IMPLEMENTATION-TYPE) => "clasp"
(LISP-IMPLEMENTATION-VERSION) => "cclasp-boehmprecise-2.4.0-cst"
(MACHINE-TYPE) => "x86_64"
(MACHINE-VERSION) => "#1 SMP PREEMPT_DYNAMIC Thu Nov  9 03:01:44 UTC 2023"
(SOFTWARE-TYPE) => "Linux"
(SOFTWARE-VERSION) => "2.4.0"
@Bike
Copy link
Member

Bike commented Dec 2, 2023

I'm afraid this is probably going to be a WONTFIX. Our defconstant evaluates the value form at compile time (which is allowed by the standard) and uses the value as a literal in subsequent forms, so the value needs to be serializable. Functions are of course not serializable.

It's possible the standard requires this code to compile - I'm not sure, which is why I'm leaving the issue open. However, even if we did check that a constant value is serializable before doing this, I don't think your code is a good idea. On SBCL, the file compiles, but loading it in the same image fails because it tries to evaluate the defconstant form again, which needs to assign the constant variable's value, but the new macro function is not eql to the previous value.

@bohonghuang
Copy link
Author

bohonghuang commented Dec 3, 2023

However, even if we did check that a constant value is serializable before doing this, I don't think your code is a good idea. On SBCL, the file compiles, but loading it in the same image fails because it tries to evaluate the defconstant form again, which needs to assign the constant variable's value, but the new macro function is not eql to the previous value.

Well, I admit that the example is a bit inappropriate because I simplified it for illustration purposes. In my case, test-macro is compiled and loaded in different files. Therefore, whether it is during the compilation or when loading the file that defines the constant, the eql condition of defconstant can be satisfied. This works in other implementations, which is why this issue arises.

@Bike
Copy link
Member

Bike commented Dec 3, 2023

Hmkay. Don't understand your use case, but that's fine. Looking at SBCL, when it compiles a constant variable reference it goes a bit out of its way to ensure references to named constants aren't compiled the same way as anonymous literals (which is what Clasp does, which causes this issue). The constant structure in the IR gets a name (https://github.com/sbcl/sbcl/blob/master/src/compiler/ir1tran.lisp#L320) and then during FASL dump, any references with a name just use that (https://github.com/sbcl/sbcl/blob/master/src/compiler/dump.lisp#L1159-L1160). It would probably be nice to do this in Clasp as well. Will take some fiddling with infrastructure in both the bytecode and Cleavir, though.

ETA: Nice to make this work, but probably more importantly so that we don't end up multiply reconstructing constants, which I think we might be doing and which would definitely be unfortunate.

Bike added a commit that referenced this issue Jan 23, 2024
See #1517. This allows FASLs to reference constants bound to
unserializable objects without error. For example:

(defconstant +foo+ #'foo)

(defun bar () +foo+)
@Bike
Copy link
Member

Bike commented Jan 23, 2024

Took a minute to try getting it working with bytecode FASLs. Doesn't seem to present any difficulty. I have not done it for FASOs, and I'm debating whether to bother doing that now or continue on the slower path to removing FASOs entirely. (ETA: That branch won't build until I push some plumbing stuff in a bit)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants