[m-rev.] diff: move library changes on the mode-constraints branch onto trunk

[Zoltan Somogyi]

On 26-Feb-2003, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> You need to modify var.h to make it namespace clean.

Done.

> > +* We've added functions get_equivalent_elements, get_minimum_element and 
> > +  remove_equivalent_elements, and their predicate versions, to eqvclass.m.
> 
> Why add predicate versions?

I guess David added them because everything else in eqvclass.m has both
predicate and function versions, but that is for backward compatibility.
I removed the predicate versions.

> > +:- pragma foreign_decl("C", "
> > +
> > +#define	NDEBUG
> 
> Defining NDEBUG here breaks namespace cleanliness.

I moved the #define to the foreign_code.

> > +#include ""../robdd/bryant.h""
> > +").
> > +
> > +:- pragma foreign_code("C", "
> > +#include ""../robdd/bryant.c""
> 
> The hard-coded "../robdd/" path names here are a problem.
> How will that work when the header files get installed into
> <prefix>/lib/mercury/inc?

The one in foreign_code isn't a problem, as it doesn't get included
in the .mh file in that directory. The one in the foreign_decl does,
but I am not *sure* why. The language reference manual doesn't say anything
about this, and since the only effects of a foreign_decl it discusses apply
only to the compilation of the code in the relevant module, implies that
this shouldn't happen. The scant documentation of .mh files doesn't say
anything on the matter either.

I think the reason why we include foreign_decls in .mh files is because
they may declare types needed by :- exported procedures. However, not
all foreign_decls do that, and for the ones that don't, including them
in the .mh file may be harmful, as here. (Regardless of path names,
anyone importing robdd.mh will need to make a robdd.h file visible to the
C compiler, which shouldn't be necessary; robdd.h is needed only when
generating robdd.o.)

Maybe we should consider adding an extra field to foreign_decls
to control this behavior.

> > +:- pragma foreign_proc("C",
> > +	one = (F::out),
> > +	[will_not_call_mercury, promise_pure],
> > +"
> > +	F = (MR_Word) MR_ROBDD_trueVar();
> > +").
> 
> The cast to MR_Word here (and in lots of other places in this file)
> is wrong for --high-level-data.  It will need treatment similar to what
> runtime/mercury_hlc_types.h does for the builtin types.
> Or alternatively (probably better), use `pragma foreign_type'.

OK, I am now using a type which is either MR_Box or MR_Word depending
on the backend.

> > -#  RCS      : $Id: Makefile,v 1.1 2000/03/10 05:17:20 dmo Exp $
> > +#  RCS      : $Id: Makefile,v 1.1.12.1 2001/02/14 04:42:48 dmo Exp $
> 
> Please *don't* include RCS Ids.

Done.

> >  CC=gcc -I../mylib -I/usr/ucbinclude -I. -ansi
> >  LIBDIRS=-L/home/staff/pets/quintus/bin3.2/sun4-5
> 
> That's not going to be right for anyone other than Peter Schachte,
> and proably not even then ;-)

I agree, but this Makefile isn't used by Mercury (it is used only for tests),
so it doesn't matter.

When is the rest of the review coming?

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