`set!' of generated temporary in macro expansion causes warning

`set!' of generated temporary in macro expansion causes warning

[Andreas Rottmann]

Hi!

This is a much-stripped-down version of a `define-values' macro,
exposing what seems like a psyntax bug:

% cat guile-psyntax-temp-set-issue.scm 
(define-syntax define+set!
  (lambda (form)
    (syntax-case form ()
      ((_)
       (with-syntax (((mutable-id) (generate-temporaries '(id))))
         #'(begin
             (define mutable-id #f)
             (set! mutable-id #t)))))))

(define+set!)

% guile guile-psyntax-temp-set-issue.scm                  
;;; note: auto-compilation is enabled, set GUILE_AUTO_COMPILE=0
;;;       or pass the --no-auto-compile argument to disable.
;;; compiling /home/rotty/tmp/guile-psyntax-temp-set-issue.scm

;;; WARNING (module system is booted, we should have a module #{ g65}#)
;;; compiled /home/rotty/.cache/guile/ccache/2.0-LE-8-2.0/home/rotty/tmp/guile-psyntax-temp-set-issue.scm.go

Note the WARNING line; it seems during expansion something goes wrong
wrt. to modules -- it'd be really nice if some psyntax guru can have a
look at this.

Regards, Rotty
-- 
Andreas Rottmann -- <http://rotty.yi.org/>

Re: `set!' of generated temporary in macro expansion causes warning

[Andreas Rottmann]

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

Stefan Israelsson Tampe <stefan.itampe@gmail.com> writes:

