[m-rev.] for post-commit review: simplify success/failure testing

Peter Wang novalazy at gmail.com
Mon Dec 11 13:15:19 AEDT 2023


On Sun, 10 Dec 2023 12:01:30 +1100 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> Simplify success vs failure testing.
> 
> compiler/make.program_target.m:
>     Simplify the code that tests for the presence of errors. Do this by
> 
>     - deleting the DepsSucceeded argument of the build_linked_target and
>       build_linked_target_2 predicates, because their caller has tested
>       its value before the call and knows that it is "succeeded",
> 
>     - refining the type of the next argument in both predicates
>       to also reflect the fact that the caller has already tested it
>       for the absence of failure,
> 
>     - using variables that reflect the success or failure of individual
>       steps in the overall process, instead of variables that encode
>       the success/failure of some combination of steps, but whose names
>       did not make clear *which* combination of steps they were about.
> 
>     Add XXXs describing situations in which the outcomes of the old logic,
>     which the simplified logic preserves, seem to be suboptimal.
> 
>     Break up a too-large predicate.
> 
>     Give a predicate a more descriptive names.
> 
> compiler/make.check_up_to_date.m:
>     Split the lhs_result type into two types, to allow the second of the
>     simplifications above.
> 
>     Give some predicates more descriptive names.
> 
> compiler/make.module_target.m:
>     Conform to the change in make.check_up_to_date.m.

> diff --git a/compiler/make.program_target.m b/compiler/make.program_target.m
> index 4c2d2c3ed..663e7000d 100644
> --- a/compiler/make.program_target.m
> +++ b/compiler/make.program_target.m
> @@ -548,6 +554,87 @@ build_linked_target_2(ProgressStream, Globals, MainModuleName, FileType,
>      AllModulesList = set.to_sorted_list(AllModules),
>      (
>          FileType = executable,
> +        make_init_obj_file_check_result(ProgressStream, NoLinkObjsGlobals,
> +            MainModuleName, AllModulesList, InitObjSucceeded, InitObjects,
> +            !Info, !IO)
> +    ;
> +        ( FileType = static_library
> +        ; FileType = shared_library
> +        ; FileType = csharp_executable
> +        ; FileType = csharp_library
> +        ; FileType = java_executable
> +        ; FileType = java_archive
> +        ),
> +        InitObjSucceeded = succeeded,
> +        InitObjects = []
> +    ),
...

Hmm, are you using the 'patience' or 'histogram' git diff algorithms?
It turns out that the (default) 'myers' or 'minimal' algorithms happen
to produce a much more reasonable diff for this part of the commit.
Not sure if it's something to extrapolate for future code-movement type
changes.

The diff looks fine.

Peter


More information about the reviews mailing list