[m-rev.] for post-commit review: fix a problem with library installation with mmc --make

Julien Fischer jfischer at opturion.com
Tue Aug 26 00:23:46 AEST 2014


The change below is for both the master and 14.01 branches.  I intend to
further change the master branch so that each base grade component only
has a single target language associated with it, since the root cause of
all this mess was the GCC backend piggybacking on the high-level C
backend.

-------------------

Fix a problem with library installation with mmc --make.

Library installation with mmc --make sometimes attempt to install libraries in
non-existent grades, for example java.par or java.trseg.  The cause of this
problem is some left over code from the GCC backend, in particular the
high-level C base grade components imply multiple possible target languages, C
and asm.  This is a problem because of the XXX in the predicate
convert_grade_option/3, where we only update the current target language if the
base grade component has a single target language associated with it.

For example, when a library is being installed in the grades java and hlc.par.gc and
we are at the step where we install it in the hlc.par.gc grade (second, since the
libgrade list is sorted), we build up the following command line for the mmc subprocess
that will do the installation in that grade:

    mmc --invoked-by-mmc-make ... --make        \
 	--grade java --use-grade-subdirs       \
 	--grade hlc.par.gc --use-grade-subdirs \
 	--java-only --use-subdirs <target>

(XXX I don't know why we need to keep repeating --use-grade-subdirs like this,
or why we have to pass --use-subdirs at all given that it is implied by
--use-grade-subdirs.  Or what the rationale for passing all the --grade options
to the subprocess is.)

When the option "--grade java" is processed the target language will be set to
Java, but because of the XXX in convert_grade_option/3, the target language
will not be set to C when the option "--grade hlc.par.gc" is subsequently
processed, hence the compiler will attempt to install the library in the
non-existent grade java.par.  (Since --target java implies automatic GC,
the .gc component will vanish.)

The fix is to remove the asm target language for the high-level C base grade
components.

Additionally, remove other left over code for supporting --target asm.

compiler/handle_options.m:
 	Delete the "asm" target language for the high-level C base grade
 	components.

compiler/make.m:
compiler/globals.m:
compiler/add_pragma.m:
compiler/write_deps_file.m:
 	Delete references to "--target asm" and update some other comments
 	where necessary.

compiler/modules.m:
 	Delete two unused predicate that were only required by the GCC backend.

compiler/compile_target_code.m:
 	Delete the unused predicate assemble/7 that was only required by the
 	GCC backend.

compiler/mlds_to_c.m:
 	Add an XXX comment about some code that was required to support
 	"--target asm".

NEWS:
 	Announce the fix.

Julien.

diff --git a/NEWS b/NEWS
index 77c1449..a791155 100644
--- a/NEWS
+++ b/NEWS
@@ -29,6 +29,8 @@ This is a bug-fix release.
    implements the documented behaviour for input lists containing null
    characters (i.e. it throws an exception).
    Likewise, for string.from_reverse_char_list in the C# grade.
