[m-rev.] for review: Don't write .module_dep file when source file contains unexpected module.

[Julien Fischer]

On Tue, 14 Jan 2020, Peter Wang wrote:

> If the user forgets to run mmc -f before running mmc --make, then
> mmc --make could create a .module_dep file for a module it expects
> to be in a source file, even if the source file is actually holds a
> different module. To recover from the mistake, the user would need to
> delete the .module_dep file manually.
>
> compiler/make.module_dep_file.m:
>    Make make_module_dependencies stop if read_module_src finds an
>    unexpected module name.
>
>    Write down a couple of problematic situations when checking
>    if .module_dep files are up-to-date.

...

> compiler/make.module_dep_file.m | 56 +++++++++++++++++++++++++++++----
> 1 file changed, 50 insertions(+), 6 deletions(-)
>
> diff --git a/compiler/make.module_dep_file.m b/compiler/make.module_dep_file.m
> index c721ef2ed..e81cbde1d 100644
> --- a/compiler/make.module_dep_file.m
> +++ b/compiler/make.module_dep_file.m
> @@ -2,6 +2,7 @@
> % vim: ft=mercury ts=4 sw=4 expandtab
> %-----------------------------------------------------------------------------%
> % Copyright (C) 2002-2009, 2011 The University of Melbourne.
> +% Copyright (C) 2014-2017, 2019-2020 The Mercury team.
> % This file may only be copied under the terms of the GNU General
> % Public License - see the file COPYING in the Mercury distribution.
> %-----------------------------------------------------------------------------%
> @@ -186,6 +187,11 @@ do_get_module_dependencies(Globals, RebuildModuleDeps, ModuleName,
>             % Since the source file was found in this directory, do not use
>             % module_dep files which might be for installed copies
>             % of the module.
> +            %
> +            % XXX SourceFileName may not actually be the correct source file
> +            % for the required module. Usually the purported source file would
> +            % have a later timestamp than the .module_dep file, though, so the
> +            % other branch would be taken.
>             read_module_dependencies_no_search(Globals, RebuildModuleDeps,
>                 ModuleName, !Info, !IO)
>         else
> @@ -223,6 +229,23 @@ do_get_module_dependencies(Globals, RebuildModuleDeps, ModuleName,
>                 then
>                     true
>                 else
> +                    % XXX The existence of a .module_dep file reflects a
> +                    % previous state of the workspace which may not match the
> +                    % current workspace.
> +                    %
> +                    % Here is a (contrived) case where we run into an issue:
> +                    % 1. create prog.m which imports the standard lexer module

standard library lexer

> +                    % 2. copy standard library lexer.m to current directory
> +                    %    for editing
> +                    % 3. run mmc --make; it creates lexer.module_dep
> +                    % 4. change lexer.m into a sub-module of prog
> +                    % 5. run mmc --make again, it no longer works
> +                    %
> +                    % The local lexer.module_dep prevents mmc --make finding
> +                    % the lexer.module_dep from the standard library, even
> +                    % though there is no longer any local source file for the
> +                    % `lexer' module.
> +
>                     make_module_dependencies(Globals, ModuleName, !Info, !IO)
>                 )
>             ;
> @@ -769,16 +792,35 @@ make_module_dependencies(Globals, ModuleName, !Info, !IO) :-
>             do_not_ignore_errors, do_not_search, ModuleName, [],
>             SourceFileName, always_read_module(do_return_timestamp), _,
>             ParseTreeSrc, Specs0, ReadModuleErrors, !IO),
> +
>         set.intersect(ReadModuleErrors, fatal_read_module_errors, FatalErrors),
>         ( if set.is_non_empty(FatalErrors) then
> +            FatalReadError = yes,
> +            DisplayErrorReadingFile = yes
> +        else if set.contains(ReadModuleErrors, rme_unexpected_module_name) then
> +            % If the source file does not contain the expected module
> +            % then do not make the .module_dep file.

Either say why here, or add a pointer to back to the documentation that
gives the reason.

> +            FatalReadError = yes,
> +            DisplayErrorReadingFile = no
> +        else
> +            FatalReadError = no,
> +            DisplayErrorReadingFile = no
> +        ),
> +        (
> +            FatalReadError = yes,

Looks fine otherwise.

Julien.