Re: Strange code in modules.c

[Mikael Djurfeldt]

Dirk Herrmann <address@hidden> 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