[m-rev.] for review: insts for type constructors

[Zoltan Somogyi]

On Sun, 4 Oct 2015 15:39:16 +1100 (AEDT), Julien Fischer <jfischer at opturion.com> wrote:
> > library/ops.m:
> >     Add "for" as an operator, as discussed on the mailing list.
> 
> The addition of "for" as an operator is potentially going to break existing
> code.  The operator table in the reference manual should be updated now and
> there should be an entry in the news file mentioning that "for" is now an
> operator.

Done and done. The NEWS entry is very bare bones.

I did notice that the operator table in the reference manual is missing
several operators in ops.m, such as "or_else". I will look into this after
I commit this diff.

> > +        % We *should* do two kinds of replacements on _ItemInstDefn0.
> > +        %
> > +        % (1) If inst i1's body contains inst i2, and i2 has been defined
> > +        % to be equivalent to some other inst i3, then we should replace
> > +        % i2 with i3 in i1's body. We haven't ever done this, and it is
> > +        % a bit surprising that it has never been a problem so far.
> 
> It isn't that surprising.  Most Mercury programs I'm aware of either don't
> define their own insts or, if they do, are not very ambitious about what
> insts they define (since being ambitious in this respect tends to run into
> limitations of the mode checker :-( )

That is why I said "a bit".

I will construct a test case after I commit this diff, and fix the bug if it is one.

> > +        % (2) If inst i1 is for type t2, and t2 has been defined to be
> > +        % equivalent to type t3, then we should record that i1 is really
> > +        % for t3. However, while t2 is required to be just a type_ctor
> > +        % and arity, t3 may be more complex. The obvious thing to do would be
> > +        % to record that i1 is for t3's top type_ctor and its arity. Whether
> 
> Are there any specific non-obvious things you had in mind?

The obvious non-obvious thing to do (:-) would be to go with my original
second proposal: to record not a type_ctor for each inst, but a type template,
i.e. a type with one OR MORE type constructors and zero or more type variables.
We would probably want to require the type vars to be distinct. However,
I don't want to implement that unless (a) it turns out implementing it is easy, or
(b) it turns out to be needed. I think (b) is unlikely.
 
> > +        % that is good enough depends on what *exactly* we will do with the
> > +        % "inst for type ctor" information. We don't yet know the answer
> > +        % to that question.
> > +        Item = Item0,
> > +        Specs = []
> > +    ;
> 
> One thing we should be able to do with the "inst for type ctor" information is
> (finally) fix  bug #89 -- inst subtyping involving existentially quantified
> data constructors.

I am adding a note to that effect.
 

> >          Item = item_inst_defn(ItemInstDefn),
> > -        ItemInstDefn = item_inst_defn_info(Name, Params, _, _, _, _),
> > +        % XXX IFTC Do we need to check _MaybeForTypeCtor?
> 
> I'm not sure, but *if* we do then we possibly also need to do so for the
> various pragmas that refer to type ctors as well (e.g. foreign_enum).

Yes, that was my feeling too. However, since I don't usually use mmc --make,
while other people do, I didn't think I should be deciding that it is ok for
mmc --make to miss some needed recompilations :-(. Luckily, in this case,
that won't be an issue until we start using this new language feature,
*and* start changing what type_ctor an inst refers to ...

> The diff looks fine.

Thanks for the review.

Zoltan.