> On Thu, Oct 4, 2012 at 8:16 PM, Diego Novillo <dnovi...@google.com> wrote:
> > On Thu, Oct 4, 2012 at 2:14 PM, Lawrence Crowl <cr...@googlers.com> wrote:
> >
> >> So, Jan Hubicka requested and approved the current spelling.
> >> What now?
> >
> > I don't think we should hold this up.  The names Jan requested seem
> > reasonable enough.  We seem to be running in circles here.
> 
> I suppose I have your promise that we'll release with consistent names.
> Please allocate some work hours on your side for the renaming of
> cgraph_node and varpool_node for the case Honza doesn't get to it in time.

Not that I would not agree with Richard with most of his points.  To be however
fair here, I only asked to continue naming I already introduced in:

/* Return true when NODE is function.  */
static inline bool
symtab_function_p (symtab_node node)
{
  return node->symbol.type == SYMTAB_FUNCTION;
}

/* Return true when NODE is variable.  */
static inline bool
symtab_variable_p (symtab_node node)
{
  return node->symbol.type == SYMTAB_VARIABLE;
}

Even if variable are represented by varpool and functions by cgraph, I do not
see these names more confusing compared to symtab_cgraph_p/symtab_varpool_p.
The most common use is when you walk the symbol table and you want to handle
functions and variables specially.

The new interface with try_ scheme gives a bit more inconsistent feeling,
but it is just an surface, nothing really changes.

I am not happy with current naming scheme and also with the fact that for
gengtype reasons we also have symtab_node typedef, but for varpool and cgraph
we use struct (this is because symtab_node has to be union without GTY
supporting inheritance).

Introducing symtab I did not have much other options and I expected that
at the end of this stage1 I will clean it up.  This is, well, more or less now
when assuming that there are no major patches to this area just to appear
for this stage1.

I guess we all agreed we want to drop cgraph/varpool nodes in favor of
functions/ variables.  How realistic is for gengtype to support inheritance
this release cycle?  I would really like to see these three turned into classes
with the inheritance this release cycle.  Renaming them during the process
should be easy and just a nice bonus.

Honza

Reply via email to