Re: [PATCH] Add gimple subclasses for every gimple code (was Re: [PATCH 0/6] Conversion of gimple types to C++ inheritance (v3))

[David Malcolm <dmalcolm@redhat.com>]

On Wed, 2013-11-06 at 16:32 +0100, Michael Matz wrote:
> Hi,
> 
> On Tue, 5 Nov 2013, David Malcolm wrote:
> 
> > Here's a followup patch which ensures that every gimple code has its own
> > subclass, by adding empty subclasses derived from the GSS_-based
> > subclasses as appropriate (I don't bother for gimple codes that already
> > have their own subclass due to having their own GSS layout).  I also
> > copied the comments from gimple.def into gimple.h, so that Doxygen picks
> > up on the descriptions and uses them to describe each subclass.
> 
> I don't like that.  The empty classes are just useless, they imply a 
> structure that isn't really there, some of the separate gimple codes are 
> basically selectors of specific subtypes of a generic concept, without 
> additional data or methods; creating a type for those is confusing.

A type system does more than just express memory layouts:
* it permits the proof of absence of certain bugs
* it supports abstraction
* it documents the intent of the code

To give an example from the patch, the proposed gimple_statement_switch
subclass of gimple_statement_with_ops adds no extra fields, but it adds
the invariant that (assuming the ptr is non-NULL), that code ==
GIMPLE_SWITCH, or, more intuitively, "it's a switch statement".

This means that if we have a gimple_statement_switch, that both a human
reading the code and the compiler can "know" at compile-time that the
code == GIMPLE_SWITCH. 

The accessors are the big use-case for this, which would be a large
patch (as discussed elsewhere in the thread), but there are other places
where this could be used.

Consider tree-switch-conversion.c: this contains various "gimple"
variables but they don't work on arbitrary gimple; they require code ==
GIMPLE_SWITCH:

For example:
/* Collect information about GIMPLE_SWITCH statement SWTCH into INFO.
*/

static void
collect_switch_conv_info (gimple swtch, struct switch_conv_info *info)
{...}

Right now we're expressing the code == GIMPLE_SWITCH invariant via
naming-conventions ("swtch") and runtime checking in the checked build
(covering documentation of intent and implementation respectively).

We could be doing this directly in the type system, and using the
compiler to check it.

> Generally I don't like complicating the type system without good reasons 
> (as in actually also making use of the complicated types).

I can post a followup patch that makes use of each of these, if it will
help.

>   The fewer 
> types the better IMO.

There's a reductio ad absurdum of that argument: reduce the number of
types to zero and use void* everywhere, replacing field lookups with
pointer arithmetic and casts [1].  Obviously this is a strawman, but
looking through tree(-core).h, gimple.h and rtl.h it feels to me all too
reminiscent of the current implementation, alas.

Hope this is constructive.
Dave

[1] or cons cells if you're that way inclined.