[m-rev.] for post-commit review: file names 1

Julien Fischer jfischer at opturion.com
Wed May 31 20:47:45 AEST 2023


On Wed, 31 May 2023, Zoltan Somogyi wrote:

> Improve infrastructure for file name translations.
> 
> compiler/file_names.m:
>     Document the meaning of the maybe_create_dirs and maybe_search types.
>
>     Delete long-obsolete references to .il files.
> 
> compiler/make.util.m:
>     Rename make_remove_target_file to remove_make_target_file, since this
>     predicate removes target files in Makefiles, and does not "make" anything.
>     Rename several other predicates in a similar manner, for the same reason.
>
>     Add an extra argument to get_file_name and some related predicates
>     that will allow future conditionally-enabled trace goals in the compiler
>     to track where the requests for file name translations come from.
> 
> compiler/write_deps_file.m:
>     Factor out some code.
> 
> compiler/make.dependencies.m:
> compiler/make.module_dep_file.m:
> compiler/make.module_target.m:
> compiler/make.program_target.m:
>     Conform to the changes above.
> 
> tools/file_name_translation_stats:
>     Allow for large numbers of file name translations.
> diff --git a/compiler/file_names.m b/compiler/file_names.m
> index b2401153e..26fd89d38 100644
> --- a/compiler/file_names.m
> +++ b/compiler/file_names.m
> @@ -76,10 +76,47 @@
>  % - Repeat the exercise with the proposed replacement code, and
>  %   compare the results to the baseline.
> 
> +    % If the proper place for a file is in a subdirectory (e.g. Mercury/css),
> +    % but the subdirectory does not exist, which in this case may mean either
> +    %
> +    % - that Mercury exists but Mercury/css does not, or
> +    % - that Mercury does exist, which of course means that Mercury/css

I think you mean that Mercury does *not* exist there.

> +    %   does not exist either,
> +    %
> +    % values of this type tell the predicate they are given to whether it
> +    % should create any such missing directories.
>  :- type maybe_create_dirs
>      --->    do_create_dirs
>      ;       do_not_create_dirs.
> 
> +    % For some kinds of files, we know exactly where (in which directory

I would just say:

     we know exactly in which directory they should be ...

> +    % they should be; for other kinds, we may have to search several
> +    % directories. For the latter, our clients will need to call the
> +    % module_name_to_search_file_name predicate, which internally sets
> +    % MaybeSearch to do_search.

That looks fine otherwise.

Julien.


More information about the reviews mailing list