[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