[m-rev.] for review: start using the new code in file_names.m

[Julien Fischer]

Hi Zoltan,

There are couple of comment below which refer to things you added in
previous diffs (that I haven't had time to review yet).

On Wed, 7 Jun 2023, Zoltan Somogyi wrote:

> For review by Julien; the diff has an XXX marked for you.
>
> I have bootchecked this in hlc.gc and csharp grades with
> only the expected number of test case failures; I just started
> a java bootcheck. However, this is the kind of change I would
> like to have tested fairly thoroughly, which is why
>
> - the diff sets things up that by default, the compiler checks
>  the old and new filename construction algorithms against
>  each other, and aborts if they differ, but
>
> - the abort can be avoided by setting the environment variable
>  named in the log message.
>
> I intend to inspect the effect of this diff, once committed, on
> the nightly tests for any failures caused by such aborts.
>
> After a couple of weeks with no reported problems, I will delete the
> old code that the new code is intended to replace.

I will set up some bootchecks using the infrastructure I use to test the
releases.  (If you have specific things you want to check let me know,
the machine I am using will be considerably quicker than the one that
runs the nightly tests.)

I will also check the effects of this change on the Opturion code base.

> Start using the new code in file_names.m.
> 
> compiler/file_names.m:
>     Change the argument vectors of the predicates that compute filenames
>     by taking *two* arguments to specify the extension: adding an argument
>     value of the "newext" type right after the old "ext" type. To make this
>     possible, export the newext type.
>
>     By default, use the new argument to do every filename computation twice,
>     with the old and new algorithms, throwing an exception if their results
>     differ. (There is no easy way to test whether the "make-any-needed-dirs"
>     part was done the same way, but this is reasonably easy to check
>     visually in the code.)
>
>     In case an exception does get thrown, this can be suppressed (hopefully
>     after the exception being reported) by setting the environment varibale
>     "NO_EXT_CHECKS" to any value.
>
>     Add representations of "get the value of this extension from this option"
>     style extensions to the newext type, for each of the options that the
>     compiler uses this way. The one exception is java_object_file_extension,
>     which was used in this way, but which had no code handling it in
>     file_names.m.
>
>     Add a representation of ".$(EXT_FOR_PIC_OBJECTS)" as a value
>     to the newext type.
>
>     Shorten some function symbol names in the newext type and its components,
>     to make them easier to fit with excessive line lengths in the modules
>     listed below.

...

> compiler/make.program_target.m:
>     Pass extensions as ext/newext pairs, not just as exts.
>
>     Add an XXX about java_object_file_extension for Julien, since he added
>     this option (in 2001 :-().

As I did the initial work on the Java backend in 2001, that's not very
surprising ...

> diff --git a/compiler/file_names.m b/compiler/file_names.m
> index 0ccbcce16..fccc8cefe 100644
> --- a/compiler/file_names.m
> +++ b/compiler/file_names.m
> @@ -146,6 +146,233 @@
>
>  %---------------------%
> 
> +:- type newext
> +    --->    newext_src
> +            % The extension string is ".m".
> +
> +    ;       newext_exec(ext_exec)
> +    ;       newext_lib(ext_lib)
> +            % Executables and library files, which are always put into
> +            % the current directory.
> +
> +    ;       newext_exec_gs(ext_exec_gs)
> +    ;       newext_lib_gs(ext_lib_gs)
> +            % Executables and library files, which are
> +            %
> +            % - put into the current directory with --no-use-grade-subdirs,
> +            % - put into a grade subdir with --use-grade-subdirs.
> +            %
> +            % Note that the documention of --use-grade-subdirs says
> +            % "Executables and libraries will be symlinked or copied into the
> +            % current directory", but if this is actually done, it is done
> +            % outside file_names.m.

Why "if this is actually done"?  Do you have some doubt as to whether it's
being done.  (It's definitely done on my machine.)

> +
> +    ;       newext_mh(ext_mh)
> +            % Information about Mercury code exported to outside C code.
> +            % The extension string is ".mh".
> +
> +    ;       newext_mih(ext_mih)
> +            % Machine-independent header files for generated C code.
> +            % XXX Yet we consider them architecture-or-grade-dependent.
> +            % The extension string is ".mih".
> +
> +    ;       newext_user(ext_user)
> +            % Compiler-generated files that are intended to be read
> +            % by the programmer, such as .err files, which are always put
> +            % into the current directory.
> +
> +    ;       newext_user_ngs(ext_user_ngs)
> +            % Compiler-generated files that are intended to be read
> +            % by the programmer, such as .defns files, which will be put
> +            % into the current directory with --no-use-subdirs, but which
> +            % will be put into a non-grade-specific subdirectory with
> +            % --use-subdirs.
> +
> +    ;       newext_mmakefile_fragment(ext_mmakefile_fragment)
> +            % Compiler-generated files that are designed to be bodily included
> +            % in Mmakefiles.
> +
> +    ;       newext_mmake_target(ext_mmake_target)
> +            % These suffixes are used not to create filenames, but to
> +            % create mmake target names. Some do refer to real files,
> +            % but they can (and some do) refer to these using extension
> +            % strings that can contain references to make variables.
> +            % Some of the other generated make targets are phony targets,
> +            % meaning that they never correspond to real files at all.
> +
> +    ;       newext_int(ext_int)
> +    ;       newext_opt(ext_opt)
> +            % Compiler-generated interface files. and optimization files,

There's an extra full stop in there.

> +            % and the timestamp files showing when they were last checked.
> +
> +    ;       newext_target_c_cs(ext_target_c_cs)
> +    ;       newext_target_java(ext_target_java)
> +    ;       newext_target_date(ext_target_date)
> +
> +    ;       newext_target_init_c(ext_init_c)
> +
> +    ;       newext_target_init_obj(ext_init_obj)
> +    ;       newext_target_obj(ext_obj)
> +
> +    ;       newext_bytecode(ext_bytecode)
> +
> +    ;       newext_analysis(ext_analysis)
> +
> +    ;       newext_misc_ngs(ext_misc_ngs)
> +    ;       newext_misc_gs(ext_misc_gs)
> +            % XXX Document me.
> +
> +    ;       newext_other(other_newext).
> +            % The general case. The extension string must not be covered
> +            % by any of the other cases above.

...

> diff --git a/compiler/make.program_target.m b/compiler/make.program_target.m
> index b53e9cee5..423b407d7 100644
> --- a/compiler/make.program_target.m
> +++ b/compiler/make.program_target.m
> @@ -636,20 +636,34 @@ build_linked_target_2(Globals, MainModuleName, FileType, OutputFileName,
>
>          (
>              CompilationTarget = target_c,
> -            pic_object_file_extension(NoLinkObjsGlobals, PIC, ObjOtherExtToUse)
> +            pic_object_file_extension(NoLinkObjsGlobals, PIC,
> +                ObjOtherExtToUse, ObjNewExt, _),
> +            NewExt = newext_target_obj(ObjNewExt)
>          ;
>              CompilationTarget = target_csharp,
>              % There is no separate object code step.
> -            ObjOtherExtToUse = other_ext(".cs")
> +            ObjOtherExtToUse = other_ext(".cs"),
> +            NewExt = newext_target_c_cs(ext_target_cs)
>          ;
>              CompilationTarget = target_java,
>              globals.lookup_string_option(NoLinkObjsGlobals,
>                  java_object_file_extension, ObjExtToUseStr),
> -            ObjOtherExtToUse = other_ext(ObjExtToUseStr)
> +            ObjOtherExtToUse = other_ext(ObjExtToUseStr),
> +            % XXX EXT JULIEN This assumes that java_object_file_extension
> +            % has its default value, ".class".
> +            %
> +            % We *could* extend the ext_target_java type with a value
> +            % that means "the value of the java_object_file_extension option",
> +            % as we have done with e.g. ext_exec_gc. However, the original
> +            % code of file_names.m, which had code to do the right thing
> +            % for options such as executable_file_extension, has no such code
> +            % for java_object_file_extension. Therefore there is no GOOD code
> +            % to model the handling of any such new ext_target_java value on.
> +            NewExt = newext_target_java(ext_target_java_class)

IIRC, the only reason that the --java-object-file-extension option exists is
because we copied the corresponding C options when doing the initial setup for
the Java backend. Since the Java compiler doesn't provide a way of letting you
specify the output class name anyway, the existence of a Mercury compiler option
to change it seems a bit pointless. We should delete the option.

The diff is fine.

Julien.