On Tue, Jan 17, 2012 at 12:41 PM, Jason Merrill <ja...@redhat.com> wrote: >> + /* If the original DIE was a specification, we need to put >> + the skeleton under the parent DIE of the declaration. */ >> + if (new_parent != NULL) >> + { >> + remove_child_with_prev (child, prev); >> + add_child_die (new_parent, skeleton); >> + } > > > This adds a new declaration without removing the old one, though it's later > removed by prune_unused_types.
Yes. At this point, I don't have the "prev" pointer needed to remove the old one efficiently, and since it will get pruned anyway, it seemed more efficient to let that happen. If it doesn't actually get pruned, I think there's no harm done other than an unnecessary DIE. Would you consider it OK with a comment? > It also seems to me that copy_declaration_context and > remove_child_or_replace_with_skeleton should be combined if they need to > work together like this. How about if I call copy_declaration_context directly from remove_child_or_replace_with_skeleton, instead of calling them sequentially in break_out_comdat_types? Or would you prefer combining them into a single function? -cary