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

Peter Moulder Peter.Moulder at infotech.monash.edu.au
Wed Oct 29 15:01:46 AEDT 2003


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.


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.

  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.]

The different versions could perhaps be generated from a `make' target,
if synchronization is an issue.

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).

pjm.
--------------------------------------------------------------------------
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