+* We have fixed a problem that caused `mmc --make' to attempt to install
+  libraries in non-existent grades.

  Changes to the Mercury compiler:

diff --git a/compiler/add_pragma.m b/compiler/add_pragma.m
index e5219d6..5c6d45b 100644
--- a/compiler/add_pragma.m
+++ b/compiler/add_pragma.m
@@ -3864,9 +3864,8 @@ check_required_feature(Globals, Context, Feature, !Specs) :-
          globals.get_gc_method(Globals, GC_Method),
          (
              % We consider gc_automatic to be conservative even it may not be.
-            % This is okay because this feature is only of interest with the
-            % C or asm backends. We ignore it if the target language is
-            % something else.
+            % This is okay because this feature is only of interest with the C
+            % backends. We ignore it if the target language is something else.

              ( GC_Method = gc_automatic
              ; GC_Method = gc_boehm
diff --git a/compiler/compile_target_code.m b/compiler/compile_target_code.m
index 40f4397..105af59 100644
--- a/compiler/compile_target_code.m
+++ b/compiler/compile_target_code.m
@@ -52,11 +52,6 @@
  :- pred do_compile_c_file(io.output_stream::in, pic::in,
      string::in, string::in, globals::in, bool::out, io::di, io::uo) is det.

-    % assemble(ErrorStream, PIC, ModuleName, Globals, Succeeded, !IO)
-    %
-:- pred assemble(io.output_stream::in, pic::in, module_name::in,
-    globals::in, bool::out, io::di, io::uo) is det.
-
      % compile_java_files(ErrorStream, JavaFiles, Succeeded, Globals, !IO)
      %
  :- pred compile_java_files(io.output_stream::in, list(string)::in,
@@ -1156,50 +1151,6 @@ java_classpath_separator = PathSeparator :-

  %-----------------------------------------------------------------------------%

-assemble(ErrorStream, PIC, ModuleName, Globals, Succeeded, !IO) :-
-    (
-        PIC = pic,
-        AsmExt = ".pic_s",
-        GCCFLAGS_FOR_ASM = "-x assembler ",
-        GCCFLAGS_FOR_PIC = "-fpic "
-    ;
-        PIC = link_with_pic,
-        % `--target asm' doesn't support any grades for
-        % which `.lpic_o' files are needed.
-        unexpected($module, $pred, "link_with_pic")
-    ;
-        PIC = non_pic,
-        AsmExt = ".s",
-        GCCFLAGS_FOR_ASM = "",
-        GCCFLAGS_FOR_PIC = ""
-    ),
-    module_name_to_file_name(Globals, ModuleName, AsmExt,
-        do_not_create_dirs, AsmFile, !IO),
-    maybe_pic_object_file_extension(Globals, PIC, ObjExt),
-    module_name_to_file_name(Globals, ModuleName, ObjExt,
-        do_create_dirs, ObjFile, !IO),
-
-    globals.lookup_bool_option(Globals, verbose, Verbose),
-    maybe_write_string(Verbose, "% Assembling `", !IO),
-    maybe_write_string(Verbose, AsmFile, !IO),
-    maybe_write_string(Verbose, "':\n", !IO),
-    % XXX should we use new asm_* options rather than
-    % reusing cc, cflags, c_flag_to_name_object_file?
-    globals.lookup_string_option(Globals, cc, CC),
-    globals.lookup_string_option(Globals, c_flag_to_name_object_file,
-        NameObjectFile),
-    globals.lookup_accumulating_option(Globals, cflags, C_Flags_List),
-    join_string_list(C_Flags_List, "", "", " ", CFLAGS),
-    % Be careful with the order here.
-    % Also be careful that each option is separated by spaces.
-    string.append_list([CC, " ", CFLAGS, " ", GCCFLAGS_FOR_PIC,
-        GCCFLAGS_FOR_ASM, "-c ", AsmFile, " ", NameObjectFile, ObjFile],
-        Command),
-    invoke_system_command(Globals, ErrorStream, cmd_verbose_commands, Command,
-        Succeeded, !IO).
-
-%-----------------------------------------------------------------------------%
-
  compile_erlang_file(ErrorStream, ErlangFile, Globals, Succeeded, !IO) :-
      globals.lookup_bool_option(Globals, verbose, Verbose),
      maybe_write_string(Verbose, "% Compiling `", !IO),
diff --git a/compiler/globals.m b/compiler/globals.m
index 7fbe37a..f25f03d 100644
--- a/compiler/globals.m
+++ b/compiler/globals.m
@@ -71,10 +71,10 @@
  :- func simple_foreign_language_string(foreign_language) = string.

      % The GC method specifies how we do garbage collection.
-    % The last four alternatives are for the C and asm back-ends;
-    % the first alternative is for compiling to IL or Java,
-    % where the target language implementation handles garbage
-    % collection automatically.
+    % The last five alternatives are for the C back-ends;
+    % the first alternative is for compiling to C#, Java, Il or Erlang
+    % where the target language implementation handles garbage collection
+    % automatically.
      %
  :- type gc_method
      --->    gc_automatic
