[m-rev.] for post-commit review: update comments in file_names.m

[Julien Fischer]

On Thu, 6 Jul 2023, Zoltan Somogyi wrote:

> For review by anyone. I would like people's feedback
> on the parts marked with "XXX DODGY".

...

> Update comments in file_names.m.
> 
> compiler/file_names.m:
>     Delete now-obsolete comments. Move their still-relevant parts
>     next to the entity that they apply to.
>
>     List the string form, if any, of each enum representing an extension.
>
>     Explain the meaning of _gs, _ngs and _opt suffixes.
>
>     Delete obsolete commented-out code.
>
>     Delete an obsolete, unused type definition.
> 
> compiler/options.m:
> doc/user_guide.texi:
>     Fix two bugs that a comment that this diff pointed to.
>
>     First, since 2017, the --dump-hlds option has output C code
>     only if the target language is C, but the documentation
>     of that option did not mention this. Fix this omission.
>
>     Second, ever since we split .mh and .mih files, the header
>     part of MLDS dumps in C form were output to .mih_dump file.
>     However, the documentation of the --dump-hlds  option said that
>     it was output to a .h_dump file. Fix this as well.
> 
> diff --git a/compiler/file_names.m b/compiler/file_names.m
> index 26c3399e5..11eb90163 100644
> --- a/compiler/file_names.m
> +++ b/compiler/file_names.m

...

>  %---------------------%
> 
> +% Values of the "ext" type partition the set of filename extensions
> +% that the Mercury compiler cares about into several categories.
> +% Generally speaking, the extensions in each category are treated
> +% the same by the module_name_to_* predicates below. (There are exceptions,
> +% but they should be eliminated soon).
> +%
> +% The two considerations that control which category a given extension
> +% belongs to are
> +%
> +% - the purpose of the files with that extension, and
> +% - how the placement of files with that extension in directories is affected
> +%   by the --use-subdir and --use-grade-subdir options being set.
> +%
> +% Generally speaking, the places where the files using a given extension
> +% (or category of extensions) will usually be either
> +%
> +% - the current directory,
> +% - a non-grade-specific subdirectory, which will be "Mercury/<X>s"
> +%   for some string X,
> +% - a grade-specific subdirectory, which will be
> +%   "Mercury/<grade>/<arch>/Mercury/<X>s" for some string X.
> +%   (See make_grade_subdir_file_name) for the rationale for this scheme.)
> +%
> +% Java extensions are an exception; they include an extra "jmercury" component
> +% in the path.

Some Java extensions (.java and .class specifically) have an extra component.
.jar, for example, does not.

> +%
> +% There are several approaches we can use to decide which directory the files
> +% using an extension should be put into. The main ones are the following.
> +%
> +% - The files of some extensions are always stored in the current directory.
> +%
> +% - The files of some extensions are stored in a non-grade-specific
> +%   subdirectory if --use-subdirs is specified, and in the current directory
> +%   otherwise.
> +%
> +% - The files of some extensions are stored in a grade-specific subdirectory
> +%   if --use-grade-subdirs is specified, and in the current directory
> +%   otherwise.
> +%
> +% - The files of some extensions are stored in a grade-specific subdirectory
> +%   if --use-grade-subdirs is specified, in a non-grade-specified dubdirectory

s/dubdrictory/subdirectory/

> +%   if --use-grade-subdirs is not specified but --use-subdirs is, and
> +%   in the current directory otherwise.
> +%
> +% (Note that --use-grade-subdirs is not *intended* to be actually usable
> +% with mmake.)
> +%
> +% However, other approaches also exist, though they are all variations
> +% on these themes.
> +%
> +% One common variation is that when constructing a filename to search for,
> +% we never include any directory name component in the filename we return.
> +% XXX There are extension classes for which we *do* return directory name
> +% components even when constructing a filename to be searched for.
> +% It is not (yet) clear to me (zs) whether this is an intentional choice,
> +% or whether what we do with do_search is immaterial because we *always*
> +% translate those extensions with do_not_search.
> +%
> +% In the function symbols below,
> +% - the "gs" suffix stands for the use of a grade-specific directory, while
> +% - the "ngs" suffix stands for the use of a non-grade-specific directory.
> +%
> +% XXX We should probably invert the classification scheme.
> +% Instead of making the role (interface file, target file, object file, etc)
> +% be the primary division point, and the directory treatment the second
> +% division point, we should make the directory treatment the first one.
> +% However, there is no point in dong that until we have firmly, and
> +% *explicitly*, decided the directory treatment we want for each and every
> +% extension. (The current system was arrived at by piling patch upon patch
> +% on a large piece of over-complex code, and cannot be considered to
> +% represent the result of explicit deliberation.)
> +
>  :- type ext
>      --->    ext_src
>              % Mercury source files.
> @@ -160,6 +236,11 @@
>              %
>              % Most of these extensions are intended to name real files,
>              % but some are intended to name mmake targets.
> +            %
> +            % XXX According to the documentation of the --user-grade subdirs

