[PATCH] Compile in a fresh module by default

[PATCH] Compile in a fresh module by default

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 556 bytes --]

Hello Guilers!

The attached patch makes `compile' and friends use fresh module rather
than the current module as the default compile-time environment.

The intent is to make sure macro definitions and side-effects made at
expansion time do not (to some extents) clutter the current module's
name space.

However, assignments to global variables made at expansion time do have
a visible effect on the running system.  This is because the compiler
and "compilee" share the whole module hierarchy, down to the pre-module
obarray.

Comments?

Thanks,
Ludo'.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: the patch --]
[-- Type: text/x-patch, Size: 6010 bytes --]

From 8879fa894377fc062f30358eb428c5ec757c43ab Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Fri, 14 Aug 2009 19:30:14 +0200
Subject: [PATCH 1/2] Compile in a fresh module by default.

* module/system/base/compile.scm (make-compilation-module,
  language-default-environment): New procedures.
  (read-and-compile, compile): Have ENV default to
  `(language-default-environment from)'.
  (compile-and-load): Compile in `(current-module)'.

* test-suite/tests/compiler.test ("psyntax")["compile uses a fresh module by
  default", "compile-time definitions are isolated"]: New tests.
  ["compile in current module"]: Specify `#:env (current-module)'.
  ["redefinition"]: Adjust.

* test-suite/tests/bytevectors.test (c&e): Explicitly compile in the
  current module so that its imports are visible.
---
 module/system/base/compile.scm    |   26 +++++++++++++++++++++++---
 test-suite/tests/bytevectors.test |    5 +++--
 test-suite/tests/compiler.test    |   31 ++++++++++++++++++++++---------
 3 files changed, 48 insertions(+), 14 deletions(-)

diff --git a/module/system/base/compile.scm b/module/system/base/compile.scm
index 8470f39..f3557cb 100644
--- a/module/system/base/compile.scm
+++ b/module/system/base/compile.scm
@@ -158,7 +158,8 @@
 
 (define* (compile-and-load file #:key (from 'scheme) (to 'value) (opts '()))
   (read-and-compile (open-input-file file)
-                    #:from from #:to to #:opts opts))
+                    #:from from #:to to #:opts opts
+                    #:env (current-module)))
 
 \f
 ;;;
@@ -187,6 +188,23 @@
           (else
            (lp (cdr in) (caar in))))))
 
