This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFC - Refactor tree.h


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.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]