--use-grade-subdirs

> +            % option, *all* executables and libraries *should* be put
> +            % into a grade subdir if that option is specified, not just some.
> +            % They should then be copied or linked to the current directory.

...



>
>      ;       ext_lib(ext_lib)
>      ;       ext_lib_gs(ext_lib_gs)
> @@ -177,6 +258,11 @@
>              %
>              % Most of these extensions are intended to name real files,
>              % but some are intended to name mmake targets.
> +            %
> +            % XXX According to the documentation of the --user-grade subdirs

Ditto.

> +            % option, *all* executables and libraries *should* be put
> +            % into a grade subdir if that option is specified, not just some.
> +            % They should then be copied or linked to the current directory.
>
>      ;       ext_mmake_fragment(ext_mmake_fragment)
>              % Compiler-generated files that are designed to be bodily included
> @@ -217,149 +303,181 @@
>      ;       ext_misc_gs(ext_misc_gs).
>              % XXX Document me.
>


...


> +%-------------%
> +
> +% Each extension specification is followed below
> +%
> +% - either by a string giving the extension (the usual case),
> +% - the name of an option giving the extension, or
> +% - a prefix and the name of an option giving the rest of the extension.
> +

>  :- type ext_obj
> -    --->    ext_obj_dollar_o
> -    ;       ext_obj_dollar_efpo
> -    ;       ext_obj_o
> -    ;       ext_obj_pic_o
> -    ;       ext_obj_obj_opt
> -    ;       ext_obj_pic_obj_opt.
> +    --->    ext_obj_dollar_o            % ".$O"
> +    ;       ext_obj_dollar_efpo         % ".$(EXT_FOR_PIC_OBJECTS)"
> +    ;       ext_obj_o                   % ".o"
> +    ;       ext_obj_pic_o               % ".pic_o"
> +    ;       ext_obj_obj_opt             % object_file_extension option
> +    ;       ext_obj_pic_obj_opt.        % pic_object_file_extension option

I suggest adding a comment that ".obj" is handled by the ext_obj_dollar_o case.

>  :- type ext_init_obj
> -    --->    ext_init_obj_dollar_o
> -    ;       ext_init_obj_o
> -    ;       ext_init_obj_pic_o
> -    ;       ext_init_obj_obj_opt
> -    ;       ext_init_obj_pic_obj_opt.
> +    --->    ext_init_obj_dollar_o       % ".init.$O"
> +    ;       ext_init_obj_o              % ".init.c"
> +    ;       ext_init_obj_pic_o          % ".init.pic_o"
> +    ;       ext_init_obj_obj_opt        % "_init" ++ object_file_extension
> +    ;       ext_init_obj_pic_obj_opt.   % "_init" ++ pic_object_file_extension

Similarly for "_init.obj"


>  :- type ext_lib
> -    --->    ext_lib_dollar_efsl
> -    ;       ext_lib_lib
> -    ;       ext_lib_so.
> +    --->    ext_lib_dollar_efsl         % ".(EXT_FOR_SHARED_LIB)"
> +    ;       ext_lib_lib                 % ".lib"
> +    ;       ext_lib_so.                 % ".so"
>
>  :- type ext_lib_gs
> -    --->    ext_lib_gs_dollar_a
> -    ;       ext_lib_gs_archive
> -    ;       ext_lib_gs_dll
> -    ;       ext_lib_gs_init
> -    ;       ext_lib_gs_jar
> -    ;       ext_lib_gs_lib_opt
> -    ;       ext_lib_gs_sh_lib_opt.
> +    --->    ext_lib_gs_dollar_a         % ".$A"
> +    ;       ext_lib_gs_archive          % ".a"
> +    ;       ext_lib_gs_dll              % ".dll"
> +    ;       ext_lib_gs_init             % ".init"
> +    ;       ext_lib_gs_jar              % ".jar"
> +    ;       ext_lib_gs_lib_opt          % library_extension
> +    ;       ext_lib_gs_sh_lib_opt.      % shared_library_extension
>
>  :- type ext_mmake_fragment
> -    --->    ext_mf_d
> -    ;       ext_mf_dv
> -    ;       ext_mf_dep
> -    ;       ext_mf_dir_sl_all_os.    % DODGY
> +    --->    ext_mf_d                    % ".d"
> +    ;       ext_mf_dv                   % ".dv"
> +    ;       ext_mf_dep                  % ".dep"
> +    % XXX DODGY This extension is use in only one place, in write_deps_file.m.
> +    % It looks strange to me (zs), because I have no idea where ".dir"
> +    % comes from, or what system component puts object files there.
> +    ;       ext_mf_dir_sl_all_os.       % ".dir/*.$O"

