[m-rev.] for review: reverse the order of compiled_code_dependencies

Peter Wang novalazy at gmail.com
Sat Dec 10 11:14:59 AEDT 2022


On Sat, 10 Dec 2022 02:37:48 +1100 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> The addition of inst_preserving_condense is separate
> from the change to make.dependencies.m, but it is needed
> by that change.
> 
> I have bootchecked these diffs in hlc, and the bootcheck
> in the java grade has just started. If it works, I intend to
> commit these diffs after a review, preferably tomorrow, but then stop
> making changes to make*.m for a day, to allow any problems
> possibly caused by this diff to surface. There shouldn't be any
> if the rules of the targets whose order this diff changes
> describe their own dependencies correctly, but it may be that
> they do not.
> 
> Zoltan.

> Reverse the order of compiled_code_dependencies.
> 
> The old order never made sense, and probably worked only
> because of dependencies between the targets listed effectively
> forced the targets in the list to be built in the reverse order anyway.

> diff --git a/compiler/make.dependencies.m b/compiler/make.dependencies.m
> index 3c8eefec9..084e68486 100644
> --- a/compiler/make.dependencies.m
> +++ b/compiler/make.dependencies.m
...
> +
> +    % XXX We used to build stage 0 deps, then stage 1 deps etc,
> +    % but since we put the previous stages' dependencies *after*
> +    % the dependencies of the previous stages, we ended up with a list
> +    % that is equivalent to this commented-out assignment to DepsAll:
> +    %
> +    % DepsAll = inst_preserving_condense(
> +    %     [DepsRegistries, DepsOpts, DepsSrcInts, DepsTracks]),
> +    %
> +    % This looked very wrong. For example, it called for imported modules'
> +    % .opt files to be built before their .int files, which the dependencies
> +    % of the .opt files on their corresponding .int files would not allow
> +    % anyway. And, as it turned out, returning dependencies in the obvious
> +    % order works as well.
> +    DepsAll = inst_preserving_condense(
> +        [DepsTracks, DepsSrcInts, DepsOpts, DepsRegistries]),
> +    Deps = combine_deps_list(DepsAll).

This changes the order that dependency sets will be *found*
but the dependency sets are unioned into a single set by
combine_deps_list either way. It has no bearing on the order that
dependencies will be built.

The change is fine, but the commit message and comment needs to be
adjusted.

Peter


More information about the reviews mailing list