Strange undefined binding bug coupled to module system

Strange undefined binding bug coupled to module system

[Mikael Djurfeldt]

Loading of the following code:

foo.scm:
----------------------------------------------------------------------
(define-module (foo))

(define (encapsulate proc)
  (lambda (_) (proc _)))

(display round)
(newline)
(define round (encapsulate round))
----------------------------------------------------------------------

Gives (since some change during last spring):

guile> (load "foo.scm")
#<primitive-procedure round>

Backtrace:
In unknown file:
   ?: 0* [primitive-load "foo.scm"]
In foo.scm:
   8: 1* (define round (encapsulate round))
   8: 2* [encapsulate ...

foo.scm:8:15: While evaluating arguments to encapsulate in expression (encapsulate round):
foo.scm:8:15: Unbound variable: round
ABORT: (unbound-variable)


_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile

Re: Strange undefined binding bug coupled to module system

[Mikael Djurfeldt]

Mikael Djurfeldt <mdj@kvast.blakulla.net> writes:

> Loading of the following code:
>
> foo.scm:
> ----------------------------------------------------------------------
> (define-module (foo))
>
> (define (encapsulate proc)
>   (lambda (_) (proc _)))
>
> (display round)
> (newline)
> (define round (encapsulate round))
> ----------------------------------------------------------------------
>
> Gives (since some change during last spring):
>
> guile> (load "foo.scm")
> #<primitive-procedure round>
>
> Backtrace:
> In unknown file:
>    ?: 0* [primitive-load "foo.scm"]
> In foo.scm:
>    8: 1* (define round (encapsulate round))
>    8: 2* [encapsulate ...
>
> foo.scm:8:15: While evaluating arguments to encapsulate in expression (encapsulate round):
> foo.scm:8:15: Unbound variable: round
> ABORT: (unbound-variable)

OK, I've found the change which causes it:

2004-04-22  Dirk Herrmann  <dirk@dirk-herrmanns-seiten.de>

	(scm_m_define): Change order to first create binding and
	evaluating the expression afterwards.

While this change works in the R5RS situation without a module system,
the presence of a module system, with the difference between imported
and local bindings, introduces complications.

It seems like, in the case where no local binding exists before, that
local binding should be created *initialized* to the value of the
corresponding imported binding, if that exists.  That is not done now,
which causes the above described non-intuitive behavior.

Since it's not immediately obvious to me how to fix this in a good
way, I'll leave that to you guys.  But please keep in mind that
variable lookup is a substantial part of loading time in Guile.  We
wouldn't want to make *two* eval closure traversals by invoking
scm_sym2var twice (with different value of definep) or something like
that.

M


_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile

Re: Strange undefined binding bug coupled to module system

[Marius Vollmer]

Mikael Djurfeldt <djurfeldt@nada.kth.se> writes:

> 2004-04-22  Dirk Herrmann  <dirk@dirk-herrmanns-seiten.de>
>
> 	(scm_m_define): Change order to first create binding and
> 	evaluating the expression afterwards.
>
> While this change works in the R5RS situation without a module system,
> the presence of a module system, with the difference between imported
> and local bindings, introduces complications.

What would happen if you just revert that change?  Dirk?


_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile

Re: Strange undefined binding bug coupled to module system

[Mikael Djurfeldt]

Marius Vollmer <marius.vollmer@uni-dortmund.de> writes:

> Mikael Djurfeldt <djurfeldt@nada.kth.se> writes:
> 
> > 2004-04-22  Dirk Herrmann  <dirk@dirk-herrmanns-seiten.de>
> >
> > 	(scm_m_define): Change order to first create binding and
> > 	evaluating the expression afterwards.
> >
> > While this change works in the R5RS situation without a module system,
> > the presence of a module system, with the difference between imported
> > and local bindings, introduces complications.
> 
> What would happen if you just revert that change?

The change is a bugfix for another bug, so reverting it would swap one
bug for another.

The bug which Dirk fixed is that Guile previously created the binding
after the expression of the define had been evaluated, while R5RS
section 5.2.1 specifies that a define is like a set! to an existing
binding.  Thus, R5RS allows an expression like:

  (define foo (begin (set! foo 3) (* 5 foo)))

Chez Scheme accepts the above expression and Guile does it after
Dirk's change.  If we reverted the change, Guile would complain that
foo is an unbound variable, which is inconsistent with R5RS.

M


_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile

Re: Strange undefined binding bug coupled to module system

[Marius Vollmer]

Mikael Djurfeldt <djurfeldt@nada.kth.se> writes:

> Marius Vollmer <marius.vollmer@uni-dortmund.de> writes:
>
>> Mikael Djurfeldt <djurfeldt@nada.kth.se> writes:
>> 
>> > 2004-04-22  Dirk Herrmann  <dirk@dirk-herrmanns-seiten.de>
>> >
>> > 	(scm_m_define): Change order to first create binding and
>> > 	evaluating the expression afterwards.
>> >
>> > While this change works in the R5RS situation without a module system,
>> > the presence of a module system, with the difference between imported
>> > and local bindings, introduces complications.
>> 
>> What would happen if you just revert that change?
>
> The change is a bugfix for another bug, so reverting it would swap one
> bug for another.

Ahh, I see.  I'd say a good behavior for 'define' then is the following:

 - When the defined name has a local binding, use that binding.

 - When the defined name has an imported binding, look up the value of
   that binding and make a new local binding initialized with that
   value.

 - When the defined name has no binding, make a new one and initialize
   it with #<unspecified>.

Then the expression is evaluated and the new (or old local) binding
from above is set to the resulting value.

Ok?


_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile

Re: Strange undefined binding bug coupled to module system

[Mikael Djurfeldt]

Marius Vollmer <marius.vollmer@uni-dortmund.de> writes:

> I'd say a good behavior for 'define' then is the following:
>
>  - When the defined name has a local binding, use that binding.
>
>  - When the defined name has an imported binding, look up the value of
>    that binding and make a new local binding initialized with that
>    value.
>
>  - When the defined name has no binding, make a new one and initialize
>    it with #<unspecified>.
>
> Then the expression is evaluated and the new (or old local) binding
> from above is set to the resulting value.
>
> Ok?

Yes, and, in addition, you need to do

  - Evaluate the body

  - Set! the binding to the resulting value

Most of this is trivial.  However, step 2 requires some coding.  Let's
break down step 2:

  2.1 When the defined name has an imported binding
  2.2 Look up the value of that binding
  2.3 Make a new local binding initialized with that value

Steps 2.1 and 2.2 could be done by calling scm_sym2var with definep =
#f, but what you would *not* want to do is to call scm_sym2var with
definep = #t in step 2.3, because that would result in two traversals
of the module tree.

What's needed seems to be to factorize scm_sym2var a little.

Or, we could switch to the environments.c implementation!  :-)

M


_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile

Re: Strange undefined binding bug coupled to module system

[Marius Vollmer]

Mikael Djurfeldt <djurfeldt@nada.kth.se> writes:

> What's needed seems to be to factorize scm_sym2var a little.
>
> Or, we could switch to the environments.c implementation!  :-)

