On Aug 9, 2013, at 3:36 PM, Diego Novillo <dnovi...@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.

Reply via email to