Re: [RFA] Reverse order of find_function_symbols/find_method in linespec.c

[Keith Seitz <keiths at redhat dot com>]

On 03/05/2013 03:06 PM, Doug Evans wrote:

> So we can fix the (anonymous namespace)::function case,
> *and* namespace::class::method (where method is in class and not a baseclass)
> simply by reversing the order of (1) and (2).

Good catch. I think you're absolutely right. Well, that and my 
philosophy is if it doesn't break the test suite, then it's probably 
okay (at least w.r.t. linespecs). I try to be pretty thorough when 
writing tests for features that I've worked implemented.

I only have one small suggestion. With the reorg introduced by your 
patch, we can unconditionally call find_function_symbols on lookup_name 
and then worry about computing class and method names and calling 
find_method if that fails. Finding a scope operator is irrelevant if we 
found a symbol for lookup_name already.

So the call to find_function_symbols can be pushed even further up. I've 
included a slight revision of your patch which does this.

Thank you for the investigative work!
Keith

diff --git a/gdb/linespec.c b/gdb/linespec.c
index b1395e8..16772cd 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -3073,10 +3073,9 @@ find_linespec_symbols (struct linespec_state *state,
 		       VEC (symbolp) **symbols,
 		       VEC (minsym_and_objfile_d) **minsyms)
 {
-  char *klass, *method, *canon;
-  const char *lookup_name, *last, *p, *scope_op;
+  char *canon;
+  const char *lookup_name;
   struct cleanup *cleanup;
-  VEC (symbolp) *classes;
   volatile struct gdb_exception except;

   cleanup = demangle_for_lookup (name, state->language->la_language,
@@ -3096,75 +3095,89 @@ find_linespec_symbols (struct linespec_state *state,
       cleanup = make_cleanup (xfree, canon);
     }

-  /* See if we can find a scope operator and break this symbol
-     name into namespaces${SCOPE_OPERATOR}class_name and method_name.  */
-  scope_op = "::";
-  p = find_toplevel_string (lookup_name, scope_op);
-  if (p == NULL)
-    {
-      /* No C++ scope operator.  Try Java.  */
-      scope_op = ".";
-      p = find_toplevel_string (lookup_name, scope_op);
-    }
+  /* It's important to not call expand_symtabs_matching unnecessarily
+     as it can really slow things down (by unnecessarily expanding
+     potentially 1000s of symtabs, which when debugging some apps can
+     cost 100s of seconds).  Avoid this to some extent by *first* calling
+     find_function_symbols, and only if that doesn't find anything
+     *then* call find_method.  This handles two important cases:
+     1) break (anonymous namespace)::foo
+     2) break class::method where method is in class (and not a 
baseclass)  */

-  last = NULL;
-  while (p != NULL)
-    {
-      last = p;
-      p = find_toplevel_string (p + strlen (scope_op), scope_op);
-    }
+  find_function_symbols (state, file_symtabs, lookup_name,
+			 symbols, minsyms);

-  /* If no scope operator was found, lookup the name as a symbol.  */
-  if (last == NULL)
+  /* If we were unable to locate a symbol of the same name, try dividing
+     the name into class and method names and searching the class and its
+     baseclasses.  */
+  if (VEC_empty (symbolp, *symbols)
+      && VEC_empty (minsym_and_objfile_d, *minsyms))
     {
-      find_function_symbols (state, file_symtabs, lookup_name,
-			     symbols, minsyms);
-      do_cleanups (cleanup);
-      return;
-    }
+      char *klass, *method;
+      const char *last, *p, *scope_op;
+      VEC (symbolp) *classes;

-  /* NAME points to the class name.
-     LAST points to the method name.  */
-  klass = xmalloc ((last - lookup_name + 1) * sizeof (char));
-  make_cleanup (xfree, klass);
-  strncpy (klass, lookup_name, last - lookup_name);
-  klass[last - lookup_name] = '\0';
-
-  /* Skip past the scope operator.  */
-  last += strlen (scope_op);
-  method = xmalloc ((strlen (last) + 1) * sizeof (char));
-  make_cleanup (xfree, method);
-  strcpy (method, last);
+      /* See if we can find a scope operator and break this symbol
+	 name into namespaces${SCOPE_OPERATOR}class_name and method_name.  */
+      scope_op = "::";
+      p = find_toplevel_string (lookup_name, scope_op);
+      if (p == NULL)
+	{
+	  /* No C++ scope operator.  Try Java.  */
+	  scope_op = ".";
+	  p = find_toplevel_string (lookup_name, scope_op);
+	}

-  /* Find a list of classes named KLASS.  */
-  classes = lookup_prefix_sym (state, file_symtabs, klass);
-  make_cleanup (VEC_cleanup (symbolp), &classes);
-  if (!VEC_empty (symbolp, classes))
-    {
-      /* Now locate a list of suitable methods named METHOD.  */
-      TRY_CATCH (except, RETURN_MASK_ERROR)
+      last = NULL;
+      while (p != NULL)
 	{
-	  find_method (state, file_symtabs, klass, method, classes,
-		       symbols, minsyms);
+	  last = p;
+	  p = find_toplevel_string (p + strlen (scope_op), scope_op);
 	}

-      /* If successful, we're done.  If NOT_FOUND_ERROR
-	 was not thrown, rethrow the exception that we did get.
-	 Otherwise, fall back to looking up the entire name as a symbol.
-	 This can happen with namespace::function.  */
-      if (except.reason >= 0)
+      /* If no scope operator was found, there is nothing more we can do;
+	 we already attempted to lookup the entire name as a symbol
+	 and failed.  */
+      if (last == NULL)
 	{
 	  do_cleanups (cleanup);
 	  return;
 	}
-      else if (except.error != NOT_FOUND_ERROR)
-	throw_exception (except);
+
+      /* LOOKUP_NAME points to the class name.
+	 LAST points to the method name.  */
+      klass = xmalloc ((last - lookup_name + 1) * sizeof (char));
+      make_cleanup (xfree, klass);
+      strncpy (klass, lookup_name, last - lookup_name);
+      klass[last - lookup_name] = '\0';
+
+      /* Skip past the scope operator.  */
+      last += strlen (scope_op);
+      method = xmalloc ((strlen (last) + 1) * sizeof (char));
+      make_cleanup (xfree, method);
+      strcpy (method, last);
+
+      /* Find a list of classes named KLASS.  */
+      classes = lookup_prefix_sym (state, file_symtabs, klass);
+      make_cleanup (VEC_cleanup (symbolp), &classes);
+
+      if (!VEC_empty (symbolp, classes))
+	{
+	  /* Now locate a list of suitable methods named METHOD.  */
+	  TRY_CATCH (except, RETURN_MASK_ERROR)
+	    {
+	      find_method (state, file_symtabs, klass, method, classes,
+			   symbols, minsyms);
+	    }
+
+	  /* If successful, we're done.  If NOT_FOUND_ERROR
+	     was not thrown, rethrow the exception that we did get.  */
+	  if (except.reason < 0 && except.error != NOT_FOUND_ERROR)
+	    throw_exception (except);
+	}
     }

-  /* We couldn't find a class, so we check the entire name as a symbol
-     instead.  */
-   find_function_symbols (state, file_symtabs, lookup_name, symbols, 
minsyms);
-   do_cleanups (cleanup);
+  do_cleanups (cleanup);
 }

 /* Return all labels named NAME in FUNCTION_SYMBOLS.  Return the