+(define (make-compilation-module)
+  "Return a fresh module to be used as the compilation environment."
+
+  ;; Ideally we'd duplicate the whole module hierarchy so that `set!',
+  ;; `fluid-set!', etc. don't have any effect in the current environment.
+
+  (let ((m (make-module)))
+    (beautify-user-module! m)
+    m))
+
+(define (language-default-environment lang)
+  "Return the default compilation environment for source language LANG."
+  (if (or (eq? lang 'scheme)
+          (eq? lang (lookup-language 'scheme)))
+      (make-compilation-module)
+      #f))
+
 (define* (read-and-compile port #:key
                            (env #f)
                            (from (current-language))
@@ -196,7 +214,8 @@
         (to (ensure-language to)))
     (let ((joint (find-language-joint from to)))
       (with-fluids ((*current-language* from))
-        (let lp ((exps '()) (env #f) (cenv env))
+        (let lp ((exps '()) (env #f)
+                 (cenv (or env (language-default-environment from))))
           (let ((x ((language-reader (current-language)) port)))
             (cond
              ((eof-object? x)
@@ -225,7 +244,8 @@
                     warnings))))
 
   (receive (exp env cenv)
-      (compile-fold (compile-passes from to opts) x env opts)
+      (let ((env (or env (language-default-environment from))))
+        (compile-fold (compile-passes from to opts) x env opts))
     exp))
 
 \f
diff --git a/test-suite/tests/bytevectors.test b/test-suite/tests/bytevectors.test
index 8b336bb..c0f5196 100644
--- a/test-suite/tests/bytevectors.test
+++ b/test-suite/tests/bytevectors.test
@@ -31,12 +31,13 @@
      (begin (pass-if (string-append test-name " (eval)")
                      (primitive-eval 'exp))
             (pass-if (string-append test-name " (compile)")
-                     (compile 'exp #:to 'value))))
+                     (compile 'exp #:to 'value #:env (current-module)))))
     ((_ (pass-if-exception test-name exc exp))
      (begin (pass-if-exception (string-append test-name " (eval)")
                                exc (primitive-eval 'exp))
             (pass-if-exception (string-append test-name " (compile)")
-                               exc (compile 'exp #:to 'value))))))
+                               exc (compile 'exp #:to 'value
+                                            #:env (current-module)))))))
 
 (define-syntax with-test-prefix/c&e
   (syntax-rules ()
diff --git a/test-suite/tests/compiler.test b/test-suite/tests/compiler.test
index f9fabd7..2eb0e78 100644
--- a/test-suite/tests/compiler.test
+++ b/test-suite/tests/compiler.test
@@ -30,18 +30,23 @@
 \f
 (with-test-prefix "psyntax"
 
-  (pass-if "redefinition"
-    ;; In this case the locally-bound `round' must have the same value as the
-    ;; imported `round'.  See the same test in `syntax.test' for details.
+  (pass-if "compile uses a fresh module by default"
+    (begin
+      (compile '(define + -))
+      (eq? (compile '+) +)))
+
+  (pass-if "compile-time definitions are isolated"
     (begin
-      (compile '(define round round))
-      (compile '(eq? round (@@ (guile) round)))))
+      (compile '(define foo-bar #t))
+      (not (module-variable (current-module) 'foo-bar))))
 
   (pass-if "compile in current module"
     (let ((o (begin
-               (compile '(define-macro (foo) 'bar))
-               (compile '(let ((bar 'ok)) (foo))))))
-      (and (module-ref (current-module) 'foo)
+               (compile '(define-macro (foo) 'bar)
+                        #:env (current-module))
+               (compile '(let ((bar 'ok)) (foo))
+                        #:env (current-module)))))
+      (and (macro? (module-ref (current-module) 'foo))
            (eq? o 'ok))))
 
   (pass-if "compile in fresh module"
@@ -52,4 +57,12 @@
                  (compile '(define-macro (foo) 'bar) #:env m)
                  (compile '(let ((bar 'ok)) (foo)) #:env m))))
       (and (module-ref m 'foo)
-           (eq? o 'ok)))))
+           (eq? o 'ok))))
+
+  (pass-if "redefinition"
+    ;; In this case the locally-bound `round' must have the same value as the
+    ;; imported `round'.  See the same test in `syntax.test' for details.
+    (let ((m (make-module)))
+      (beautify-user-module! m)
+      (compile '(define round round) #:env m)
+      (eq? round (module-ref m 'round)))))
-- 
1.6.1.3

Re: [PATCH] Compile in a fresh module by default

[Ludovic Courtès]

Hello!

ludo@gnu.org (Ludovic Courtès) writes:

> The attached patch makes `compile' and friends use fresh module rather
> than the current module as the default compile-time environment.

I committed a slightly modified version in
87c595c757b7db84ffdcfda96f736ab235e674a8.

Thanks,
Ludo’.

Re: [PATCH] Compile in a fresh module by default

[Julian Graham]

Hi Ludovic,

Naive question (and I realize I'm a bit late here): What if there are
bindings in the current module that need to be present for expansion
and compilation to succeed?  I ask because I'm working on a macro that
transforms R6RS library expressions, which contain nested `define'
calls, into Guile modules.  My code rewrites these defines as location
declarations that get exported as part of a module interface (at
least, in theory -- right now it just erases them).

When I bind my `library' macro in the REPL after building from the
current HEAD of master and then load and compile a source file
containing an R6RS library form, I get a syncase error during
compilation about defines in expression context; when I revert the
effects of this patch, the error goes away.


Regards,
Julian


2009/9/20 Ludovic Courtès <ludo@gnu.org>:
> Hello!
>
> ludo@gnu.org (Ludovic Courtès) writes:
>
>> The attached patch makes `compile' and friends use fresh module rather
>> than the current module as the default compile-time environment.
>
> I committed a slightly modified version in
> 87c595c757b7db84ffdcfda96f736ab235e674a8.

Re: [PATCH] Compile in a fresh module by default

[Ludovic Courtès]

Hi Julian,

Julian Graham <joolean@gmail.com> writes:

> Naive question (and I realize I'm a bit late here): What if there are
> bindings in the current module that need to be present for expansion
> and compilation to succeed?

Just specify ‘(compile EXP #:env (current-module))’.  This is what
‘repl-compile’ in ‘(system repl common)’ does, so that one can define
macros at the REPL and actually use them.

> I ask because I'm working on a macro that transforms R6RS library
> expressions, which contain nested `define' calls, into Guile modules.
> My code rewrites these defines as location declarations that get
> exported as part of a module interface (at least, in theory -- right
> now it just erases them).

Can you give an example of the macro transformation?

> When I bind my `library' macro in the REPL after building from the
> current HEAD of master and then load and compile a source file
> containing an R6RS library form, I get a syncase error during
> compilation about defines in expression context; when I revert the
> effects of this patch, the error goes away.

Can you post a simplified example that reproduces the problem?

Thanks,
Ludo’.

Re: [PATCH] Compile in a fresh module by default

[Andy Wingo]

On Wed 14 Oct 2009 10:11, ludo@gnu.org (Ludovic Courtès) writes:

>> I ask because I'm working on a macro that transforms R6RS library
>> expressions, which contain nested `define' calls, into Guile modules.
>> My code rewrites these defines as location declarations that get
>> exported as part of a module interface (at least, in theory -- right
>> now it just erases them).
>
> Can you give an example of the macro transformation?

(define-syntax foo ...)
(load "file-that-needs-foo")

The load needs to be done relative to the current module, which it is --
unless it's autocompiling, in which case we go through the autocompile
case in boot-9.scm:924 (not the one in load.c), which is missing an
#:env (current-module).

Probably needs to be fixed before today's release.

Andy

-- 
http://wingolog.org/

Re: [PATCH] Compile in a fresh module by default

[Ludovic Courtès]

Andy Wingo <wingo@pobox.com> writes:

> (define-syntax foo ...)
> (load "file-that-needs-foo")
>
> The load needs to be done relative to the current module, which it is --
> unless it's autocompiling, in which case we go through the autocompile
> case in boot-9.scm:924 (not the one in load.c), which is missing an
> #:env (current-module).
>
> Probably needs to be fixed before today's release.

Yes indeed.

Thanks,
Ludo'.