> It looks like you just put the symbol in the current module, so maybe the
> warning is
> just unessesary.
>
> here is the code in psyntax:
>
> (define get-global-definition-hook
>         (lambda (symbol module)
>           (if (and (not module) (current-module))
>               (warn "module system is booted, we should have a module"
> symbol))
>           (let ((v (module-variable (if module
>                                         (resolve-module (cdr module))
>                                         (current-module))
>                                     symbol)))
>
>
> so module parameter beeing #f will lead to a warning and an evaluation of
>    (module-variable (current-module) symbol)
>
> So if it's ok for the temporaries to be in the current-module then the
> warning need to silenced.
>
> E.g. In
> (set! generate-temporaries
>           (lambda (ls)
>             (arg-check list? ls 'generate-temporaries)
> (map (lambda (x) (wrap (gensym-hook) top-wrap #f)) ls)))
>
> Replace #f with (current-module) and you will not have a warning. But My
> psyntax-fu is
> weak so this is just my 2c
>
Thanks; I've now locally applied this patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: generate-temporaries.diff --]
[-- Type: text/x-diff, Size: 844 bytes --]

From: Andreas Rottmann <a.rottmann@gmx.at>
Subject: Silence warnings for variables created by `generate-temporaries'



---
 module/ice-9/psyntax.scm |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/module/ice-9/psyntax.scm b/module/ice-9/psyntax.scm
index 5380ba7..788eefd 100644
--- a/module/ice-9/psyntax.scm
+++ b/module/ice-9/psyntax.scm
@@ -2428,7 +2428,8 @@
     (set! generate-temporaries
           (lambda (ls)
             (arg-check list? ls 'generate-temporaries)
-            (map (lambda (x) (wrap (gensym-hook) top-wrap #f)) ls)))
+            (let ((mod (cons 'hygiene (module-name (current-module)))))
+              (map (lambda (x) (wrap (gensym-hook) top-wrap mod)) ls))))
 
     (set! free-identifier=?
           (lambda (x y)
-- 
tg: (589bc52..) t/generate-temporaries (depends on: stable-2.0)

[-- Attachment #3: Type: text/plain, Size: 84 bytes --]


It seems to work OK, but I have no idea if it is really "correct".

Regards, Rotty

Re: `set!' of generated temporary in macro expansion causes warning

[Andreas Rottmann]

Stefan Israelsson Tampe <stefan.itampe@gmail.com> writes:

> One thing that can go wrong is that we generate a lot of gensymed variables
> in current-module
>
> So be careful and have a peek at the defined module variables to see if
> doesn't get polluted
>
The use of `generate-temporaries' is actually a workaround for Guile
currently breaking hygiene when a macro introduces a top-level
identifier.  Anyway, the namespace certainly will get polluted, but
there's no way around it with current Guile -- see also
<https://savannah.gnu.org/bugs/?31472>, and
<http://lists.gnu.org/archive/html/guile-devel/2011-02/msg00241.html>.

I think (but can't find the message ATM) that Andy has changed his mind
from what he stated in his immediate reply to the bug report, and is now
in favor of eventually making macro-introduced names hygienic; but IIRC
there has not yet been found a way to avoid namespace pollution.

Regards, Rotty

PS: Would you consider avoiding top-posting, and doing a "full reply" in
    your MUA, so the mailing list (bug-guile@gnu.org) gets a copy of
    your messagess as well?

Re: `set!' of generated temporary in macro expansion causes warning

[Andy Wingo]

Hi,

On Mon 13 Jun 2011 14:10, Andreas Rottmann <a.rottmann@gmx.at> writes:

> From: Andreas Rottmann <a.rottmann@gmx.at>
> Subject: Silence warnings for variables created by `generate-temporaries'

Applied, also having regenerated psyntax-pp.scm.

FWIW the procedure there is that, after having modified psyntax.scm,
recompiled everything, and all is well, then you cd module/; make
ice-9/psyntax-pp.scm.gen; make.

Thanks for the report and fix, and thanks for the help, Stefan.

Andy

p.s.  Regarding hygienically introducing identifiers, I had an idea.
From a mail I sent to scheme-reports:

    To recap:

       (define-syntax define-const
         (syntax-rules ()
           ((_ name val)
            (define t val)
            (define-syntax name (syntax-rules () ((_) t))))))

    Guile currently does not make the generated toplevel definition "t" have
    a fresh name.  It would be nice if it could but it can't be a really
    random name -- it needs to be predicatable.

    Well why not have the name of "t" be "t" plus some string which depends
    only on the incoming form -- like its hash value.  (Or the outgoing
    form; the considerations are different but similar.)

    That way you do preserve the "compatible recompilation" aspect, trading
    off true secrecy, but hey.  Oh well.

This would obviously be for master / 2.2 and not 2.0.
-- 
http://wingolog.org/

Re: `set!' of generated temporary in macro expansion causes warning

[Mark H Weaver]

Hi Andy,

Andy Wingo <wingo@pobox.com> writes:
> p.s.  Regarding hygienically introducing identifiers, I had an idea.
> From a mail I sent to scheme-reports:
>
>     To recap:
>
>        (define-syntax define-const
>          (syntax-rules ()
>            ((_ name val)
>             (define t val)
>             (define-syntax name (syntax-rules () ((_) t))))))
>
>     Guile currently does not make the generated toplevel definition "t" have
>     a fresh name.  It would be nice if it could but it can't be a really
>     random name -- it needs to be predicatable.
>
>     Well why not have the name of "t" be "t" plus some string which depends
>     only on the incoming form -- like its hash value.  (Or the outgoing
>     form; the considerations are different but similar.)

This won't work.  Two identical invocations of the same macro must
generate distinct toplevel variables, in the same way that identical
invocations of the same procedure must generate distinct lexical
variables.  Therefore, the name must depend not only on the form;
identical forms must somehow be made unique.  One possibility is to
include the source code location in addition to (or instead of) the
form.

Furthermore, it is not sufficient to include only one incoming form.  We
must include the entire stack of incoming forms (each annotated to make
them unique).  Suppose macro A invokes macro B twice, and macro B makes
a private top-level variable.  Invoking macro A once should create two
distinct top-level variables.

If these names were constructed from the stack of source code locations,
this could also be a great help in generating useful error messages
involving these generated top-level variables.

    Best,
     Mark


>     That way you do preserve the "compatible recompilation" aspect, trading
>     off true secrecy, but hey.  Oh well.
>
> This would obviously be for master / 2.2 and not 2.0.

Re: `set!' of generated temporary in macro expansion causes warning

[Andy Wingo]

Hi Mark,

Good to hear from you!

On Thu 30 Jun 2011 19:35, Mark H Weaver <mhw@netris.org> writes:

> Andy Wingo <wingo@pobox.com> writes:
>>     Well why not have the name of "t" be "t" plus some string which depends
>>     only on the incoming form -- like its hash value.  (Or the outgoing
>>     form; the considerations are different but similar.)
>
> This won't work.  Two identical invocations of the same macro must
> generate distinct toplevel variables

There are important cases when the opposite is true.

Consider a GNU/Linux distribution, which provides binary packages for
Guile and for Guile-Foo.  Both distribute compiled Guile files --
currently .go files, perhaps ELF .so files in the future.  Let's say
that Guile-Foo uses a record type from Guile, so that it residualizes a
reference to a generaated temporary from Guile.  Cool.

Now I download the source package for Guile from the distro.  I want to
make an unrelated change to Guile, in the spirit of the LGPL, compile a
binary Guile package, and try it with the new Guile-Foo.  But here's the
kicker:  compiling the define-record form from Guile *must* be able to
produce the same "name" for the generated temporary.

> One possibility is to include the source code location in addition to
> (or instead of) the form.

I am skeptical of this.  In my example above, let's say that I'm using a
record type from (rnrs exceptions).  Let's say that my change involves
some procedure in module/rnrs/exceptions.scm, but before the
define-record invocation, and that it results in the define-record being
on a different line.  Having my change cause a different temporary to be
generated would be quite surprising.

Cheers,

Andy
-- 
http://wingolog.org/

Fwd: `set!' of generated temporary in macro expansion causes warning

[Stefan Israelsson Tampe]

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

---------- Forwarded message ----------
From: Stefan Israelsson Tampe <stefan.itampe@gmail.com>
Date: Mon, Jun 13, 2011 at 1:03 PM
Subject: Re: `set!' of generated temporary in macro expansion causes warning
To: Andreas Rottmann <a.rottmann@gmx.at>


It looks like you just put the symbol in the current module, so maybe the
warning is
just unessesary.

here is the code in psyntax:

(define get-global-definition-hook
        (lambda (symbol module)
          (if (and (not module) (current-module))
              (warn "module system is booted, we should have a module"
symbol))
          (let ((v (module-variable (if module
                                        (resolve-module (cdr module))
                                        (current-module))
                                    symbol)))


so module parameter beeing #f will lead to a warning and an evaluation of
   (module-variable (current-module) symbol)

So if it's ok for the temporaries to be in the current-module then the
warning need to silenced.

E.g. In
(set! generate-temporaries
          (lambda (ls)
            (arg-check list? ls 'generate-temporaries)
(map (lambda (x) (wrap (gensym-hook) top-wrap #f)) ls)))

Replace #f with (current-module) and you will not have a warning. But My
psyntax-fu is
weak so this is just my 2c.

Be aware that changing this can pollute  current-module

/Stefan




On Mon, Jun 13, 2011 at 12:10 PM, Andreas Rottmann <a.rottmann@gmx.at>wrote:

> Hi!
>
> This is a much-stripped-down version of a `define-values' macro,
> exposing what seems like a psyntax bug:
>
> % cat guile-psyntax-temp-set-issue.scm
> (define-syntax define+set!
>  (lambda (form)
>    (syntax-case form ()
>      ((_)
>       (with-syntax (((mutable-id) (generate-temporaries '(id))))
>         #'(begin
>             (define mutable-id #f)
>             (set! mutable-id #t)))))))
>
> (define+set!)
>
> % guile guile-psyntax-temp-set-issue.scm
> ;;; note: auto-compilation is enabled, set GUILE_AUTO_COMPILE=0
> ;;;       or pass the --no-auto-compile argument to disable.
> ;;; compiling /home/rotty/tmp/guile-psyntax-temp-set-issue.scm
>
> ;;; WARNING (module system is booted, we should have a module #{ g65}#)
> ;;; compiled
> /home/rotty/.cache/guile/ccache/2.0-LE-8-2.0/home/rotty/tmp/guile-psyntax-temp-set-issue.scm.go
>
> Note the WARNING line; it seems during expansion something goes wrong
> wrt. to modules -- it'd be really nice if some psyntax guru can have a
> look at this.
>
> Regards, Rotty
> --
> Andreas Rottmann -- <http://rotty.yi.org/>
>
>

[-- Attachment #2: Type: text/html, Size: 3471 bytes --]