[m-rev.] for review: add foreign code support to mmake for the IL backend

Peter Ross peter.ross at miscrit.be
Mon Jul 23 23:14:29 AEST 2001


Note that a lot of the changes to this module have been made redundant by
the thread about multiple foreign language support.

----- Original Message -----
From: "Fergus Henderson" <fjh at cs.mu.OZ.AU>
To: <mercury-reviews at cs.mu.OZ.AU>
Sent: Monday, July 23, 2001 9:59 AM
Subject: Re: [m-rev.] for review: add foreign code support to mmake for the
IL backend


> On 17-Jul-2001, Peter Ross <peter.ross at miscrit.be> wrote:
> > Index: compiler/modules.m
> ...
> > @@ -2862,20 +2899,56 @@
> >  io__write_string(DepStream, MakeVarName),
> >  io__write_string(DepStream, ".mods ="),
> >  write_dependencies_list(Modules, "", DepStream),
> > + io__write_string(DepStream, "\n"),
> > +
> > + globals__io_get_target(Target),
> > + globals__io_lookup_foreign_language_option(
> > + backend_foreign_language, ForeignLang),
> > + { ForeignExt = "." ++ simple_foreign_language_string(ForeignLang) },
> > + ( { Target = il } ->
> > + { ForeignModules = foreign_modules(ForeignLang,
> > + Modules, DepsMap) }
> > + ;
> > + { ForeignModules = [] }
> > + ),
>
> Why is that only done when Target = il?
>
> IMHO this at least deserves a comment.
>
This was a performance optimization.  Only the IL backend could produce
these foreign language files, and at one stage of coding this module used to
traverse the item list of a module, so it was quite expensive.  It will
remove this optimization because it is no longer a necessity.

> >  io__write_string(DepStream, MakeVarName),
> > + io__write_string(DepStream, ".foreign_cs = "),
> > + write_compact_dependencies_list(ForeignModules, "$(os_subdir)",
> > + ForeignExt, ForeignBasis, DepStream),
> > + io__write_string(DepStream, "\n"),
>
> Shouldn't that be `cs_subdir' rather than `os_subdir'?
>
> Please test this change with `--use-subdirs'.
>
Yes it should have been.

> > + io__write_string(DepStream, MakeVarName),
> > + io__write_string(DepStream, ".foreign_os = "),
> > + write_compact_dependencies_list(ForeignModules, "$(os_subdir)",
".obj",
> > + ForeignBasis, DepStream),
> > + io__write_string(DepStream, "\n"),
>
> Shouldn't that be ".$(O)" rather than ".obj"?
>
This change has been removed, we now automatically remove the .obj file
generated by the C++ compiler as part of the build rule for building the
DLL.  Also .$(O) here is problematic because if we are using a compiler
built with gcc to compile this module then $(O) is set to o instead of obj.

> > @@ -3599,6 +3675,22 @@
> >  modules_that_need_headers(Modules, DepsMap) =
> >  list__filter(module_needs_header(DepsMap), Modules).
> >
> > +:- func foreign_modules(foreign_language, list(module_name), deps_map)
=
> > + list(module_name).
> > +
> > +foreign_modules(ForeignLang, Modules, DepsMap) = ForeignModules :-
> > + P = (pred(M::in, FM::out) is semidet :-
> > + Ext = "__" ++ simple_foreign_language_string(ForeignLang) ++
> > + "_code",
> > + module_needs_header(DepsMap, M),
> > + ( M = unqualified(Name),
> > + FM = unqualified(Name ++ Ext)
> > + ; M = qualified(Module, Name),
> > + FM = qualified(Module, Name ++ Ext)
> > + )
> > + ),
> > + list__filter_map(P, Modules, ForeignModules).
>
> It would help to have a comment or two here explaining what this procedure
does.
>
> The pattern `"__" ++ simple_foreign_language_string(...) ++ "_code"' seems
to
> occur in several places -- it might be worth making that a separate
function.
>


--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------



More information about the reviews mailing list