Re: RFC - Refactor tree.h

[Richard Biener <richard dot guenther at gmail dot com>]

Mike Stump <mikestump@comcast.net> wrote:
>On Aug 9, 2013, at 3:36 PM, Diego Novillo <dnovillo@google.com> wrote:
>> This patch is still WIP.  It builds stage1, but I'm getting ICEs
>> during stage 2.
>> 
>> The patch splits tree.h into three files:
>> 
>> - tree-core.h: All data structures, enums and typedefs from
>>  tree.h
>> 
>> - tree-api.h: All extern function definitions from tree.h
>> 
>> - tree-macros.h: All macro accessors, tree checks and other
>>  inline functions.
>
>I don't like this split.  You focus in on the details and sort code by
>detail.  I think this is wrong.  I want code sorted by the features and
>functions it provides, and all like this, go into explainable
>functional bins.  One day, a function might be implemented by a data
>structure, the next day, by a routine, or a macro, or an inline
>function, the form of it doesn't matter.
>
>It is like sorting routines by the first character of the spelling of
>the name of the routine.  tree_to_int, goes into t.c, because tree
>starts with a t.  You rename the function, and suddenly it moves files?
> Bad.
>
>Examine double_int.  All macros for double int, go in double-int.h or
>double-int.c.  All function go in one or the other.  All routines goes
>in one or the other.  All double-int code goes into this bin.  What bin
>is it?  macros?  No, the bin is double-int.
>
>Likewise vec.[ch].  You want tree.h smaller, pick the largest abstract
>group that is contained in that file, and remove it to it's own file. 
>For example, fold.  fold is fold, fold is special.  fold can be in
>fold.h.  If you examine the nature of the code in tree.h:
>
>/* In fold-const.c */
>
>/* Non-zero if we are folding constants inside an initializer; zero    
>                       
>   otherwise.  */
>extern int folding_initializer;
>
>This is a dead giveaway.  The second giveaway, fold-const.c exists. 
>That code isn't in tree.c.  Now, maybe you call it fold-const.h, maybe
>that's better, you get the idea.
>
>Look for isolatable hunks that can exist in their own group of some
>sort.  Look for "/* In ", this is a better way to sort.

I mostly agree - tree-macros.h is a red herring. It should be tree-core.h and tree.h only. What does not belong there should go to other appropriate places, like fold-const.h for example.

Thanks,
Richard.