[m-rev.] for review: fix silent failure of making .int file
Julien Fischer
jfischer at opturion.com
Fri Dec 17 12:26:47 AEDT 2021
On Thu, 16 Dec 2021, Zoltan Somogyi wrote:
> Fix a possible silent failure of making .int files.
>
> When I split two files in a recent change, I ran into an annoying problem.
> The problem was caused by unneeded imports in the interface of the new modules,
> whose initial part I originally simply copied from the source module.
> The problem was that when I attempted to compile new module A which imported
> new module B, the compilation of module A would fail with a message about
> not finding module B's .int file. It couldn't find B.int because the
> compiler invocation that was supposed to create it failed, but it did not
> print any error message about *why* it failed, and as a consequence,
> it also did not set the exit status to nonzero to tell mmake that
> the file was not actually built, so later build actions that need that file
> should not be executed.
>
> The cause of this problem was the following.
>
> - The default value of the --warn-unused-imports is off, but COMP_FLAGS
> turns it on for modules in the compiler directory. This enables warnings
> generated by unused_imports.m.
>
> - There is code that does a similar job in module_qual.qual_errors.m, but
> that one is limited to imports in interface sections. Due to the overlap
> between tasks, when this code finds an unused import_module declaration
> in an interface, it generates an error message that was conditional
> on --warn-unused-imports being off. When it was on, as it is with
> COMP_FLAGS, it generates an error_spec that, when given to write_error_specs,
> generates no output.
>
> - Code in write_module_interface_files.m that decided whether the building
> of the .int file has failed, tested only whether the process of generating
> its contents has returned any error_specs, not whether it returned
> any error_specs that would actually be printed, and, by being printed
> with a sufficiently high severity, would set the exit status to signal
> failure.
>
> compiler/error_util.m:
> The two changes to this file together fix the root cause of this problem.
>
> First, a new predicate checks whether an error_spec has any part
> whose printing is NOT disabled by being attached to an unmet condition.
>
> Second, the predicate through which we pass all error_specs created
> during the generation of the contents of the .int file filters out
> any error_specs that yield no output.
>
> The later changes are not strictly part of the bugfix, they are there
> simply to make the code simpler to understand, in the hope that this fact
> will reduce the probability of similar problems in the future.
>
> compiler/module_qual.qual_errors.m:
> Instead of generating error_specs that are conditional on
> --warn-unused-imports being OFF, generate them condition on
s/condition/condtional/
That's fine otherwise.
Julien.
More information about the reviews
mailing list