Re: wip-namespace review

[Andy Wingo]

Hi,

Thanks for the review. Some more background:

On Sat 24 Apr 2010 18:55, address@hidden (Ludovic Courtès) writes:

> Regarding the single module/variable name space, I was convinced that it
> was annoyingly constraining module names; however, as illustrated at
> <http://thread.gmane.org/gmane.lisp.guile.devel/10166> that constraint
> has vanished somehow, making it less of a problem IMO.

It still does constrain module names, but only if you have a local
variable of the same name. If I even do (define bar ...) in my '(foo)
module, I can't define a '(foo bar) module. This is undocumented, leads
to poorly-understood errors, and is just wrong ;-)

> Besides, you said at
> <http://thread.gmane.org/gmane.lisp.guile.devel/10132> that this is “not
> something we can change right now.”.  :-)

I realized that we could change it in a backwards-compatible way. My
experience points have allowed me to level up in Refactoring ;)

> So, apologize if that should be obvious to me, but can you please
> re-explain why this is important?

It's important to me to get things right, and if we can do that without
downsides, then we should.

Beyond that, I don't want to merge in r6rs support that has hacks around
the module system (cf recent thread about (rnrs) and (rnrs
syntax-case)). The version comparison code is already complicated enough
as it is; I would rather not maintain hacks in the future.

> To app or not to %app?

So, this question is a slightly different one ;) It used to be that
there was an (app modules ...) hierarchy, traversed through the normal
value namespace. The root was the module known as (guile), and it had a
binding for `app'. Module resolution was implemented in resolve-module
to resolve with respect to (guile), via "full names" -- so one would
resolve, for example, (app modules guile), and that found the root of
the hierarchy via the `app' binding in the (guile) module.

This provided the illusion of a separate root, at (app), or even above
it, but that was never the case, because (resolve-module '(guile)) =
(resolve-module '(guile app modules guile)).

At some point it was realized that if you had a script, if you defined a
variable named "app", you were screwed -- because the (guile-user)
module shared an obarray with (guile), and that would overwrite the
binding for "app", overwriting the root of the module resolution tree.

So, app was changed to %app. Only there was some code out there that did
(nested-ref the-root-module '(app module ...)), so "app" had to stay
around -- but there was no deprecation mechanism to encourage users to
switch to %app.

>   commit 30ce621c5ac9b67420a9f159b2195f6cd682e237
>   Author: Andy Wingo <address@hidden>
>   Date:   Thu Apr 22 15:08:13 2010 +0200
>
>       (app modules) -> (%app modules)

This kind of commit is simply to switch away from the deprecated "app"
to "%app".

Now, "%app" is still something of a hack; instead of doing that wierd
thing in resolve-module where resolving '(foo) actually resolved '(app
modules foo), why not make resolve-module close over the module formerly
addressed as (app modules), and just resolve modules relative to that
new root, the module that is returned when doing a (resolve-modules '()
#f) ? Then we can deprecate the "%app" binding, and never need to
document it.

>   commit cb67c838f5658a74793f593c342873d32e4a145c
>   Author: Andy Wingo <address@hidden>
>   Date:   Thu Apr 22 15:25:09 2010 +0200
>
>       deprecate %app

That's what we're up to here.

So, we've gotten rid of %app, but old code will still work if
--enable-deprecated is used.

Now that we've simplified the resolve-module mechanism, we can look at
separating the value namespace from the module namespace. Basically we
add module-ref-submodule and module-define-submodule!, which will in the
future access that second namespace but whose initial definition is in
terms of module-ref.

Then we make nested-ref etc traverse submodules via
module-ref-submodule, and when they reach the last element in the nested
name they use nested-ref. We also define nested-ref-module, which uses
module-ref-module for the last element.

Then eventually we add a submodules array to modules themselves, switch
module-ref-submodule to use that, and voila separated namespaces, with
back-compat in deprecated.scm.

>   diff --git a/libguile/modules.c b/libguile/modules.c
>   index ccb68b7..c8a4c2a 100644
>   --- a/libguile/modules.c
>   +++ b/libguile/modules.c
>   @@ -44,7 +44,16 @@ scm_t_bits scm_module_tag;
>
>    static SCM the_module;
>
>   +static SCM module_make_local_var_x_var;
>
> Please keep comments above variables, or add one for all of them (info
> "(standards) Comments").

OK, will do.

> There also needs to be the C counterpart:
>
>   #define scm_module_index_import_public_interface 9
>
>   #define SCM_MODULE_PUBLIC_INTERFACE(module) \
>     SCM_PACK (SCM_STRUCT_DATA (module)
>     [scm_module_index_public_interface])
>
> and change ‘scm_module_public_interface ()’ to use that instead of
> ‘scm_call_1 (...)’.

This won't work in general, because in the case in which you have
deprecated code enabled, you still need to go through the Scheme
function (overridden by ice-9 deprecated).

Regarding the indices, only a subset of module's fields currently have
record indices and accessors defined. Given that these would be new C
interfaces, I would prefer to avoid defining them unless there were a
need.

> Finally there needs to be ‘scm_set_module_public_interface_x ()’ as
> discussed at <http://thread.gmane.org/gmane.lisp.guile.devel/10136>.

I'm not sure; Lilypond seems to be using this interface as:

      mod = scm_call_0 (maker);
      scm_module_define (mod, ly_symbol2scm ("%module-public-interface"),
                         mod);

Which is a verbose (and somewhat incorrect) way to say, "export
everything in this module". Perhaps, given that this would be a new
interface, we should just provide a more appropriate interface --
module-export-all! or something.

Andy
-- 
http://wingolog.org/