diff --git a/compiler/handle_options.m b/compiler/handle_options.m
index 3d27a98..414e39d 100644
--- a/compiler/handle_options.m
+++ b/compiler/handle_options.m
@@ -2965,7 +2965,7 @@ grade_component_table("hl", comp_gcc_ext, [
          highlevel_code          - bool(yes),
          gcc_nested_functions    - bool(no),
          highlevel_data          - bool(yes)],
-        yes([string("c"), string("asm")]), yes).
+        yes([string("c")]), yes).
  grade_component_table("hlc", comp_gcc_ext, [
          asm_labels              - bool(no),
          gcc_non_local_gotos     - bool(no),
@@ -2973,7 +2973,7 @@ grade_component_table("hlc", comp_gcc_ext, [
          highlevel_code          - bool(yes),
          gcc_nested_functions    - bool(no),
          highlevel_data          - bool(no)],
-        yes([string("c"), string("asm")]), yes).
+        yes([string("c")]), yes).
  grade_component_table("hl_nest", comp_gcc_ext, [
          asm_labels              - bool(no),
          gcc_non_local_gotos     - bool(no),
@@ -2981,7 +2981,7 @@ grade_component_table("hl_nest", comp_gcc_ext, [
          highlevel_code          - bool(yes),
          gcc_nested_functions    - bool(yes),
          highlevel_data          - bool(yes)],
-        yes([string("c"), string("asm")]), yes).
+        yes([string("c")]), yes).
  grade_component_table("hlc_nest", comp_gcc_ext, [
          asm_labels              - bool(no),
          gcc_non_local_gotos     - bool(no),
@@ -2989,7 +2989,7 @@ grade_component_table("hlc_nest", comp_gcc_ext, [
          highlevel_code          - bool(yes),
          gcc_nested_functions    - bool(yes),
          highlevel_data          - bool(no)],
-        yes([string("c"), string("asm")]), yes).
+        yes([string("c")]), yes).
  grade_component_table("il", comp_gcc_ext, [
          asm_labels              - bool(no),
          gcc_non_local_gotos     - bool(no),
diff --git a/compiler/make.m b/compiler/make.m
index 12ceadb..6ba915c 100644
--- a/compiler/make.m
+++ b/compiler/make.m
@@ -199,8 +199,7 @@
  :- type compilation_task_type
      --->    process_module(module_compilation_task_type)

-            % The `pic' argument is only used for
-            % `--target c' and `--target asm'.
+            % The `pic' argument is only used for `--target c'.
      ;       target_code_to_object_code(pic)
      ;       foreign_code_to_object_code(pic, foreign_language)
      ;       fact_table_code_to_object_code(pic, file_name).
diff --git a/compiler/mlds_to_c.m b/compiler/mlds_to_c.m
index 8fa8d8a..a30a15d 100644
--- a/compiler/mlds_to_c.m
+++ b/compiler/mlds_to_c.m
@@ -2752,6 +2752,7 @@ mlds_output_type_prefix(Opts, MLDS_Type, !IO) :-
          % For binary compatibility with the --target asm back-end,
          % we need to output these as a generic type, rather than making
          % use of the C type name
+        % XXX target asm no longer exists, so no longer need to do this.
          io.write_string("MR_Box", !IO)
      ;
          MLDS_Type = mlds_class_type(Name, Arity, ClassKind),
diff --git a/compiler/modules.m b/compiler/modules.m
index 4ff0cb1..f9106e3 100644
--- a/compiler/modules.m
+++ b/compiler/modules.m
@@ -2894,27 +2894,6 @@ append_to_init_list(DepStream, InitFileName, Module, !IO) :-

  %-----------------------------------------------------------------------------%

-    % Find out which modules we need to generate C header files for,
-    % assuming we're compiling with `--target asm'.
-    %
-:- func modules_that_need_headers(list(module_name), deps_map)
-    = list(module_name).
-
-modules_that_need_headers(Modules, DepsMap) =
-    list.filter(module_needs_header(DepsMap), Modules).
-
-    % Succeed iff we need to generate a C header file for the specified
-    % module, assuming we're compiling with `--target asm'.
-    %
-:- pred module_needs_header(deps_map::in, module_name::in) is semidet.
-
-module_needs_header(DepsMap, Module) :-
-    map.lookup(DepsMap, Module, deps(_, ModuleImports)),
-    ModuleImports ^ mai_has_foreign_code = contains_foreign_code(Langs),
-    set.member(lang_c, Langs).
-
-%-----------------------------------------------------------------------------%
-
  process_module_private_interfaces(_, _, [], _, _, !DirectImports,
          !DirectUses, !Module, !IO).
  process_module_private_interfaces(Globals, HaveReadModuleMap,
diff --git a/compiler/write_deps_file.m b/compiler/write_deps_file.m
index bc7e0f9..68ab044 100644
--- a/compiler/write_deps_file.m
+++ b/compiler/write_deps_file.m
@@ -570,10 +570,10 @@ write_dependency_file(Globals, Module, AllDepsSet, MaybeTransOptDeps, !IO) :-
                  ForeignImportExt = ".hrl"
              ;
                  Target = target_c,
-                % NOTE: for C (and asm) the possible targets might be a .o
-                % file _or_ a .pic_o file.  We need to include dependencies
-                % for the latter otherwise invoking mmake with a <module>.pic_o
-                % target will break.
+                % NOTE: for C the possible targets might be a .o file _or_ a
+                % .pic_o file.  We need to include dependencies for the latter
+                % otherwise invoking mmake with a <module>.pic_o target will
+                % break.
                  ForeignImportTargets = [ObjFileName, PicObjFileName],
                  ForeignImportExt = ".mh"
              ;



More information about the reviews mailing list