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

[Jonathan Morgan]

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?

Jon

Index: compiler/mlds_to_il.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/mlds_to_il.m,v
retrieving revision 1.181
diff -u -u -r1.181 mlds_to_il.m
--- compiler/mlds_to_il.m	6 Jun 2007 01:17:34 -0000	1.181
+++ compiler/mlds_to_il.m	9 Jun 2007 12:07:20 -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,11 +1080,15 @@
             class_member_name(ParentClass, ctor), []))]
     ;
         IsCons = no,
-        predlabel_to_il_id(PredLabel, ProcId, MaybeSeqNum, MemberName0),
-        predlabel_to_managed_id(PredLabel, ProcId, MaybeSeqNum,
-                ManagedMemberName0),
-        MemberName = id(MemberName0),
-        ManagedMemberName = id(ManagedMemberName0),
+        ( Name = entity_function(PredLabel, ProcId, MaybeSeqNum, _PredId) ->
+            predlabel_to_il_id(PredLabel, ProcId, MaybeSeqNum, MemberName0),
+            predlabel_to_managed_id(PredLabel, ProcId, MaybeSeqNum,
+                    ManagedMemberName0),
+            MemberName = id(MemberName0),
+            ManagedMemberName = id(ManagedMemberName0)
+        ;
+            unexpected(this_file, "IL procedure is not a function")
+        ),
         CtorInstrs = []
     ),

@@ -1160,9 +1157,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, _ProcId, no, _),
+        MainPredLabel = mlds_user_pred_label(pf_predicate, no, "main", 2,
+            model_det, no)
     ->
         EntryPoint = [entrypoint],
         !:Info = !.Info ^ has_main := yes,
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------