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

[Michael Matz <matz at suse dot de>]

Hi,

On Wed, 6 Nov 2013, David Malcolm wrote:

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

I know all that.  But there's a point on the scale from a void* type 
system to a different-type-for-everything system where the advantages of a 
more complex type system are anulled by the disadvantages of a more 
complex type system.  And I'm merely of the opinion that you crossed that 
point with this separation.

You know we could have a typesystem where every different natural number 
is a type (and with C++ we _really_ could do that).  Obviously that's also 
a reductio ad absurdum, but it demonstrates the point that there _are_ 
type systems with a too low complexity to expressiveness ratio.

> * it permits the proof of absence of certain bugs
> * it supports abstraction
> * it documents the intent of the code

Well, yes.  You can document code with other things than types, you can 
add abstractions via different means, and certain bugs can indeed only be 
ruled out by different static types.

In my mind types should abstract different larger concepts foremost (and 
not every little aspect of a concept).  Only _then_ should they be used 
for other effects like the above, and only if the disadvantages don't 
overshadow that.  Disadvantages being mostly simply more complexity: which 
types relate to others in which way; what are the methods you can do with 
that but not the other type; which are the members of the 100 types there 
are (and if a type system is too large to be used only by IDEs with code 
completion, then it's too complex); and of course also simply having to 
write more text when you can't use the same gimple variable for several 
statements.

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

But a gimple switch is actually a special case of a gimple conditional 
jump (the normal GIMPLE_COND being another) which itself is a special 
case of a gimple control transfer (a thing causing one or multiple 
outgoing edges and ending a basic block).  That enlightens that your 
hierarchy might not actually be the best split of types, it's merely a 
formal translation of gimple codes into types.

And I question exactly that automatic introduction of types.  There's not 
enough human thought put into the hierarchy to warrant the change.  Types 
are a fundament and they shouldn't be introduced lightly.  As gimple codes 
are simply flat it doesn't matter so much that the inherent structure of 
gimple statements isn't reflected (although there _is_ some structure by 
having the with_ops and with_mem_ops codes be together).  But for a _type 
hierarchy_ I would pretty much require that real thought and insights are 
put into it.

> Consider tree-switch-conversion.c: this contains various "gimple"
> variables but they don't work on arbitrary gimple; they require code ==
> GIMPLE_SWITCH:
> ...
> We could be doing this directly in the type system, and using the
> compiler to check it.

Then please do that while introducing one new type.

> >   The fewer 
> > types the better IMO.
> 
> There's a reductio ad absurdum of that argument:

See above for my variant of that showing that it also goes into the other 
direction of more types.


Ciao,
Michael.
P.S.: And do away with the "_statement" part of the type, gimple_switch 
ought to be enough.