Re: Cgraph Modification Plan

[Jan Hubicka <hubicka at ucw dot cz>]

> What do you think of the following plan for turning cgraph into
> a class hierarchy?  We cannot finish it until we have gengtype
> understanding single inheritance, but we can start changing APIs
> in preparation.

Good you told me, I was about trying that myself. Did not know gengtype
do not understand inheritance yet.
> 
> APPROACH ###########################################
> 
> Add converters and testers.
> Change callers to use those.
> Change callers to use type-safe parameters.
> Change implementation to class hierarchy.
> Add accessors.

Sounds good to me.
> 
> 
> CONVERTERS AND TESTERS ###########################################
> 
> add
>     symtab_node_base &symtab_node_def::ref_symbol()
> 	{ return symbol; }
>     symtab_node_base &cgraph_node::ref_symbol()
> 	{ return symbol; }
>     symtab_node_base &varpool_node::ref_symbol()
> 	{ return symbol; }
> 
> change
>     node->symbol.whatever
> to
>     node->ref_symbol().whatever

OK, the basic idea is that symtab_node is basetype of cgraph_node and
varpool_node.  We may want to drop the historica cgraph/varpool names here,
since function_node/variable_node would sound better. Cgraph still exists, but
varpool is more or less historical relic.

I would expect inheritance to allow me to write node->whatever and to have
casting operators to convert things back and forth, so we only need to add
testers?  Probably is_function/is_variable & try_function/try_variable sounds
more readable to me.
What do you think? (I just arrived from China, so will take it more tought
once unjetlagged)

Honza
> 
> ----
> 
> should not need to add these
> 
>     cgraph_node &symtab_node_def::ref_cgraph()
> 	{ gcc_assert (symbol.type == SYMTAB_FUNCTION);
> 	  return x_function; }
>     varpool_node &symtab_node_def::ref_varpool()
> 	{ gcc_assert (symbol.type == SYMTAB_VARIABLE);
> 	  return x_variable; }
> 
> ----
> 
> add
>     symtab_node_base *symtab_node_def::try_symbol()
> 	{ return &symbol; }
>     cgraph_node *symtab_node_def::try_cgraph()
> 	{ return symbol.type == SYMTAB_FUNCTION ? &x_function : NULL; }
>     varpool_node *symtab_node_def::try_varpool()
> 	{ return symbol.type == SYMTAB_VARIABLE ? &x_variable : NULL; }
> 
> change
>     if (symtab_function_p (node) && cgraph (node)->analyzed)
>       return cgraph (node);
> to
>     if (cgraph_node *p = node->try_cgraph())
>       if (p->analyzed)
>         return p;
> 
> change
>     if (symtab_function_p (node) && cgraph (node)->callers)
>       ....
> to
>     if (cgraph_node *p = node->try_cgraph())
>       if (p->callers)
>         ....
> 
> change
>     if (symtab_function_p (node))
>       {
>         struct cgraph_node *cnode = cgraph (node);
>         ....
> to
>     if (cgraph_node *cnode = node->try_cgraph ())
>       {
>         ....
> 
> 
> likewise "symtab_variable_p (node)" and "varpool (node)"
> 
> ----
> 
> If there are any "symtab_function_p (node)" expressions left,
> 
> add
>     bool symtab_node_def::is_cgraph()
>         { return symbol.type == SYMTAB_FUNCTION; }
>     bool symtab_node_def::is_varpool()
>         { return symbol.type == SYMTAB_VARIABLE; }
> 
> change
>     symtab_function_p (node)
> to
>     node->is_cgraph ()
> 
> likewise "symtab_variable_p (node)"
> 
> 
> ----
> 
> Though we would like to avoid doing so,
> if there are any "cgraph (node)" or "varpool (node)" expressions left,
> 
> add
> 
>     symtab_node_base *symtab_node_def::ptr_symbol()
> 	{ return &symbol; }
>     cgraph_node *symtab_node_def::ptr_cgraph()
> 	{ gcc_assert (symbol.type == SYMTAB_FUNCTION);
> 	{ return &x_function; }
>     varpool_node *symtab_node_def::ptr_varpool()
> 	{ gcc_assert (symbol.type == SYMTAB_VARIABLE);
> 	{ return &x_variable; }
> 
> change
>     cgraph (node) => node->ptr_cgraph()
> 
> likewise "varpool (node)"
> 
> 
> TYPE SAFETY ###########################################
> 
> If a function asserts that its symtab_node parameter is symtab_function_p,
> then convert the function to take a cgraph_node*
> and change the callers to convert as above.
> 
> 
> PROPOSED HIERARCHY ###########################################
> 
> enum symtab_type { SYMTAB_SYMBOL, SYMTAB_FUNCTION, SYMTAB_VARIABLE };
> 
> struct symtab_node_base /* marked SYMTAB_SYMBOL */
>   embeds symtab_type
> 
> cgraph_node : symtab_node_base /* marked SYMTAB_FUNCTION */
> 
> varpool_node : symtab_node_base /* marked SYMTAB_VARIABLE */
> 
> changing
>     typedef union symtab_node_def *symtab_node;
>     typedef const union symtab_node_def *const_symtab_node;
> to
>     typedef symtab_node_base *symtab_node;
>     typedef const symtab_node_base *const_symtab_node;
> 
> changing used of
>     symtab_node_def
> to
>     symtab_node_base
> 
> -- 
> Lawrence Crowl