[m-rev.] diff: .NET backend runtime and library fixes

Fergus Henderson fjh at cs.mu.OZ.AU
Thu May 3 00:45:01 AEST 2001


On 02-May-2001, Tyson Dowd <trd at cs.mu.OZ.AU> wrote:
> library/.cvsignore:
> 	Add .cpp .dll and .il files.

We should not ignore .cpp files, since they may be hand-written.
Likewise for .il files, for that matter.
Instead, we should use a different suffix for automatically-generated
.cpp and .il files than we do for hand-written ones.
For example we could use .m.cpp and .m.il for the automatically
generated versions.

The same point applies to .c, for that matter, but unfortunately
that assumption is already hardwired in to too many places to easily
change now.  But, having made the mistake once, we shouldn't make the
same mistake again for the .NET back-end.

> Index: library/Mmakefile
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/library/Mmakefile,v
> retrieving revision 1.60
> diff -u -r1.60 Mmakefile
> --- library/Mmakefile	2001/04/20 06:43:20	1.60
> +++ library/Mmakefile	2001/05/02 13:28:17
> @@ -86,7 +86,7 @@
>  			$(INTERMODULE_OPTS) $(CHECK_TERM_OPTS)
>  MGNUC	=	$(M_ENV) $(SCRIPTS_DIR)/mgnuc
>  MGNUCFLAGS =	$(DLL_CFLAGS)
> -MS_CLFLAGS  =	-I$(RUNTIME_DIR) 
> +MS_CLFLAGS  =	-AI$(RUNTIME_DIR) -I$(RUNTIME_DIR)

What does the -AI option do?
A comment here would be helpful.

> +HACK_ALL_DLLS=$(CPP_DLLS) mercury_all.dll $(RUNTIME_DLLS)
> +
> +HACK_ALL_DLLS_BASE  = $(HACK_ALL_DLLS:%.dll=%)
> +HACK_EMBED_ALL_DLLS = $(foreach dll_name,$(HACK_ALL_DLLS_BASE),$(EMBED_ONE_DLL))

I think you should have a comment here explaining what this is for
and what the "HACK_" prefix means.

> +# If you do generate a new strong name, you had better update the compiler
> +# to generate references to it.  This is sub-optimal -- the compiler should
> +# automatically find out what the strong name is.
> +library_strong_name.sn:
> +	sn -k library_strong_name.sn

Which part(s) of the compiler need to be updated?

> Index: library/exception.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/library/exception.m,v
> retrieving revision 1.45
> diff -u -r1.45 exception.m
> --- library/exception.m	2001/03/15 07:42:21	1.45
> +++ library/exception.m	2001/05/02 13:28:23
> @@ -304,21 +304,15 @@
>  #ifndef ML_DETERMINISM_GUARD
>  #define ML_DETERMINISM_GUARD
>  
> -	/*
> -	** The enumeration constants in this enum must be in the same
> -	** order as the functors in the Mercury type `determinism'
> -	** defined above.
> -	*/
> -	typedef enum {
> -		ML_DET,
> -		ML_SEMIDET,
> -		ML_CC_MULTI,
> -		ML_CC_NONDET,
> -		ML_MULTI,
> -		ML_NONDET,
> -		ML_ERRONEOUS,
> -		ML_FAILURE
> -	} ML_Determinism;
> +#define ML_DET	0
> +#define	ML_SEMIDET	1
> +#define	ML_CC_MULTI	2
> +#define	ML_CC_NONDET	3
> +#define	ML_MULTI	4
> +#define	ML_NONDET	5
> +#define	ML_ERRONEOUS	6
> +#define	ML_FAILURE	7

The comment there should not be deleted.
Instead, it should be retained (with appropriate minor wording changes).
There should also be an XXX comment saying that we should use an enum
instead, and explaining why we don't.

> Index: library/math.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/library/math.m,v
> retrieving revision 1.27
> diff -u -r1.27 math.m
> --- library/math.m	2001/03/15 07:42:23	1.27
> +++ library/math.m	2001/05/02 13:28:28
> @@ -307,7 +307,7 @@
>  :- pragma foreign_proc("MC++", 
>  	math__ceiling(Num::in) = (Ceil::out),
>  		[will_not_call_mercury, thread_safe],"
> -	Ceil = System::Math::Ceil(Num);
> +	Ceil = System::Math::Ceiling(Num);
>  ").
>  
>  %
> @@ -360,7 +360,7 @@
>  	math__truncate(X::in) = (Trunc::out),
>  		[will_not_call_mercury, thread_safe],"
>  	if (X < 0.0) {
> -		Trunc = System::Math::Ceil(X);
> +		Trunc = System::Math::Ceiling(X);
>  	} else {
>  		Trunc = System::Math::Floor(X);
>  	}

I thikn it would be better to implement math__truncate in Mercury.

> Index: runtime/mercury_il.il
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/runtime/mercury_il.il,v
> retrieving revision 1.3
> diff -u -r1.3 mercury_il.il
> --- runtime/mercury_il.il	2001/01/22 04:20:40	1.3
> +++ runtime/mercury_il.il	2001/05/02 13:28:40
> @@ -11,11 +11,16 @@
>  
>  // Declare the assemblies we use
>  
> -.assembly extern mercury { }
> -
> -.assembly extern 'mercury_mcpp' { }
> +.assembly extern 'mercury'{
> +	.ver 0:0:0:0
> +	.publickeytoken = ( 22 8C 16 7D 12 AA B B ) 
> +}

You should document where those magic numbers came from.

> -.assembly extern 'mercury.io' { }
> +.assembly extern 'mscorlib'{
> +	.ver 1:0:2411:0
> +	.publickeytoken = ( B7 7A 5C 56 19 34 E0 89 ) 
> +	.hash = ( B0 73 F2 4C 14 39 A 35 25 EA 45 F 60 58 C3 84 E0 3B E0 95 ) 
> +}

Likewise.

> Index: runtime/mercury_mcpp.cpp
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/runtime/mercury_mcpp.cpp,v
> retrieving revision 1.4
> diff -u -r1.4 mercury_mcpp.cpp
> --- runtime/mercury_mcpp.cpp	2001/01/22 04:20:41	1.4
> +++ runtime/mercury_mcpp.cpp	2001/05/02 13:28:40
> @@ -30,7 +30,7 @@
>  {
>  public:
>  	// XXX there should be a Mercury object here.
> -    Exception(MR_String Msg) : Exception(Msg)
> +    Exception(MR_String Msg)

Shouldn't that be

    Exception(MR_String Msg) : System::Exception(Msg)

?

Otherwise this change looks fine.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
                                    |  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