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

[Fergus Henderson]

On 05-Aug-1999, Warwick Harvey <wharvey at cs.monash.edu.au> wrote:
> Fergus wrote:
> > 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.

Is that sufficient?  Won't that lead to conflicting definitions for
`struct MR_TypeCtorInfo_struct'?  I think the compiler-generated
declarations of the hand-defined type_ctor_infos will contain
compiler-generated _definitions_ of their types, and I think these
definitions will conflict with the hand-defined definition of that struct.

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

This is not the right fix, I think.
It may happen to work, but it breaks invariants that the compiler
relies on, so even if it does work it may cause problems for maintenance.

The invariants in question are these:
(1) the compiler should know the type of every field that it initializes
(2) the compiler should know the type of every expression that it generates
(3) if an expression is used in a context which expects a different type,
    the compiler should insert the appropriate cast or type conversion.

Previously, the compiler assumed that the arguments of a type_ctor_info
struct were all `Word'; this is indicated by the `uniform(no)' in the
following code

      DataName = type_ctor(info, TypeName, TypeArity),
      CModule = comp_gen_c_data(ModuleName, DataName, Exported,
		      [ArityArg | FinalArgs], uniform(no), Procs),

from line 175 of base_type_info.m.

By changing the definition of the type_ctor_info struct without changing
this `uniform(no)', you broke (1).  This is what caused the compiler warnings.
Your proposed fix to insert a cast in the generated code for `string_consts'
without changing llds_const_type/2 accordingly breaks (2).

I think a fix which maintained invariants (1) and (2) would be better
than your proposed change.

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

The llds_const_type predicate should return a type which corresponds
with the type of the expression that llds_out.m spits out for that constant.
So if you've changed llds_out.m as above, then you should change
llds_const_type/2 accordingly.

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

I don't think committing your full change as is would be a good idea,
because it breaks those invariants mentioned above.

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

I don't think that would be a good idea, because of the problem
with conflicting definitions for those struct types.

> 3.  Do something else.

The right way to fix this, I think, would be to modify line 175 of
base_type_info.m so that it specifies the correct field types for a
type_ctor_info; if you do that, then I think llds_out.m will insert
the correct casts without you needing to do anything more.

(I don't think are any other places which create type_ctor_infos,
and which therefore need similar treatment, but I'm not certain,
so do a grep.)

Cheers,
	Fergus.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>  |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3        |     -- 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
--------------------------------------------------------------------------