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

Fergus Henderson fjh at cs.mu.OZ.AU
Wed Feb 26 21:27:17 AEDT 2003


On 26-Feb-2003, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> robdd/bryantPrint.[ch]:
> robdd/table.[ch]:
> robdd/test_abexit.c:
> robdd/test_abunify.c:
> robdd/test_abglb.c:
> robdd/test_iff.c:
> robdd/test_rename.c:
> robdd/test_restrict.c:
> robdd/test_rglb.c:
> robdd/test_upclose.c:
> robdd/test_var.c:
> robdd/test_vars.c:
> robdd/timing.[ch]:
> robdd/var.h:
> 	Conform to the changes in bryant.h. Note that since the code in these
> 	files won't end up in Mercury program code, they don't need to be
> 	namespace clean.

This comment is not correct for var.h, which gets included by bryant.h,
which gets included by robdd.m, which gets included in the Mercury
library since it is referenced from both pprint.m and library.m.

You need to modify var.h to make it namespace clean.

> +++ NEWS	26 Feb 2003 08:05:55 -0000
> @@ -108,6 +108,18 @@
>    These have been replaced by cc_multi versions in which success or failure
>    is indicated by returning a maybe type.
>  
> +* 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?

> +:- module robdd.
> +
> +:- interface.
...
> +:- implementation.
...
> +:- pragma foreign_decl("C", "
> +
> +#define	NDEBUG

Defining NDEBUG here breaks namespace cleanliness.

> +#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?

It would be better to just include "bryant.h" and "bryant.c",
and add -I($ROBDD_DIR) to CFLAGS in Mmake.workspace.

> +:- 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'.

In my previous review, I said it was OK if there were warnings for
--high-level-data.  The reason for this was because there are already
lots of warnings for --high-level-data.  But we have a solution for
those other warnings now, namely compiling with intermodule-optimization,
and the new warnings that would be produced for these casts to MR_Word
are actually errors with other C compilers, so we should avoid them.

> +++ robdd/Makefile	26 Feb 2003 08:23:33 -0000
> @@ -1,5 +1,10 @@
> +#-----------------------------------------------------------------------------#
> +# Copyright (C) 2001-2003 University of Melbourne. 
> +# This file may only be copied under the terms of the GNU General
> +# Public Licence - see the file COPYING in the Mercury distribution.
> +#-----------------------------------------------------------------------------#
>  #  File     : Makefile
> -#  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.  The id is alread listed in the
CVS/Entries file, and including it here causes spurious differences
when diffing different workspaces, trouble with interdiff, etc.

> -OPTIMIZE=-O3 -DNDEBUG -funroll-loops
> +#OPTIMIZE=-O3 -DNDEBUG -funroll-loops
> +OPTIMIZE=-DNDEBUG

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

[to be continued.]

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
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