diff mbox

[(updated)] Convert symtab, cgraph and varpool nodes into a real class hierarchy

Message ID 1383016588.29677.52.camel@surprise
State New
Headers show

Commit Message

David Malcolm Oct. 29, 2013, 3:16 a.m. UTC 
On Mon, 2013-10-28 at 22:57 -0400, David Malcolm wrote:
> On Fri, 2013-09-20 at 17:33 +0200, Jan Hubicka wrote:
> > > This patch is the handwritten part of the conversion of these types
> > > to C++; it requires the followup patch, which is autogenerated.
> > > 
> > > It converts:
> > >   struct symtab_node_base
> > > to:
> > >   class symtab_node_base
> > > 
> > > and converts:
> > >   struct cgraph_node
> > > to:
> > >   struct cgraph_node : public symtab_node_base
> > > and:
> > >   struct varpool_node
> > > to:
> > >   class varpool_node : public symtab_node_base
> > > 
> > > dropping the symtab_node_def union.
> > 
> > Yep, incrementally we should continue with the grand renaming 
> > retiring symtab_node_base and getting things symmetric.
> > > 
> > > 	* cgraph.h (symtab_node_base): Convert to a class;
> > > 	add GTY((desc ("%h.type"), tag ("SYMTAB_SYMBOL"))).
> > > 	(cgraph_node): Inherit from symtab_node; add GTY option
> > > 	tag ("SYMTAB_FUNCTION").
> > > 	(varpool_node): Inherit from symtab_node; add GTY option
> > > 	tag ("SYMTAB_VARIABLE").
> > > 	(symtab_node_def): Remove.
> > > 	(is_a_helper <cgraph_node>::test (symtab_node_def *)): Convert to...
> > > 	(is_a_helper <cgraph_node>::test (symtab_node_base *)): ...this.
> > > 	(is_a_helper <varpool_node>::test (symtab_node_def *)): Convert to...
> > > 	(is_a_helper <varpool_node>::test (symtab_node_base *)): ...this.
> > > 
> > > 	* ipa-ref.h (symtab_node_def): Drop.
> > > 	(symtab_node): Change underlying type from symtab_node_def to
> > > 	symtab_node_base.
> > > 	(const_symtab_node): Likwise.
> > > 
> > > 	* is-a.h: Update examples in comment.
> > > 
> > > 	* symtab.c (symtab_hash): Change symtab_node_def to symtab_node_base.
> > > 	(assembler_name_hash): Likewise.
> > 
> > This patch is OK.  Thanks for working on this!
> 
> These symtab changes were dependent on having gengtype support for
> inheritance, which is now in trunk, so I'm now revisiting these patches.
> 
> The above patch hasn't bitrotted, though the autogenerated one that goes
> with it needed regenerating.
> 
> A new version of the autogenerated patch can be seen at:
> http://dmalcolm.fedorapeople.org/gcc/large-patches/eaba9669644c84592ea32be2dcd19ba92beca381-0003-Autogenerated-fixes-of-symbol.-to.patch
> Is that new patch OK?  (it's 450KB so one's eyes tend to glaze over
> after a while, but FWIW you approved an earlier version of that in:
>   http://gcc.gnu.org/ml/gcc-patches/2013-09/msg00730.html
> and the test suite for the script that generated the patch can be seen
> at:
> https://github.com/davidmalcolm/gcc-refactoring-scripts/blob/master/test_refactor_symtab.py )
> 
> Also, I noticed when reviewing the autogenerated marking routines in
> gtype-desc.c that the symtab_node_def union had chain_next/chain_prev
> markings, and that my patch above effectively dropped them.
> 
> The attached patch reinstates them, albeit to the symtab_node_base base
> class, and inspection of the generated gtype-desc.c shows that the
> generated code does the right thing.
...and here's a revised version one that avoids overlong lines.
diff mbox

Patch

commit 739276306f1d8f5d0b1202da21cc29b5fcac102e
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Mon Oct 28 22:25:38 2013 -0400

    Add chain_next and chain_prev options back to symtab_node_base
    
    gcc/
    	* cgraph.h (symtab_node_base): Add missing chain_next and
    	chain_prev GTY options.

diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 5a582d8..4cc2049 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -39,7 +39,9 @@  enum symtab_type
 
 /* Base of all entries in the symbol table.
    The symtab_node is inherited by cgraph and varpol nodes.  */
-class GTY((desc ("%h.type"), tag ("SYMTAB_SYMBOL"))) symtab_node_base
+class GTY((desc ("%h.type"), tag ("SYMTAB_SYMBOL"),
+	   chain_next ("%h.next"), chain_prev ("%h.previous")))
+  symtab_node_base
 {
 public:
   /* Type of the symbol.  */