Strange code in modules.c

Strange code in modules.c

[Dirk Herrmann]

Hi Folks,

the following patch fixes a piece of code in modules.c which seems wrong 
to me. I am, however, not quite sure that I am not missing something. If 
there are no complaints I will submit the patch in the next couple of days.

RCS file: /cvsroot/guile/guile/guile-core/libguile/modules.c,v
retrieving revision 1.51
diff -u -r1.51 modules.c
--- libguile/modules.c  12 Sep 2003 15:11:09 -0000      1.51
+++ libguile/modules.c  21 Nov 2003 22:27:29 -0000
@@ -277,8 +277,7 @@
 module_variable (SCM module, SCM sym)
 {
 #define SCM_BOUND_THING_P(b) \
-  (!SCM_FALSEP(b) && \
-   (!SCM_VARIABLEP(b) || !SCM_UNBNDP (SCM_VARIABLE_REF (b))))
+  (SCM_VARIABLEP (b) && !SCM_UNBNDP (SCM_VARIABLE_REF (b)))
 
   /* 1. Check module obarray */
   SCM b = scm_hashq_ref (SCM_MODULE_OBARRAY (module), sym, SCM_UNDEFINED);

Best regards
Dirk



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: Strange code in modules.c

[Mikael Djurfeldt]

Dirk Herrmann <dirk@dirk-herrmanns-seiten.de> writes:

> the following patch fixes a piece of code in modules.c which seems
> wrong to me. I am, however, not quite sure that I am not missing
> something. If there are no complaints I will submit the patch in the
> next couple of days.
>
> RCS file: /cvsroot/guile/guile/guile-core/libguile/modules.c,v
> retrieving revision 1.51
> diff -u -r1.51 modules.c
> --- libguile/modules.c  12 Sep 2003 15:11:09 -0000      1.51
> +++ libguile/modules.c  21 Nov 2003 22:27:29 -0000
> @@ -277,8 +277,7 @@
>  module_variable (SCM module, SCM sym)
>  {
>  #define SCM_BOUND_THING_P(b) \
> -  (!SCM_FALSEP(b) && \
> -   (!SCM_VARIABLEP(b) || !SCM_UNBNDP (SCM_VARIABLE_REF (b))))
> +  (SCM_VARIABLEP (b) && !SCM_UNBNDP (SCM_VARIABLE_REF (b)))
>   /* 1. Check module obarray */
>    SCM b = scm_hashq_ref (SCM_MODULE_OBARRAY (module), sym, SCM_UNDEFINED);

It looks like the old code takes care of two kinds of obarrays:

1. Those which bind identifiers to locations represented by hash buckets.

2. Those which bind identifiers to locations represented by variables
   (in a hash bucket).

(The code assumes that variables aren't passed around as values on the
user level, which is probably wrong.)

I think Guile used to have both.  The "system" obarrays (the ones
which were initialized early before booting the module system) was of
the first kind and module system obarrays was of the second kind.

You'd better check that this is no longer the case, and that *all*
bindings use variables.  (That is of course the proper way.)

We should also be aware that the change you suggest introduces a kind
of backward incompatibility with regards to the module system.  I
doubt that many people have relied much on the module system API, though...

Best regards,
Mikael



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel

Re: Strange code in modules.c

[Dirk Herrmann]

Mikael Djurfeldt wrote:

>You'd better check that this is no longer the case, and that *all*
>bindings use variables.  (That is of course the proper way.)
>
>We should also be aware that the change you suggest introduces a kind
>of backward incompatibility with regards to the module system.  I
>doubt that many people have relied much on the module system API, though...
>  
>
Thanks for your comment. As far as I see it, the function scm_sym2var 
would throw an error if it ever encountered some non-variable object in 
an obarray (obtained from scm_eval_closure_lookup, which obtains it from 
module_variable, which is the function under discussion). Therefore I 
consider it safe to assume that Guile has for some time only supported 
obarrays holding variables.

Best regards
Dirk Herrmann



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/guile-devel