Summary

csi needs double imports

Metadata

Attachments

Description

Here's a simple test case:

 $ cat assoc.scm                                                                                                    
 (import srfi-1)
 (define e2 '((2 3) (5 7) (11 13)))
 (assoc 5 e2 <)
 
 $ ~/local/chicken-5-head/bin/csi -s assoc.scm 
 
 Error: bad argument count - received 3 but expected 2: #<procedure (scheme#assoc x231 lst232)>
 
         Call history:
 
         <syntax>          (##core#begin (##core#require expand chicken.syntax#))
         <syntax>          (##core#require expand chicken.syntax#)
         <syntax>          (##sys#load-library (##core#quote expand))
         <syntax>          (##core#quote expand)
         <eval>    (##sys#load-library (##core#quote library))
         <eval>    (##sys#load-library (##core#quote library))
         <eval>    (##sys#load-library (##core#quote expand))
         <syntax>          (define e2 (quote ((2 3) (5 7) (11 13))))
         <syntax>          (##core#begin (##core#ensure-toplevel-definition e2) (##core#set! e2 (quote ((2 3) (5 7) (11 13)))))
         <syntax>          (##core#ensure-toplevel-definition e2)
         <syntax>          (##core#undefined)
         <syntax>          (##core#set! e2 (quote ((2 3) (5 7) (11 13))))
         <syntax>          (quote ((2 3) (5 7) (11 13)))
         <syntax>          (##core#quote ((2 3) (5 7) (11 13)))
         <syntax>          (assoc 5 e2 <)
         <eval>    (assoc 5 e2 <)        <--

The compiled version works ok:

 $ ~/local/chicken-5-head/bin/csc assoc.scm 
 In file included from assoc.c:8:
 /home/mario/local/chicken-5-head/include/chicken/chicken.h:2411:37: warning: expression result unused [-Wunused-value]
     C_CHECKp(x,C_bignump(C_VAL1(x)),0);
                                     ^
 /home/mario/local/chicken-5-head/include/chicken/chicken.h:858:37: note: expanded from macro 'C_CHECKp'
 # define C_CHECKp(v,a,x)           (x)
                                     ^
 1 warning generated.
 
 $ ./assoc
 
 $ echo $?
 0

If I import srfi-1 twice, it works in the interpreter:

 $ cat assoc.scm 
 (import srfi-1)
 (import srfi-1)
 (define e2 '((2 3) (5 7) (11 13)))
 (assoc 5 e2 <)
 
 $ ~/local/chicken-5-head/bin/csi -s assoc.scm
 
 $ echo $?
 0

I suspect the same problem is affecting srfi-71: https://salmonella-linux-x86-64.call-cc.org/master/gcc/linux/x86-64/2018/01/02/salmonella-report/test/srfi-71.html

I'm using

 $ ~/local/chicken-5-head/bin/csi -version
 (c) 2008-2017, The CHICKEN Team
 (c) 2000-2007, Felix L. Winkelmann
 Version 5.0.0 (rev baf6363)
 linux-unix-clang-x86-64 [ 64bit dload ptables ]

I cannot reproduce this problem with 5.0.0pre4.

Changes and comments

[2018-01-02 20:59:39 UTC] mario changed description

[2018-01-02 20:59:39 UTC] mario changed summary

[2018-01-06 15:40:53 UTC] felix changed component from unknown to expander

[2018-01-06 15:40:53 UTC] felix changed milestone from someday to 5.0

[2018-01-07 14:27:09 UTC] sjamaan wrote:

It looks like this is happening because `(import srfi-1)` causes the import library to be loaded and imported into the environment, but then also causes `srfi-1.so` to be loaded, which contains `(eval '(scheme chicken.base chicken.syntax))` since 24325e3a6e1e829e9e9be8d22e28d1711a796bd2.

This `eval` clobbers the bindings we just imported with those from `scheme`. Because calling `(import srfi-1)` again at the REPL won't cause the srfi-1 library's toplevel to be evaluated again, the bindings are clobbered yet again, but this time with those from srfi-1, causing us to actually see the intended bindings.

You can see it clearly in a DEBUGBUILD:

 #;1> (import srfi-1)
 ; loading /home/sjamaan/chickens/chicken-5-dbg/lib/chicken/9/srfi-1.import.so ...
 ; loading /home/sjamaan/chickens/chicken-5-dbg/lib/chicken/9/chicken.import.so ...
 
 Note: re-importing already imported identifier: assoc  [this is the srfi-1 binding we asked for, srfi-1#assoc]
 
 Note: re-importing already imported identifier: member [same]
 ; loading /home/sjamaan/chickens/chicken-5-dbg/lib/chicken/9/srfi-1.so ...
 
 Note: re-importing already imported identifier: member [srfi-1 toplevel executing, importing scheme#assoc as assoc]
 
 Note: re-importing already imported identifier: assoc [same]
 #;2> (import srfi-1)
 
 Note: re-importing already imported identifier: assoc [and now we ask for srfi-1#assoc again, and we get it because no new code is run]
 
 Note: re-importing already imported identifier: member

I'm not sure about the best fix for this. We could change the ordering of what `import` does, so that the import of the requested library is done **after** all the dependency loading, but that's not a complete fix. Perhaps only proper programs (not libraries) should get these `eval` expressions injected at the toplevel?

[2018-01-07 14:29:45 UTC] sjamaan wrote:

Replying to sjamaan: > We could change the ordering of what `import` does, so that the import of the requested library is done **after** all the dependency loading, but that's not a complete fix.

Nah, it's not a fix at all. If you do something like:

 (import srfi-1)
 (import some-other-library)

the toplevel of `some-other-library` will cause a re-import of `scheme` at the toplevel, clobbering the srfi-1 imports.

[2018-01-07 16:37:30 UTC] sjamaan attached toplevel-evals.diff (description=Attempt that fails)

[2018-01-07 16:38:50 UTC] sjamaan wrote:

The above attachment should fix the situation, but for some reason this causes `make check` to fail on the `test-finalizers.scm` interpreted test.

[2018-01-07 21:44:13 UTC] felix wrote:

Replying to sjamaan: > The above attachment should fix the situation, but for some reason this causes `make check` to fail on the `test-finalizers.scm` interpreted test.

I don't think this is related. The test works sometimes, depending on cmdline-arguments or how it is run. It's a different issue that causes finalizers not to run in certain situations.

[2018-01-16 13:18:54 UTC] felix wrote:

Note that this problem still exists in the recent change for allowing import of base modules in static executables (commit 81e400fc2516b64d2c768fcd5e97772ecc8e5327). I assume this is caused by a wrong initialization order of imports. To reproduce this problem, try this:

 ;; compile: bin/csc -static x.scm && ./x -:rd
 (import srfi-1)
 (print (assoc 'a '((a . 1)) eq?))
 (print (eval '(begin (import srfi-1) (assoc 'a '((a . 1)) eq?))))

The first call to assoc succeeds, the second fails with an error message.

[2018-04-28 11:06:21 UTC] sjamaan wrote:

I just noticed if you split up into two evals, it works correctly!

 (import srfi-1)
 (print (assoc 'a '((a . 1)) eq?))
 (print (eval '(import srfi-1)))
 (print (eval '(assoc 'a '((a . 1)) eq?)))

This probably is something going wrong in the evaluator's lookup or in how begin is handled, not the imports as such.

[2018-04-28 11:48:44 UTC] sjamaan wrote:

Looks like this will also fail when using the compiler:

 (begin
   (import srfi-1)
   (print (assoc 'a '((a . 1)) eq?)))

When compiling this:

 Warning: at toplevel:
   (test.scm:3) in procedure call to `scheme#assoc', expected 2 arguments but was given 3 arguments

And when run, it will fail too.

The reason is that in the compiler, we walk the toplevel using `map`, which calls `canonicalize-expression` on each toplevel expression. This in turn accesses `(##sys#current-environment)` once at the start and then passes this on as the syntax environment (`se`), which gets passed along. When expanding an import, we update `##sys#current-environment`, but this won't affect `se` because that's another list (at best, it's the tail of the new `##sys#current-environment`).

In the evaluator, the same happens. So you'd have to map `eval` over the "toplevel" expressions, instead.

[2018-04-29 00:48:59 UTC] sjamaan changed status from new to closed

[2018-04-29 00:48:59 UTC] sjamaan set resolution to fixed

[2018-04-29 00:48:59 UTC] sjamaan wrote:

Fixed by bf374b022bc40c3cc24d4e7e7267ea300f8be987