[m-dev.] For review: Using MR_TypeCtorInfo for generated type_ctor_infos

[Warwick Harvey]

Fergus wrote:
> If the HAL stuff is sufficiently stable, you might want to consider
> committing it into the main branch.   Of course, for this to happen,
> any parts in the runtime should be #ifdef'd, any parts in the compiler
> should be either #ifdef'd or controlled by a compiler option,
> and the changes would have to be reasonably well isolated.

The "normal" compiler grades should, in theory, be completely unchanged.  To 
get the HAL features, one needs to specify some extra grade components.  
This is not strictly true, since one cannot have certain predicates 
available in one grade but not another, and so there are some extra library 
predicates available in "normal" grades which should not be there (they do 
nothing except print an error at runtime).

The other thing is that we actually track the alias branch rather than the 
main branch, for its better mode handling.  The alias branch doesn't give us 
as much as we first thought it would, so we still have to generate hacky 
code for some kinds of operations.  These are not so different from the 
kinds of hacks we would need to get it to work on the main branch, so in 
theory it's possible for us to move back to tracking the main branch, but 
that's not a move to be taken lightly.

> Well, if the change is going to cause lots of warnings, then you should
> not commit it.  Those warnings ("assignment from incompatible pointer type")
> should be treated as errors, since many C compilers do in fact treat them
> as such; GNU C is undesirably permissive in this respect.

Yes; I planned to find a solution to the warnings before committing, unless 
directed otherwise.  (Addendum: I seem to have found said solution --- see 
below.)

> Changing just the hand-defined ones might still lead to problems,
> because the compiler creates compiler-generated declarations for
> uses of the hand-defined ones, and these declarations might conflict
> with the hand-defined definitions.

Yes, it does and they do (which is why I changed the compiler-generated 
declarations), but I found a solution to that: #define the 
compiler-generated type to `MR_TypeCtorInfo_struct' for any hand-defined 
type_ctor_infos.  This needed to be done for some anyway, for bootstrapping.


Anyway, I've done some more experimentation, and appear to have eliminated 
all the warnings I introduced.  I did it by adding casts to `String' to all 
the generated `string_const' calls.  I hadn't done this initially because I 
was worried that other things might generate calls to `string_const', and so 
I'd just be shifting where the warnings were.  That appears not to be the 
case.

Of course, the definition of `string_const' itself includes a cast (to `Word 
*'), so that could be changed instead, but there *are* other places which 
call `string_const' manually, so I don't particularly want to "risk" 
changing it.

Here's the relative diff:

Index: compiler/llds_out.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/llds_out.m,v
retrieving revision 1.115
diff -u -r1.115 llds_out.m
--- llds_out.m  1999/07/13 08:53:06     1.115
+++ llds_out.m  1999/08/05 07:33:44
@@ -3660,14 +3676,14 @@
        io__write_string("(Float) "),
        io__write_float(FloatVal).
 output_rval_const(string_const(String)) -->
-       io__write_string("string_const("""),
+       io__write_string("(String) string_const("""),
        output_c_quoted_string(String),
        { string__length(String, StringLength) },
        io__write_string(""", "),
        io__write_int(StringLength),
        io__write_string(")").
 output_rval_const(multi_string_const(Length, String)) -->
-       io__write_string("string_const("""),
+       io__write_string("(String) string_const("""),
        output_c_quoted_multi_string(Length, String),
        io__write_string(""", "),
        io__write_int(Length),



Also, while I was playing around with the above, I made the following 
change, because it looked wrong and I thought it might have had something to 
do with the problem.  It didn't, and the change seems to have had no effect 
(as far as I could tell), but there may be other implications.  Can somebody 
who knows about this stuff comment on whether it was fine as it was, or 
whether it should be changed?  Of course if it should be changed, no doubt 
the `multi_string_const' entry should be changed as well.

Index: compiler/llds.m
===================================================================
RCS file: /home/mercury1/repository/mercury/compiler/llds.m,v
retrieving revision 1.244
diff -u -r1.244 llds.m
--- llds.m      1999/07/13 08:53:05     1.244
+++ llds.m      1999/08/05 07:33:43
@@ -1006,7 +1006,7 @@
 llds__const_type(false, bool).
 llds__const_type(int_const(_), integer).
 llds__const_type(float_const(_), float).
-llds__const_type(string_const(_), data_ptr).
+llds__const_type(string_const(_), string).
 llds__const_type(multi_string_const(_, _), data_ptr).
 llds__const_type(code_addr_const(_), code_ptr).
 llds__const_type(data_addr_const(_), data_ptr).


Anyway, can somebody who know more about this stuff (I knew nothing about it 
just a few days ago, so I don't exactly trust my own judgement here) tell me 
whether I should:

1.  Go with the full change (all type_ctor_info structures use the 
MR_TypeCtorInfo type, whether hand-defined or compiler-generated) with the 
above fix for the warnings.

2.  Go with a partial change, where just the hand-defined type_ctor_info 
structures use the MR_TypeCtorInfo type, with the #define trick to cope with 
compiler-generated type declarations for these structures.

or

3.  Do something else.

Presumably another review will be warranted, except possibly for option 1.

Warwick

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