> 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