Zulu-Inuoe / jzon

A correct and safe(er) JSON RFC 8259 reader/writer with sane defaults.
MIT License
151 stars 14 forks source link

CCL/LW/ACL: DEFCONSTANT needs an EVAL-ALWAYS #27

Closed phoe closed 1 year ago

phoe commented 1 year ago
Clozure Common Lisp Version 1.12 (v1.12) LinuxX8664

For more information about CCL, please see http://ccl.clozure.com.

CCL is free software.  It is distributed under the terms of the Apache
Licence, Version 2.0.
? (ql:quickload :com.inuoe.jzon)
To load "com.inuoe.jzon":
  Load 6 ASDF systems:
    asdf closer-mop documentation-utils flexi-streams
    trivial-gray-streams uiop
  Install 2 Quicklisp releases:
    float-features jzon
Downloading http://beta.quicklisp.org/archive/float-features/2023-02-14/float-features-20230214-git.tgz
##########################################################################
Downloading http://beta.quicklisp.org/archive/jzon/2023-02-15/jzon-20230215-git.tgz
##########################################################################
; Loading "com.inuoe.jzon"
[package trivial-indent]..........................
[package documentation-utils].....................
[package float-features]..........................
[package com.inuoe.jzon/eisel-lemire]..
Read error between positions 3533 and 3621 in /home/phoe/.roswell/lisp/quicklisp/dists/quicklisp/software/jzon-20230215-git/src/eisel-lemire.lisp.
> Error: Unbound variable: +%POW10-MIN+
> While executing: CCL::CHEAP-EVAL-IN-ENVIRONMENT, in process listener(1).
> Type :GO to continue, :POP to abort, :R for a list of available restarts.
> If continued: Retry getting the value of +%POW10-MIN+.
> Type :? for other options.
1 > 

These two need an EVAL-WHEN :COMPILE-TOPLEVEL around them so that the definition is always available at compilation time:

https://github.com/Zulu-Inuoe/jzon/blob/90c9eed6f4b4efaa290ce1d5fc763b8aa6e2a9db/src/eisel-lemire.lisp#L30-L31

Zulu-Inuoe commented 1 year ago

Thanks for the heads-up, adding that now. I've been under the assumption that constants were evaluated at both compile and load time, but apparently that's up to the impl

Yehouda commented 1 year ago

The COMPILER in COMPILE-FILE does put the result of the evaluation in of defconstant its own environment, so when it encounters the same symbol, it can use the value instead (though doesn't have to). The problem in this specific case is that the constants are used later in the same file inside a #. form, so it is the READER that evaluates them, and the READER doesn't know about the COMPILER environment. That is not implementation dependent, and implementation that doesn't give an an error in this case is actually not compliant.

Zulu-Inuoe commented 1 year ago

Is there still an issue after the addition of cl:eval-when ? Should I avoid the #. form or something else?

Yehouda commented 1 year ago

No and maybe.

I just thought it would be useful to clarify that it is not impementation dependent. It was actually a bug that needed fixing.

You often get this kind of bugs when using #., which probably make it better to avoid use it. But once you fixed it, it will work, and in this specific case there is no obvious better solution.

Zulu-Inuoe commented 1 year ago

Okay cool, thanks. Yeah I was just concerned there was something I missed