[m-rev.] for post-commit review: update comments in file_names.m
Julien Fischer
jfischer at opturion.com
Fri Jul 7 18:36:15 AEST 2023
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.
More information about the reviews
mailing list