On Tue, Mar 1, 2022 at 8:13 AM Patrick Palka <ppa...@redhat.com> wrote: > > 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.
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 > >
