[m-rev.] For review: Improve the name mangling of foreign methods in the IL backend

[Jonathan Morgan]

On 6/9/07, Jonathan Morgan <jonmmorgan at gmail.com> wrote:
> On 6/8/07, Jonathan Morgan <jonmmorgan at gmail.com> wrote:
> > On 6/7/07, Julien Fischer <juliensf at csse.unimelb.edu.au> wrote:
> > > The problem here is that the ml code generator introduces
> > > constructors for the classes it generates for du types.  (The relevant
> > > piece of code is ml_type_gen.m:795.)
> > >
> > > The mlds_defn's for these constructor methods have their entity_name
> > > field set to `entity_export("<constructor>")' for some reason.  (This
> > > doesn't quite line with the documentation for entity_export/1 in
> > > mlds.m which says they are for pragma exported entities.)
> > >
> > > It looks (from the comments in ml_type_gen.m) that this particular name
> > > is just used as a place-holder, as the actual constructor names are
> > > backend specific.  I not sure why we do things in this particular way?
> > > Pete, do you have any ideas?
> > >
> > > I think we can work-around the problem at the moment by only performing
> > > the sanity check when we do not have a constructor method.  The
> > > diff below makes this change.  With it the valid/foreign_type_spec
> > > passes again - I haven't checked whether it is okay with respect to
> > > the rest of Jon's change.
> > >
> > > Index: mlds_to_il.m
> > > ===================================================================
> > > RCS file: /home/mercury/mercury1/repository/mercury/compiler/mlds_to_il.m,v
> > > retrieving revision 1.181
> > > diff -u -r1.181 mlds_to_il.m
> > > --- mlds_to_il.m        6 Jun 2007 01:17:34 -0000       1.181
> > > +++ mlds_to_il.m        7 Jun 2007 08:46:24 -0000
> > > @@ -1051,13 +1051,6 @@
> > >           ClassMember, !Info) :-
> > >       Entity = mlds_function(_MaybePredProcId, Params, MaybeStatement,
> > >           Attributes, EnvVarNames),
> > > -    ( Name = entity_function(PredLabel0, ProcId0, MaybeSeqNum0, _PredId) ->
> > > -        PredLabel = PredLabel0,
> > > -        ProcId = ProcId0,
> > > -        MaybeSeqNum = MaybeSeqNum0
> > > -    ;
> > > -        unexpected(this_file, "IL procedure is not a function")
> > > -    ),
> > >
> > >       expect(set.empty(EnvVarNames), this_file,
> > >           "generate_method: EnvVarNames"),
> > > @@ -1087,6 +1080,13 @@
> > >               class_member_name(ParentClass, ctor), []))]
> > >       ;
> > >           IsCons = no,
> > > +        ( Name = entity_function(PredLabel0, ProcId0, MaybeSeqNum0, _) ->
> > > +            PredLabel = PredLabel0,
> > > +            ProcId = ProcId0,
> > > +            MaybeSeqNum = MaybeSeqNum0
> > > +        ;
> > > +            unexpected(this_file, "IL procedure is not a function")
> > > +        ),
> > >           predlabel_to_il_id(PredLabel, ProcId, MaybeSeqNum, MemberName0),
> > >           predlabel_to_managed_id(PredLabel, ProcId, MaybeSeqNum,
> > >                   ManagedMemberName0),
> > > @@ -1160,9 +1160,9 @@
> > >       % in an exception handler and call the initialization instructions
> > >       % in the cctor of this module.
> > >       (
> > > -        PredLabel = mlds_user_pred_label(pf_predicate, no, "main", 2,
> > > -            model_det, no),
> > > -        MaybeSeqNum = no
> > > +        Name = entity_function(MainPredLabel, _, no, _PredId),
> > > +        MainPredLabel = mlds_user_pred_label(pf_predicate, no, "main", 2,
> > > +            model_det, no)
> > >       ->
> > >           EntryPoint = [entrypoint],
> > >           !:Info = !.Info ^ has_main := yes,
> >
> > The workaround looks fine with respect to my changes.  You could move
> > the construction of MemberName and ManagedMemberName inside the
> > if-then-else and remove all the PredLabel0's and similar if you felt
> > it made it clearer.
>
> I have tested the following patch (based on yours) and it passes
> foreign_type_spec and appears to behave correctly.  Should this be
> committed, and if so, will you do it?

I was a little optimistic.  mlds_to_il appears to use entity_export
internally when processing foreign_export pragmas.

However, adding the following lines to the if-then-else allows the
standard library to be built without triggering the assertion, and I
think that they would be correct (as this is what was done by
entity_name_to_ilds_id before my change).

+        ; Name = entity_export(ExportName) ->
+            MemberName = id(ExportName),
+            ManagedMemberName = id(ExportName)

Jon
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------