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

[Julien Fischer]

Hi Zoltan,

On Thu, 12 Nov 2015, Zoltan Somogyi wrote:

> 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.)

No objections from me.

> 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.

I have no objections to the proposed change.

Julien.