[m-dev.] for review: add MC++ implementation of library and runtime
Fergus Henderson
fjh at cs.mu.OZ.AU
Sun Dec 24 21:45:48 AEDT 2000
On 24-Dec-2000, Tyson Dowd <trd at cs.mu.OZ.AU> wrote:
> +++ library/int.m
> @@ -472,6 +472,10 @@
> #define ML_BITS_PER_INT (sizeof(MR_Integer) * CHAR_BIT)
> ").
>
> +:- pragma foreign_decl("MC++", "
> + #define ML_BITS_PER_INT 32
> +").
I think an XXX here is warranted.
Why do you have to hard-code this?
Is it just because CHAR_BIT isn't defined,
because you haven't included the appropriate system header?
> :- pragma foreign_decl("MC++", "
>
> + // XXX we should re-use the same stream writer on a stream, perhaps
> + // we should store it with a stream.
I suggest adding "for efficiency" at the start of this comment.
> +++ library/library.m
> @@ -67,8 +67,9 @@
> :- pragma foreign_code("MC++",
> library__version(Version::out), will_not_call_mercury,
> "
> - Version = String::Concat(MR_VERSION,
> - "", configured for "", MR_FULLARCH);
> + // XXX we should use string literals with an S at the start
> + // so this code uses just managed types.
> + Version = MR_VERSION "", configured for "", MR_FULLARCH;
> ").
That looks wrong -- the comma before "MR_FULLARCH" shouldn't be there.
(This probably because a comma expression and hence just sets Version
to MR_FULLARCH.)
> +static int MR_TYPECTOR_REP_ENUM = MR_TYPECTOR_REP_ENUM_val;
> +static int MR_TYPECTOR_REP_ENUM_USEREQ = MR_TYPECTOR_REP_ENUM_USEREQ_val;
> +static int MR_TYPECTOR_REP_DU = MR_TYPECTOR_REP_DU_val;
> +static int MR_TYPECTOR_REP_DU_USEREQ = 3;
There should be some comments before here explaining
why you use `static int' rather than `enum' or `static const int'.
> +++ mercury_il.il
> @@ -1,7 +1,15 @@
> +//
> +// Copyright (C) 2000 The University of Melbourne.
> +// This file may only be copied under the terms of the GNU Library General
> +// Public License - see the file COPYING.LIB in the Mercury distribution.
> +//
> +
> +// mercury_mcpp.cpp - This file defines the system runtime types and
> +// methods that ** are used when generating code for the .NET backend.
> +// It is written in Microsoft's IL assembly language.
The comment here refers to mercury_mcpp.cpp
but the file is mercury_il.il.
> diff -u mercury_mcpp.cpp mercury_mcpp.cpp
> --- mercury_mcpp.cpp
> +++ mercury_mcpp.cpp
> @@ -1,43 +1,53 @@
> +//
> +// Copyright (C) 2000 The University of Melbourne.
> +// This file may only be copied under the terms of the GNU Library General
> +// Public License - see the file COPYING.LIB in the Mercury distribution.
> +//
> +
> +// mercury_mcpp.cpp - This file defines the system runtime types and
> +// methods that are used when generating code for the .NET backend.
> +// It is written using Managed Extensions for C++ (usually called Managed C++
> +// or MC++).
I suggest s/Managed Extensions/Microsoft's Managed Extensions/
> +++ scripts/Mmake.vars.in
> +# MS_CL is the command line version of Microsoft VC++, which we use to compile
Might as well spell it out: s/VC++/Visual C++/
> +++ mercury_mcpp.h Sun Dec 24 16:04:05 2000
> @@ -0,0 +1,216 @@
> +
> +#include <stddef.h>
There should be a copyright statement
and some header comments explaining what this file is for.
> +namespace mercury {
> +
> +typedef int MR_Integer;
> +typedef System::Int32 MR_BoxedInt;
> +
> +typedef System::Char MR_Char; // `Char' is MS's name for unicode characters
> +
> +typedef double MR_Float;
> +#define MR_BoxedFloat System::Double
It's worth a comment explaining why you use #define
rather than typedef.
> +#define MR_COMPARE_EQUAL 0
> +#define MR_COMPARE_LESS 1
> +#define MR_COMPARE_GREATER 2
> +
> +#define MR_STRINGIFY(x) MR_STRINGIFY_2(x)
> +#define MR_STRINGIFY_2(x) #x
> +
> +#define MR_PASTE2(a,b) MR_PASTE2_2(a,b)
> +#define MR_PASTE2_2(a,b) a##b
> +#define MR_PASTE3(a,b,c) MR_PASTE3_2(a,b,c)
> +#define MR_PASTE3_2(a,b,c) a##b##c
> +#define MR_PASTE4(a,b,c,d) MR_PASTE4_2(a,b,c,d)
> +#define MR_PASTE4_2(a,b,c,d) a##b##c##d
> +#define MR_PASTE5(a,b,c,d,e) MR_PASTE5_2(a,b,c,d,e)
> +#define MR_PASTE5_2(a,b,c,d,e) a##b##c##d##e
> +#define MR_PASTE6(a,b,c,d,e,f) MR_PASTE6_2(a,b,c,d,e,f)
> +#define MR_PASTE6_2(a,b,c,d,e,f) a##b##c##d##e##f
> +#define MR_PASTE7(a,b,c,d,e,f,g) MR_PASTE7_2(a,b,c,d,e,f,g)
> +#define MR_PASTE7_2(a,b,c,d,e,f,g) a##b##c##d##e##f##g
There's some code duplication with this code and the code in
mercury_std.h/mercury_type_info.h; you should at least document the
fact that it is duplicated in both places.
> +#define Declare_entry(a)
> +#define Declare_struct(a)
Comments please.
> +#define MR_NULL 0
Is that needed? The code elsewhere was using NULL
rather than MR_NULL.
> +#define MR_TYPECTOR_REP_ENUM_val 0
> +#define MR_TYPECTOR_REP_ENUM_USEREQ_val 1
> +#define MR_TYPECTOR_REP_DU_val 2
> +#define MR_TYPECTOR_REP_DU_USEREQ_val 3
> +#define MR_TYPECTOR_REP_NOTAG_val 4
> +#define MR_TYPECTOR_REP_NOTAG_USEREQ_val 5
> +#define MR_TYPECTOR_REP_EQUIV_val 6
> +#define MR_TYPECTOR_REP_EQUIV_VAR_val 7
> +#define MR_TYPECTOR_REP_INT_val 8
> +#define MR_TYPECTOR_REP_CHAR_val 9
> +#define MR_TYPECTOR_REP_FLOAT_val 10
> +#define MR_TYPECTOR_REP_STRING_val 11
> +#define MR_TYPECTOR_REP_PRED_val 12
> +#define MR_TYPECTOR_REP_UNIV_val 13
> +#define MR_TYPECTOR_REP_VOID_val 14
> +#define MR_TYPECTOR_REP_C_POINTER_val 15
> +#define MR_TYPECTOR_REP_TYPEINFO_val 16
> +#define MR_TYPECTOR_REP_TYPECLASSINFO_val 17
> +#define MR_TYPECTOR_REP_ARRAY_val 18
> +#define MR_TYPECTOR_REP_SUCCIP_val 19
> +#define MR_TYPECTOR_REP_HP_val 20
> +#define MR_TYPECTOR_REP_CURFR_val 21
> +#define MR_TYPECTOR_REP_MAXFR_val 22
> +#define MR_TYPECTOR_REP_REDOFR_val 23
> +#define MR_TYPECTOR_REP_REDOIP_val 24
> +#define MR_TYPECTOR_REP_TRAIL_PTR_val 25
> +#define MR_TYPECTOR_REP_TICKET_val 26
> +#define MR_TYPECTOR_REP_NOTAG_GROUND_val 27
> +#define MR_TYPECTOR_REP_NOTAG_GROUND_USEREQ_val 28
> +#define MR_TYPECTOR_REP_EQUIV_GROUND_val 29
This code is very similar to code elsewhere; you should at least
document this fact in both places.
> +#define MR_DEFINE_BUILTIN_TYPE_CTOR_INFO_FULL(m, n, a, cr, u, c) \
> + Declare_entry(u) \
> + Declare_entry(c) \
> + Declare_struct(MR_STATIC_CODE_CONST struct MR_TypeCtorInfo_Struct) \
> + MR_STRUCT_INIT(MR_PASTE5(mercury_data_, __type_ctor_info_, n, _, a) = {) \
> + MR_CLASS_INIT(MR_PASTE4(type_ctor_init_, n, _, a)) \
> + MR_BOX_INT(a), \
> + MR_MAYBE_STATIC_CODE(n##_unify), \
> + MR_MAYBE_STATIC_CODE(n##_unify), \
> + MR_MAYBE_STATIC_CODE(n##_compare), \
> + MR_TYPECTOR_REP(cr), \
> + MR_NULL, \
> + MR_NULL, \
> + MR_string_const(MR_STRINGIFY(m), sizeof(MR_STRINGIFY(m))-1), \
> + MR_string_const(MR_STRINGIFY(n), sizeof(MR_STRINGIFY(n))-1), \
> + MR_RTTI_VERSION, \
> + MR_NULL, \
> + MR_NULL, \
> + MR_BOX_INT(-1), \
> + MR_BOX_INT(-1) \
> + MR_STRUCT_INIT_END(}) \
> + MR_CLASS_INIT_END(m, MR_PASTE5(__, type_ctor_info_, n, _, a), MR_PASTE4(type_ctor_init_, n, _, a))
This code is very similar to code elsewhere; you should
document this fact in both places.
Apart from that, this diff 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-developers mailing list
Post messages to: mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions: mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------
More information about the developers
mailing list