[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