[m-rev.] for review: rewrite mercury_mcpp.cpp in C#

Peter Ross pro at missioncriticalit.com
Thu Oct 30 01:58:12 AEDT 2003


On Wed, Oct 29, 2003 at 03:01:46PM +1100, Peter Moulder wrote:
> On Sat, Oct 25, 2003 at 04:39:09PM +0200, Peter Ross wrote:
> > [...]
> > +	// Make a MR_Word with the given tag and arity.
> > +    public static object[] make_MR_Word(int tag, int arity)
> > +    {
> > +        object[] o = new object[arity + 1];
> > +        o[0] = tag;
> > +        return o;
> > +    }
> 
> I don't know C#, but it looks strange to me that o[0] has type `object'
> while tag has type `int', with no explicit conversion
> (cast/construction/function application).  (The MC++ version was
> `MR_Word e; MR_newobj(e, tag, arity); return e;', btw.)
> In list_is_cons, there is an explicit System.Convert.ToInt32 call
> when converting in the opposite direction.
> 
Every type in .NET is a descendent from object, so no cast is necessary
here (implicitly the .NET runtime will box the integer, as int is a
value type).  However one explicitly needs to downcast the object back
to an integer.

> 
> The following minor issues were present in the MC++ version.
> 
>                 // the parent constructor sets the error message that
> 		// will be printed.
> 
> Sentences begin with capital letters.  I see no advantage of starting
> with a lowercase letter, and there is a minor disadvantage that the
> maintainer must take a mental detour to consider whether or not the
> beginning of the sentence has been accidentally removed, which distracts
> one's thought.
> 
I will add the captial letters.


>   public class Constants
>   {
>     // These constants are duplicated in library/private_builtin.m.
>     // They must be kept sychronized.
> 
>         // XXX it would be nice if these could be const or an enum.  But
>         // there are some problems with accessing the values from IL if
>         // we do that because neither alternatives seem to define field
>         // names we can reference from IL.
> 
> Costs of not telling the compiler that these are constants:
> 
>   - Allows the possibility of accidental modification.
> 
>   - Runtime efficiency (time & space).  Uses involve a probably-uncached
>     read from main memory.  Compiler can't do constant optimizations.
> 
> These constants already appear in two places library/private_builtin.m
> (one in Java; though see below).  We could have three instead of two
> versions: one accessible from IL, and another known by the compiler to
> be constant.  [I'm ignorant of the needs of IL for using these
> constants.]
> 
These constants are needed to interpret the RTTI structures which is all
done in low-level code, hence the need for them to be defined for Java,
IL and C.

> The different versions could perhaps be generated from a `make' target,
> if synchronization is an issue.
> 
I believe, Zoltan can comment on this, this problem will go away when we
shit to using true Mercury data types for representing the RTTI.  I am
not sure what the status of this is.

> Re "duplicated", there are in fact differences between the different
> occurrences.  MR_SECTAG_VARIABLE=3 occurs only in the private_builtin.m
> MC++ version.  The Java version in private_builtin.m has this
> difference:
> 
>   -MR_TYPECTOR_REP_SUBGOAL = 13;
>   -MR_TYPECTOR_REP_VOID	 = 14;
>   +MR_TYPECTOR_REP_UNI	 = 13;
>   +MR_TYPECTOR_REP_SUBGOAL = 14;
> 
> (and doesn't have any MR_SECTAG_* constants).
--------------------------------------------------------------------------
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