On Thu, Feb 17, 2022 at 3:24 PM Patrick Palka <ppa...@redhat.com> wrote: > > name_lookup::search_unqualified uses a statically allocated vector > in order to avoid repeated reallocation, under the assumption that > the function can't be called recursively. With modules however, > this assumption turns out to be false, and search_unqualified can > be called recursively as demonstrated by testcase in comment #19 > of PR99479[1] where the recursive call causes the vector to get > reallocated which invalidates the reference held by the parent call. > > This patch makes search_unqualified instead use an auto_vec with 16 > elements of internal storage (since with the various libraries I tested, > the size of the vector never exceeded 12). In turn we can simplify the > API of subroutines to take the vector by reference and return void. > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for > trunk?
Ping. > > [1]: https://gcc.gnu.org/PR99479#c19 > > PR c++/99479 > > gcc/cp/ChangeLog: > > * name-lookup.cc (name_lookup::using_queue): Change to an > auto_vec (with 16 elements of internal storage). > (name_lookup::queue_namespace): Change return type to void, > take queue parameter by reference and adjust function body > accordingly. > (name_lookup::do_queue_usings): Inline into ... > (name_lookup::queue_usings): ... here. As in queue_namespace. > (name_lookup::search_unqualified): Don't make queue static, > assume its incoming length is 0, and adjust function body > accordingly. > --- > gcc/cp/name-lookup.cc | 62 +++++++++++++++---------------------------- > 1 file changed, 22 insertions(+), 40 deletions(-) > > diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc > index 93c4eb7193b..5c965d6fba1 100644 > --- a/gcc/cp/name-lookup.cc > +++ b/gcc/cp/name-lookup.cc > @@ -429,7 +429,7 @@ class name_lookup > { > public: > typedef std::pair<tree, tree> using_pair; > - typedef vec<using_pair, va_heap, vl_embed> using_queue; > + typedef auto_vec<using_pair, 16> using_queue; > > public: > tree name; /* The identifier being looked for. */ > @@ -528,16 +528,8 @@ private: > bool search_usings (tree scope); > > private: > - using_queue *queue_namespace (using_queue *queue, int depth, tree scope); > - using_queue *do_queue_usings (using_queue *queue, int depth, > - vec<tree, va_gc> *usings); > - using_queue *queue_usings (using_queue *queue, int depth, > - vec<tree, va_gc> *usings) > - { > - if (usings) > - queue = do_queue_usings (queue, depth, usings); > - return queue; > - } > + void queue_namespace (using_queue& queue, int depth, tree scope); > + void queue_usings (using_queue& queue, int depth, vec<tree, va_gc> > *usings); > > private: > void add_fns (tree); > @@ -1084,39 +1076,35 @@ name_lookup::search_qualified (tree scope, bool > usings) > /* Add SCOPE to the unqualified search queue, recursively add its > inlines and those via using directives. */ > > -name_lookup::using_queue * > -name_lookup::queue_namespace (using_queue *queue, int depth, tree scope) > +void > +name_lookup::queue_namespace (using_queue& queue, int depth, tree scope) > { > if (see_and_mark (scope)) > - return queue; > + return; > > /* Record it. */ > tree common = scope; > while (SCOPE_DEPTH (common) > depth) > common = CP_DECL_CONTEXT (common); > - vec_safe_push (queue, using_pair (common, scope)); > + queue.safe_push (using_pair (common, scope)); > > /* Queue its inline children. */ > if (vec<tree, va_gc> *inlinees = DECL_NAMESPACE_INLINEES (scope)) > for (unsigned ix = inlinees->length (); ix--;) > - queue = queue_namespace (queue, depth, (*inlinees)[ix]); > + queue_namespace (queue, depth, (*inlinees)[ix]); > > /* Queue its using targets. */ > - queue = queue_usings (queue, depth, NAMESPACE_LEVEL > (scope)->using_directives); > - > - return queue; > + queue_usings (queue, depth, NAMESPACE_LEVEL (scope)->using_directives); > } > > /* Add the namespaces in USINGS to the unqualified search queue. */ > > -name_lookup::using_queue * > -name_lookup::do_queue_usings (using_queue *queue, int depth, > - vec<tree, va_gc> *usings) > +void > +name_lookup::queue_usings (using_queue& queue, int depth, vec<tree, va_gc> > *usings) > { > - for (unsigned ix = usings->length (); ix--;) > - queue = queue_namespace (queue, depth, (*usings)[ix]); > - > - return queue; > + if (usings) > + for (unsigned ix = usings->length (); ix--;) > + queue_namespace (queue, depth, (*usings)[ix]); > } > > /* Unqualified namespace lookup in SCOPE. > @@ -1128,15 +1116,12 @@ name_lookup::do_queue_usings (using_queue *queue, int > depth, > bool > name_lookup::search_unqualified (tree scope, cp_binding_level *level) > { > - /* Make static to avoid continual reallocation. We're not > - recursive. */ > - static using_queue *queue = NULL; > + using_queue queue; > bool found = false; > - int length = vec_safe_length (queue); > > /* Queue local using-directives. */ > for (; level->kind != sk_namespace; level = level->level_chain) > - queue = queue_usings (queue, SCOPE_DEPTH (scope), > level->using_directives); > + queue_usings (queue, SCOPE_DEPTH (scope), level->using_directives); > > for (; !found; scope = CP_DECL_CONTEXT (scope)) > { > @@ -1144,19 +1129,19 @@ name_lookup::search_unqualified (tree scope, > cp_binding_level *level) > int depth = SCOPE_DEPTH (scope); > > /* Queue namespaces reachable from SCOPE. */ > - queue = queue_namespace (queue, depth, scope); > + queue_namespace (queue, depth, scope); > > /* Search every queued namespace where SCOPE is the common > ancestor. Adjust the others. */ > - unsigned ix = length; > + unsigned ix = 0; > do > { > - using_pair &pair = (*queue)[ix]; > + using_pair &pair = queue[ix]; > while (pair.first == scope) > { > found |= search_namespace_only (pair.second); > - pair = queue->pop (); > - if (ix == queue->length ()) > + pair = queue.pop (); > + if (ix == queue.length ()) > goto done; > } > /* The depth is the same as SCOPE, find the parent scope. */ > @@ -1164,7 +1149,7 @@ name_lookup::search_unqualified (tree scope, > cp_binding_level *level) > pair.first = CP_DECL_CONTEXT (pair.first); > ix++; > } > - while (ix < queue->length ()); > + while (ix < queue.length ()); > done:; > if (scope == global_namespace) > break; > @@ -1181,9 +1166,6 @@ name_lookup::search_unqualified (tree scope, > cp_binding_level *level) > > dedup (false); > > - /* Restore to incoming length. */ > - vec_safe_truncate (queue, length); > - > return found; > } > > -- > 2.35.1.193.g45fe28c951 >