I have changed module-make-local-var! a bit:

  (define (module-make-local-var! m v)
    (or (let ((b (module-obarray-ref (module-obarray m) v)))
          (and (variable? b)
               (begin
                 ;; Mark as modified since this function is called when
                 ;; the standard eval closure defines a binding
                 (module-modified m)
                 b)))

        ;; No local variable yet, so we need to create a new one.  That
        ;; new variable is initialized with the old imported value of V,
        ;; if there is one.
        (let ((imported-var (module-variable m v))
              (local-var (or (and (module-binder m)
                                  ((module-binder m) m v #t))
                             (begin
                               (let ((answer (make-undefined-variable)))
                                 (module-add! m v answer)
                                 answer)))))
          (if (and imported-var (not (variable-bound? local-var)))
              (variable-set! local-var (variable-ref imported-var)))
          local-var)))

That should do it, or?


_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile

Re: Strange undefined binding bug coupled to module system

[Mikael Djurfeldt]

On Wed, 22 Dec 2004 16:35:18 +0100, Marius Vollmer
<marius.vollmer@uni-dortmund.de> wrote:
> I have changed module-make-local-var! a bit:
> 
>   (define (module-make-local-var! m v)
> [...]
>         (let ((imported-var (module-variable m v))
>               (local-var (or (and (module-binder m)
>                                  ((module-binder m) m v #t))
>                             (begin
>                               (let ((answer (make-undefined-variable)))
>                                 (module-add! m v answer)
>                                 answer)))))
> [...]
> 
> That should do it, or?

Well, unfortunately I don't have time to update myself on the module
system code well enough to know, but it seems OK. It's a relief that
it was much simpler to fix than I anticipated.

M


_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile