Opened 11 months ago

Closed 3 weeks ago

#1548 closed defect (fixed)

Extra modules leak into environment when using -j

Reported by: megane Owned by:
Priority: major Milestone: 5.2
Component: compiler Version: 5.0.0
Keywords: macro Cc:
Estimated difficulty:

Description

Found by dieggsy on IRC.

When defining multiple modules in a single file and generating an import file for only one of those the namespace from the other modules leaks into the importing code.

In this case it's the srfi-13 leaking into the importing code. Just saying (import m) should only bring module m's imports into the environment.

This seems to be a regression from C4.

(module helper *
        (import scheme)
        (import srfi-13)
        ;; (import chicken) (use srfi-13) ; for C4

        (define-syntax complex-foreign-lambda
          (ir-macro-transformer
           (lambda (e i c)
             `(begin)))))

(module m *)

;; 4.13.0
;; $ csc -s m.scm -j m && csi -qbn -e '(import m) (print (string-null? "") (string-null? "1"))'
;;
;; Error: unbound variable: string-null?

;; chicken-5.0.0rc2
;; $ csc -s m.scm -j m && csi -qbn -e '(import m) (print (string-null? "") (string-null? "1"))'
;; #t#f

;; $ csc -debug 2 -s m.scm -j m
;; [canonicalized]
;; (##core#callunit library)
;;
;; (##core#callunit eval)
;;
;; (##core#callunit expand)
;;
;; (##core#undefined)
;;
;; (##core#undefined)
;;
;; (##core#undefined)
;;
;; (let ((t43 (##core#provide helper#)))
;; (let ((t44 (scheme#eval '(import-syntax scheme srfi-13)))) ; <---- offending import
;; (let ((t45 (##sys#register-compiled-module
;; 'helper
;; 'helper
;; (scheme#list)
;; '()
;; (scheme#list
;; (scheme#cons
;; 'complex-foreign-lambda
;; (chicken.syntax#ir-macro-transformer
;; (##core#lambda (e37 i38 c39) (##sys#list 'begin)))))
;; (scheme#list))))
;; (let ((t46 (chicken.load#load-extension 'srfi-13 '(srfi-13#) 'require)))
;; (##core#undefined)))))
;;
;; (##core#provide m#)
;;
;; (##core#undefined)

Attachments (1)

0001-Preserve-global-environment-when-executing-module-re.patch (5.4 KB) - added by felix winkelmann 3 weeks ago.
with-environment

Download all attachments as: .zip

Change History (8)

comment:1 Changed 11 months ago by megane

I forgot to mention that using -J fixes the problem. This moves the offending import to helper.import.scm.

comment:2 Changed 11 months ago by sjamaan

Milestone: 5.05.1

I'm not sure this is an entirely correct comparison, given that (import) is now equivalent to (use) in CHICKEN 4. If you do csc -s m.scm -j m && csi -qbn -e '(use m) (print (string-null? "") (string-null? "1"))', you'll notice that it prints the same as C5. So, IMO it's not a true regression and we should probably postpone this to 5.1, especially given that fixing this would probably involve changing the import machinery, which might lead to new bugs and cause other unexpected changes this late in the process.

comment:3 in reply to:  2 ; Changed 11 months ago by megane

Replying to sjamaan:

I'm not sure this is an entirely correct comparison, given that (import) is now equivalent to (use) in CHICKEN 4. If you do csc -s m.scm -j m && csi -qbn -e '(use m) (print (string-null? "") (string-null? "1"))', you'll notice that it prints the same as C5. So, IMO it's not a true regression and we should probably postpone this to 5.1, especially given that fixing this would probably involve changing the import machinery, which might lead to new bugs and cause other unexpected changes this late in the process.

On CHICKEN 4 csc -s m.scm -j m && csi -qbn -e '(use m) (print (string-null? "") (string-null? "1")) gives me Error: unbound variable: string-null?. That is the expected behavior.

comment:4 in reply to:  3 Changed 11 months ago by megane

Replying to megane:

Replying to sjamaan:

...

There was a PEBKAC issue here. I was compiling the (import srfi-13) version, when I should have used the (use srfi-13) version. No wonder string-null? wasn't found. So my regression judgement is wrong.

Last edited 11 months ago by megane (previous) (diff)

comment:5 Changed 5 months ago by sjamaan

Milestone: 5.15.2

Getting ready for 5.1, moving tickets which won't make it in to 5.2.

comment:6 Changed 3 weeks ago by felix winkelmann

Attached is a patch that seems to fix the problem. The module registration code pollutes the global namespace, which doesn't cause problems in import libraries, because the loading is wrapped in a parameterize form that preserves the environment. The patch factors out the preservation into a spearate form (##sys#with-environment) and wraps the module-registration in such a form.

Changed 3 weeks ago by felix winkelmann

with-environment

comment:7 Changed 3 weeks ago by sjamaan

Resolution: fixed
Status: newclosed

Fixed by f43fb51e

Note: See TracTickets for help on using tickets.