[m-rev.] should we break up module_qual.m

[Zoltan Somogyi]

On Thu, 12 Nov 2015 01:40:49 +1100 (AEDT), "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> At the moment, we assume (in module_qual.qualify_items.m,
> in module_qualify_items_in_src_item_blocks, that nothing
> that a parent module imports is unused, because a child
> module may use it. The code that does it is this:
> 
>     (
>         Incls = []
>     ;
>         Incls = [_ | _],
>         % The submodule might make use of *any* of the imported modules.
>         % There is no way for us to tell which ones. So we conservatively
>         % assume that it uses *all* of them.
>         % XXX ITEM_LIST Fix this too-conservative behavior.
>         % If a submodule needs a module that the parent doesn't,
>         % it should import it for itself. Anything else may lead to
>         % unnecessary recompilations.
>         map.init(UnusedModules),
>         mq_info_set_as_yet_unused_interface_modules(UnusedModules, !Info)
>     ),
> 
> I would like to propose that we eliminate this assumption,
> and just delete this code. Having to have N children import
> a module instead just one parent may be a bit of extra work,
> but the time lost this way will be paid back many times over
> if some child does not need an import, since that child then
> won't be unnecessarily recompiled when the module imported
> changes its interface. Effectively, the cost does not grow
> with project lifetime, while the benefit does.

I actually tried this out, deleted the imports you now got warnings
about, added the imports that were required in other modules
to compensate for those deletions, and put the above code back.

The attached diff is the result. It does not change the behavior
of the compiler, it just changes how its parts import each other.

This diff would have two practical effects: it should reduce
the number of modules that will need to be recompiled
when we add a new compiler module (see the log message
for the reason), and it simplifies the rules governing the addition
of a new import_module declaration in a compiler module.
Previously, if you added an import of module A.B, you would
sometimes have to import module A, and sometimes you
wouldn't, because your own parent imported A. After this diff,
you would always have to add an import of module A, if there
wasn't already such an import in the current module.
(But see below.)

Does anyone object to committing this diff? (The diff itself
is huge and boring; the log message is worth reading.)

To allow us to maintain this change, I propose that we add
a new option, which would disable the above code if specified.
It could be called e.g. --report-unused-imports-in-parents.

> We should also consider changing the rules of inheritance
> between parent and child module so that the parent's imports
> are not made available to the child module automatically.
> We could simply not make them available at all, or we could
> make available only the imports that define entities (types,
> insts etc) that are needed to make sense of the other things
> (e.g. predicate declarations) in the implementation of the parent
> that the child gets access to via the parent's .int0 file.
> 
> What do people think?

While doing the above diff, I wondered about *why* the compiler
says that a module C importing module A.B is required to import
module A as well. What I could find in the log messages is that
we need to ensure that module A actually contains an include_module
declaration for A.B. That is a valid reason, but I don't think it justifies
making everything in the interface of A available to C; I think it just
means that when compiling C, we should check A's interface
to see if it contains an include_module for A.B. That can be done
without a full import.

Does anyone know of any other reason why we require an import
of A to be available to module C if module C imports A.B? I don't,
and as far as I can tell, the reference manual doesn't say; in fact,
I couldn't even find any mention of the requirement itself.

If noone knows of any other reason, I propose we change the de-facto
rules to NOT require the import of A into C when C imports A.B,
but to implement only the "does A include B?" check mentioned above.
Unfortunately, we would still have to read A's interface for that check,
but it should be sufficient to read A.int3, not A.int. Since .int3 files
change less frequently than .int files, this should still allow us to avoid
a lot of unnecessary recompilations when new modules are added.

For all I know, this may have been Fergus's intention when he
implemented submodules, but implementing this check without
actually doing an import may have been too hard with the
old item list program representation :-(

Zoltan.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: Log.no_parent_import
Type: application/octet-stream
Size: 1548 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20151112/e0844781/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: DIFF.no_parent_import
Type: application/octet-stream
Size: 194166 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20151112/e0844781/attachment-0001.obj>