[m-rev.] for review: delete the hl grade component

[Julien Fischer]

On Sat, 11 Apr 2020, Zoltan Somogyi wrote:

> For review by Julien.

...

> I also have a question. This diff deletes every occurrence of
> the C macro MR_HIGHLEVEL_DATA, since it is now unnecessary,
> but a bunch of occurrences of that string remain in C# code,
> almost all in #ifdefs in library/rtti_implementation.m. The
> #else parts of those #ifdefs seem to me to be relics of the
> .NET backend, when it tried to access low-level-data RTTI.
> Since compile_target_code.m *always* defined MR_HIGHLEVEL_DATA
> for C#, I think the non-MR_HIGHLEVEL_DATA sides of those
> #ifdefs are dead code, but I am not 100% sure. Does anyone
> have any reason to believe that these apparently dead pieces
> of code are in fact needed? And if not, is there any reason why
> we need to continue defining MR_HIGHLEVEL_DATA for C#?

They are dead code; they were used for the ilc grade -- they should
have been removed when it was removed.

...

> Remove the hl grade component.
> 
> As we discussed, it has fallen into disuse. Its main purpose was to
> pave the way for the .net backend and later for the java and csharp grades.
> Now that the .net backend is ancient history and the java and csharp grades
> are established, that purpose is gone, and for every other purpose,
> hlc is better because it is simpler and faster.
> 
> compiler/options.m:
>     Delete the --high-level-data option. It is no longer needed,
>     bacause the data representation scheme is now a direct function
>     of the target language.
> 
> doc/user_guide.texi:
>     Delete references to the --high-level-data option.
> 
> NEWS:
>     Mention that --high-level-data is no longer supported.
> 
> compiler/compute_grade.m:
>     Delete references to the hl grade component, and conform
>     to the deletion of the --high-level-data option.
> 
> compiler/handle_options.m:

Did you leave something out here?

...

> diff --git a/NEWS b/NEWS
> index 6c1e3e523..b0e4560ec 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -203,6 +203,10 @@ Changes to the Mercury compiler
>    that the `--trail-segments` option now has no effect, and it is therefore
>    deprecated.
> 
> +* `--high-level-data`
> +
> +  We have deleted the --high-level-data option.

Quote --high-level-data there.  I suggest a little bit more detail, e.g.

     We have deleted the `--high-level-data` option. Its effects are now
     implied by the target language.

...

> diff --git a/compiler/compile_target_code.m b/compiler/compile_target_code.m
> index 6f3f76632..681494f08 100644
> --- a/compiler/compile_target_code.m
> +++ b/compiler/compile_target_code.m

...

> @@ -2881,14 +2871,7 @@ create_csharp_exe_or_lib(Globals, ErrorStream, LinkTargetType, MainModuleName,
>      % NOTE: we use the -option style options in preference to the /option
>      % style in order to avoid problems with POSIX style shells.
>      globals.lookup_string_option(Globals, csharp_compiler, CSharpCompiler),
> -    globals.lookup_bool_option(Globals, highlevel_data, HighLevelData),
> -    (
> -        HighLevelData = yes,
> -        HighLevelDataOpt = "-define:MR_HIGHLEVEL_DATA"
> -    ;
> -        HighLevelData = no,
> -        HighLevelDataOpt = ""
> -    ),
> +    HighLevelDataOpt = "-define:MR_HIGHLEVEL_DATA",
>      globals.lookup_bool_option(Globals, target_debug, Debug),
>      (
>          Debug = yes,

As noted elsewhere in this thread, defining MR_HIGHLEVEL_DATA here should
not be necessary.  (The non-MR_HIGHLEVEL_DATA C# code paths should be deleted,
they were for the old ilc grade.)

...

> diff --git a/compiler/ml_unify_gen_construct.m b/compiler/ml_unify_gen_construct.m
> index a10701b95..ca59aceb4 100644
> --- a/compiler/ml_unify_gen_construct.m
> +++ b/compiler/ml_unify_gen_construct.m

...

> @@ -1562,8 +1559,15 @@ construct_ground_term_tagword_initializer_lld(RHSVarTypeWidth,
>  %---------------------------------------------------------------------------%
>
>  ml_generate_const_structs(ModuleInfo, Target, ConstStructMap, !GlobalData) :-
> -    module_info_get_globals(ModuleInfo, Globals),
> -    globals.lookup_bool_option(Globals, highlevel_data, HighLevelData),
> +    (
> +        Target = ml_target_c,
> +        HighLevelData = no
> +    ;
> +        ( Target = ml_target_java
> +        ; Target = ml_target_csharp
> +        ),
> +        HighLevelData = yes
> +    ),

This chunk of code is repeated in a few spots, perhaps it should be a separate
predicate in ml_util.m?

...

> diff --git a/scripts/parse_ml_options.sh-subr.in b/scripts/parse_ml_options.sh-subr.in
> index 4c8e8d3ca..bc13e1d16 100644
> --- a/scripts/parse_ml_options.sh-subr.in
> +++ b/scripts/parse_ml_options.sh-subr.in
> @@ -1,4 +1,6 @@
>  #---------------------------------------------------------------------------#
> +# vim: ts=4 sw=4 expandtab ft=sh
> +#---------------------------------------------------------------------------#
>  # Copyright (C) 2001-2007, 2011 The University of Melbourne.
>  # This file may only be copied under the terms of the GNU General
>  # Public License - see the file COPYING in the Mercury distribution.
> @@ -18,7 +20,7 @@
>
>  # initialize common options
>  mercury_config_dir=${MERCURY_STDLIB_DIR- at CONFIG_LIBDIR@}
> -mercury_config_dir=${MERCURY_CONFIG_DIR=$mercury_config_dir}
> +mercury_config_dir=${MERCURY_CONFIG_DIR=${mercury_config_dir}}
>  mercury_stdlib_dir=${MERCURY_STDLIB_DIR=@LIBDIR@}
>
>  # initialize ml options
> @@ -27,7 +29,7 @@ allow_undef=false
>  trace=false
>  ssdb=false
>  readline=true
> -case $FULLARCH in
> +case ${FULLARCH} in
>  	*-win95|*-winnt|*-win32|*-cygwin32|*-cygwin|*mingw*)

Not that it is relevant to this diff, but I'm pretty sure some
of those alternatives (*-win95)  can go now.

The diff looks fine otherwise.

Julien.