[m-rev.] for review: RTTI/unify/compare for builtin types in the runtime

[Fergus Henderson]

On 07-Aug-2002, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> +++ library/profiling_builtin.m	2002/08/07 03:41:19
> @@ -252,260 +252,30 @@
>  ").
>  
>  %---------------------------------------------------------------------------%
> -% Call port procedures
> +% Port procedures
>  %---------------------------------------------------------------------------%
>  
> +:- external(det_call_port_code_ac/3).
> +:- external(det_call_port_code_sr/4).
> +:- external(det_exit_port_code_ac/2).
> +:- external(det_exit_port_code_sr/3).
> +:- external(semi_call_port_code_ac/3).
> +:- external(semi_call_port_code_sr/4).
> +:- external(semi_exit_port_code_ac/2).
> +:- external(semi_exit_port_code_sr/3).
> +:- external(semi_fail_port_code_ac/2).
> +:- external(semi_fail_port_code_sr/3).
> +:- external(non_call_port_code_ac/4).
> +:- external(non_call_port_code_sr/5).
> +:- external(non_exit_port_code_ac/2).
> +:- external(non_exit_port_code_sr/3).
> +:- external(non_redo_port_code_ac/2).
> +:- external(non_redo_port_code_sr/2).
> +:- external(non_fail_port_code_ac/2).
> +:- external(non_fail_port_code_sr/3).

It would be helpful to put a comment here explaining where they are defined.

> --- /dev/null	Tue Aug  6 19:10:00 2002
> +++ mercury_builtin_types.c	Sat Aug  3 20:49:28 2002
...
> +
> +/* unify and compare of type_ctor_infos are usually handled by generic code */

I don't understand that comment.
(Likewise for similar ones after this.)

> Index: runtime/mercury_builtin_types.h
> +/*
> +** mercury_builtin_types.h
> +**
> +*/
> +
> +#ifndef MERCURY_BUILTIN_H
> +#define MERCURY_BUILTIN_H

The guard macro name should match the header file name.

> +#define MR_DEFINE_PROC_STATICS(mod, n, a)                               \
> +    MR_proc_static_compiler_empty(mod, __Unify__, n, a, 0,              \
> +        MR_STRINGIFY(MR_PASTE2(mod, .m)), 0, MR_TRUE);                  \
> +    MR_proc_static_compiler_empty(mod, __Compare__, n, a, 0,            \
> +        MR_STRINGIFY(MR_PASTE2(mod, .m)), 0, MR_TRUE);

I think that is undefined behaviour, because `foo.m' is not a valid
preprocessing token.  (The relevant sections in the C99 standard
are 6.10.3.3 [#3] and 6.4 [#1], which I have quoted below.)

Rather than using token pasting, you can just use ANSI/ISO string
concatenation:

    MR_proc_static_compiler_empty(mod, __Compare__, n, a, 0,               \
        MR_STRINGIFY(mod) ".m", 0, MR_TRUE);

Incidentally, why do these strings include the ".m" anyway?

 |        6.10.3.3  The ## operator
 |        [#3]   ...  each  instance  of  a  ##
 |        preprocessing token in the replacement  list  ...
 |        is  deleted and the preceding preprocessing token
 |        is concatenated  with  the  following  preprocessing  token.
 |        ... If the result is not a valid preprocessing token, the  behavior
 |         is undefined.

 |        6.4  Lexical elements
 |  
 |        Syntax
 |  
 |        [#1]
 |                preprocessing-token:
 |                        header-name
 |                        identifier
 |                        pp-number
 |                        character-constant
 |                        string-literal
 |                        punctuator
 |                        each non-white-space character that cannot be one
 |			          of the above

> +++ make_port_code	Sun Aug  4 13:35:53 2002

In general, some more comments here, either in the generated code
or sprinkled among the code which generates it, would be helpful.
Why does the generated code have the form that it does?
What is the function of this code?

> +                echo "#define MR_PROCNAME \"${msgname}\""
> +                echo "#define MR_VERSION_${IMPL}"
> +                echo "#define ${portdef}"
> +                echo "${outermost_define} MR_NEED_NEW_OUTERMOST"
> +                cat ${file}

Why not
		   echo "#include ${file}"
?

> +#---------------------------------------------------------------------------#
> +# assembler the header file

s/assembler/assemble/ ?

> Index: trace/Mmakefile
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/trace/Mmakefile,v
> retrieving revision 1.31
> diff -u -b -r1.31 Mmakefile
> --- trace/Mmakefile	2002/06/22 19:16:15	1.31
> +++ trace/Mmakefile	2002/08/03 11:07:24
> @@ -20,7 +20,7 @@
>  #-----------------------------------------------------------------------------#
>  
>  CFLAGS		+= -I$(BROWSER_DIR) -g $(DLL_CFLAGS) \
> -			-DMERCURY_BOOTSTRAP_H -DMERCURY_CONF_BOOTSTRAP_H
> +			-DMERCURY_CONF_BOOTSTRAP_H
>  MGNUCFLAGS	+= --no-ansi

This change disables the static checking that nothing in the
trace directory depends on obsolete stuff in mercury_bootstrap.h.
That static checking is important, IMHO.

Otherwise that change looks fine.

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