I don't recall even seeing that before.

>  :- type ext_user_ngs
> -    --->    ext_user_ngs_defn_ext
> -    ;       ext_user_ngs_defn_lc
> -    ;       ext_user_ngs_defns
> -    ;       ext_user_ngs_imports_graph
> -    ;       ext_user_ngs_lct
> -    ;       ext_user_ngs_lct_order
> -    ;       ext_user_ngs_mode_constr
> -    ;       ext_user_ngs_order_to
> -    ;       ext_user_ngs_type_repns
> -    ;       ext_user_ngs_xml.
> +    --->    ext_user_ngs_defn_ext       % ".defn_extents"
> +    ;       ext_user_ngs_defn_lc        % ".defn_line_counts"
> +    ;       ext_user_ngs_defns          % ".defns"
> +    ;       ext_user_ngs_imports_graph  % ".imports_graph"
> +    ;       ext_user_ngs_lct            % ".local_call_tree"
> +    ;       ext_user_ngs_lct_order      % ".local_call_tree_order"
> +    ;       ext_user_ngs_mode_constr    % ".mode_constraints"
> +    ;       ext_user_ngs_order_to       % ".order_trans_opt"
> +    ;       ext_user_ngs_type_repns     % ".type_repns"
> +    ;       ext_user_ngs_xml.           % ".xnl"

s/.xnl/.xml/ there.

>
>  :- type ext_analysis
> -    --->    ext_an_analysis
> -    ;       ext_an_date
> -    ;       ext_an_status
> -    ;       ext_an_imdg
> -    ;       ext_an_request.
> +    --->    ext_an_analysis             % ".analysis"
> +    ;       ext_an_date                 % ".analysis_date"
> +    ;       ext_an_status               % ".analysis_status"
> +    ;       ext_an_imdg                 % ".imdg"
> +    ;       ext_an_request.             % ".request"
>
>  :- type ext_bytecode
> -    --->    ext_bc_mbc
> -    ;       ext_bc_bytedebug.
> +    --->    ext_bc_mbc                  % ".bc"
> +    ;       ext_bc_bytedebug.           % ".bytedebug"
> +
> +% XXX The extensions in the two misc categories probably belong
> +% in one or another category above, but I (zs) am not sure where
> +% each *would* belong.
>
>  :- type ext_misc_ngs
> -    --->    ext_misc_ngs_module_dep     % DODGY
> -    ;       ext_misc_ngs_err_date       % DODGY
> -    ;       ext_misc_ngs_prof.          % DODGY
> +    --->    ext_misc_ngs_module_dep     % ".module_dep"
> +            % XXX DODGY What is the correctness argument for making this
> +            % a NON-grade-specific extension? If *anything* in a .module_dep
> +            % file can *ever* be grade dependent, this should be a
> +            % grade-specific extension.
> +    ;       ext_misc_ngs_err_date       % ".err_date"
> +            % XXX DODGY If you recompile a module in a different grade,
> +            % the contents of the .err file may change, for example
> +            % because one grade satisfies the requirements of a
> +            % require_feature_set declaration and the other does not.
> +            % To me (zs), this argues in favor of .err_date files
> +            % belonging in a grade-specific directory. The fact that
> +            % it would obviously be harder to people to find them there
> +            % is no relevant, since people shouldn't *have* to find
> +            % .err_date files.

Agreed.

> +    ;       ext_misc_ngs_prof.          % ".prof"
> +            % XXX DODGY Given that different profiling grades generate
> +            % different profiles (specifically, they produce different subsets
> +            % of the whole set of kinds of info that the non-deep profiler can
> +            % generate), shouldn't this be a grade-specific extension?

I'm going to make a guess and say that was just overlooked.  (Given that it's
rare to install more than one .prof grade I can see that happening.)

Julien.