[m-rev.] for post-commit review: new names in make.dependencies.m

[Peter Wang]

On Fri, 06 Oct 2023 23:14:27 +1100 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> For review by Peter.
> 
> Zoltan.

> Improve names in make.dependencies.m.
> 
> compiler/make.dependencies.m:
>     Replace some names which were sort-of-but-not-quite-right
>     with names that correctly describe the operation they are attached to
> 
>     Note some possibilities for improvement, to be applied when
>     the old machinery is deleted.

> diff --git a/compiler/make.dependencies.m b/compiler/make.dependencies.m
> index d4342bc90..017c717e5 100644
> --- a/compiler/make.dependencies.m
> +++ b/compiler/make.dependencies.m
> @@ -397,8 +397,10 @@ compiled_code_dependencies(Globals, Deps, DepSpecs) :-
>          self_fact_table_files,
>          self_foreign_include_files,
>          self(module_target_int1),
> +        % XXX MDNEW The next two dep_specs should be a single
> +        % combined dep_spec, anc01_dir1_indir2.
>          ancestors(module_target_int1),
> -        imports_012
> +        anc0_dir1_indir2
>      ],
>  
>      globals.lookup_bool_option(Globals, intermodule_optimization, IntermodOpt),
> @@ -416,8 +418,17 @@ compiled_code_dependencies(Globals, Deps, DepSpecs) :-
>          ],
>          DepSpecsOpts = [
>              self(module_target_opt),
> +            % XXX MDNEW Given that we compute the set of intermod imports
> +            % for this dep_spec ...
>              intermod_imports(module_target_opt),
> -            intermod_imports_their_ancestors_and_012
> +            % ... why do we have to compute it AGAIN, as part of this
> +            % dep_spec as well?
> +            %
> +            % We should replace both of these dep_specs with one that
> +            % does the job of both but computes the set of intermod_imports
> +            % modules set just once. This would save even the cost of a
> +            % cache hit.
> +            anc0_dir1_indir2_of_ancestors_of_intermod_imports
>          ]
>      ;
>          AnyIntermod = no,

Tracing the history of that line (rather, its equivalent in the old
machinery) leads to commit 811c0af92, in which the redundant computation
was already present. The old machinery tried to express dependencies in
a "declarative" way with the `of` combinator, etc. The new machinery is
more willing to be a bit ad-hoc, namely with the anc0_dir1_indir2 and
anc0_dir1_indir2_of_ancestors_of_intermod_imports constructors, which
would make it easier to eliminate the redundant computation in this
case.

The diff looks okay.

Peter