On Mon, Apr 16, 2018 at 9:45 PM, Alexandre Oliva <aol...@redhat.com> wrote: > On Apr 16, 2018, Jason Merrill <ja...@redhat.com> wrote: > >> This change looks good. One other nit: get_decl_maybe can also return a >> list, so the name seems wrong. > > Uhh, it's "give me the decl if you got one, but it's ok if you give me a > list" (the latter is what makes it _maybe). It replaces what used to be > called just 'decl'. Maybe wrong is a bit too strong, but... do you > have any suggestion? get_decl_or_tree_list_but_dont_build_the_list is a > bit too long IMHO ;-)
I was thinking maybe_get_node, since it returns either the same as get_node or null (and maybe usually comes early in a function name). >> And this line > >> + if (obj->list_p () && obj->get_decl_maybe ()) > >> could be > >> if (tree_list_p ()) > > It could if we made tree_list_p public. It's private because that's an > internal implementation detail, though it's one that happens to leak > through the combination of calls above. I suppose better would be for + if (obj->list_p () && obj->get_decl_maybe ()) + tree_list_freelist ().free (obj->get_node ()); to be e.g. obj->maybe_free_list(); Or put the list handling in a destructor for tinst_level, and have freelist use placement new and explicit destruction. Jason
