Improve choice sorting in ada-lang.c

ada-lang.c has a "sort_choices" function that claims to sort the
symbol choices, but which does not really implement sorting.  This
patch changes this code to really sort the result vector, sorting
first by filename, then line number, and finally by the symbol name.

The filename sorting is done first by comparing basenames.  It turns
out that gnatmake and gprbuild invoke the compiler a bit differently,
so depending on which one you use, the results of a naive sort might
be different (due to the use of absolute or relative paths).

gdb/ada-lang.c

@@ -3424,67 +3424,45 @@ ada_decoded_op_name (enum exp_opcode op)
   error (_("Could not find operator name for opcode"));
 }
 
-/* Returns true (non-zero) iff decoded name N0 should appear before N1
+/* Sort SYMS to put the choices in a canonical order by the encoded
-   in a listing of choices during disambiguation (see sort_choices, below).
+   names.  */
-   The idea is that overloadings of a subprogram name from the
-   same package should sort in their source order.  We settle for ordering
-   such symbols by their trailing number (__N  or $N).  */
-
-static int
-encoded_ordered_before (const char *N0, const char *N1)
-{
-  if (N1 == NULL)
-    return 0;
-  else if (N0 == NULL)
-    return 1;
-  else
-    {
-      int k0, k1;
-
-      for (k0 = strlen (N0) - 1; k0 > 0 && isdigit (N0[k0]); k0 -= 1)
-	;
-      for (k1 = strlen (N1) - 1; k1 > 0 && isdigit (N1[k1]); k1 -= 1)
-	;
-      if ((N0[k0] == '_' || N0[k0] == '$') && N0[k0 + 1] != '\000'
-	  && (N1[k1] == '_' || N1[k1] == '$') && N1[k1 + 1] != '\000')
-	{
-	  int n0, n1;
-
-	  n0 = k0;
-	  while (N0[n0] == '_' && n0 > 0 && N0[n0 - 1] == '_')
-	    n0 -= 1;
-	  n1 = k1;
-	  while (N1[n1] == '_' && n1 > 0 && N1[n1 - 1] == '_')
-	    n1 -= 1;
-	  if (n0 == n1 && strncmp (N0, N1, n0) == 0)
-	    return (atoi (N0 + k0 + 1) < atoi (N1 + k1 + 1));
-	}
-      return (strcmp (N0, N1) < 0);
-    }
-}
-
-/* Sort SYMS[0..NSYMS-1] to put the choices in a canonical order by the
-   encoded names.  */
 
 static void
-sort_choices (struct block_symbol syms[], int nsyms)
+sort_choices (std::vector<struct block_symbol> &syms)
 {
-  int i;
+  std::sort (syms.begin (), syms.end (),
+	     [] (const block_symbol &a, const block_symbol &b)
+	     {
+	       if (!a.symbol->is_objfile_owned ())
+		 return true;
+	       if (!b.symbol->is_objfile_owned ())
+		 return true;
 
-  for (i = 1; i < nsyms; i += 1)
+	       const char *fna = a.symbol->symtab ()->filename;
-    {
+	       const char *fnb = b.symbol->symtab ()->filename;
-      struct block_symbol sym = syms[i];
-      int j;
 
-      for (j = i - 1; j >= 0; j -= 1)
+	       /* First sort by basename.  This is done because,
-	{
+		  depending on how GNAT was invoked, different sources
-	  if (encoded_ordered_before (syms[j].symbol->linkage_name (),
+		  might have relative or absolute paths, but we'd like
-				      sym.symbol->linkage_name ()))
+		  similar ones to appear together.  */
-	    break;
+	       int cmp = strcmp (lbasename (fna), lbasename (fnb));
-	  syms[j + 1] = syms[j];
+	       if (cmp != 0)
-	}
+		 return cmp < 0;
-      syms[j + 1] = sym;
+
-    }
+	       /* The basenames are the same, so group identical paths
+		  together.  */
+	       cmp = strcmp (fna, fnb);
+	       if (cmp != 0)
+		 return cmp < 0;
+
+	       if (a.symbol->line () < b.symbol->line ())
+		 return true;
+	       if (a.symbol->line () > b.symbol->line ())
+		 return false;
+
+	       return strcmp (a.symbol->natural_name (),
+			      b.symbol->natural_name ()) < 0;
+	     });
 }
 
 /* Whether GDB should display formals and return types for functions in the
@@ -3619,7 +3597,7 @@ get_selections (int *choices, int n_choices, int max_results,
   return n_chosen;
 }
 
-/* Given a list of NSYMS symbols in SYMS, select up to MAX_RESULTS>0
+/* Given a list symbols in SYMS, select up to MAX_RESULTS>0
    by asking the user (if necessary), returning the number selected,
    and setting the first elements of SYMS items.  Error if no symbols
    selected.  */
@@ -3628,18 +3606,16 @@ get_selections (int *choices, int n_choices, int max_results,
    to be re-integrated one of these days.  */
 
 static int
-user_select_syms (struct block_symbol *syms, int nsyms, int max_results)
+user_select_syms (std::vector<struct block_symbol> &syms, int max_results)
 {
   int i;
-  int *chosen = XALLOCAVEC (int , nsyms);
-  int n_chosen;
   int first_choice = (max_results == 1) ? 1 : 2;
   const char *select_mode = multiple_symbols_select_mode ();
 
   if (max_results < 1)
     error (_("Request to select 0 symbols!"));
-  if (nsyms <= 1)
+  if (syms.size () <= 1)
-    return nsyms;
+    return syms.size ();
 
   if (select_mode == multiple_symbols_cancel)
     error (_("\
@@ -3650,15 +3626,15 @@ See set/show multiple-symbol."));
      Only do that if more than one symbol can be selected, of course.
      Otherwise, display the menu as usual.  */
   if (select_mode == multiple_symbols_all && max_results > 1)
-    return nsyms;
+    return syms.size ();
 
   gdb_printf (_("[0] cancel\n"));
   if (max_results > 1)
     gdb_printf (_("[1] all\n"));
 
-  sort_choices (syms, nsyms);
+  sort_choices (syms);
 
-  for (i = 0; i < nsyms; i += 1)
+  for (i = 0; i < syms.size (); i += 1)
     {
       if (syms[i].symbol == NULL)
 	continue;
@@ -3733,8 +3709,10 @@ See set/show multiple-symbol."));
 	}
     }
 
-  n_chosen = get_selections (chosen, nsyms, max_results, max_results > 1,
+  int *chosen = XALLOCAVEC (int , syms.size ());
-			     "overload-choice");
+  int n_chosen = get_selections (chosen, syms.size (),
+				 max_results, max_results > 1,
+				 "overload-choice");
 
   for (i = 0; i < n_chosen; i += 1)
     syms[i] = syms[chosen[i]];
@@ -3921,7 +3899,7 @@ ada_resolve_variable (struct symbol *sym, const struct block *block,
   else
     {
       gdb_printf (_("Multiple matches for %s\n"), sym->print_name ());
-      user_select_syms (candidates.data (), candidates.size (), 1);
+      user_select_syms (candidates, 1);
       i = 0;
     }
 
@@ -4127,7 +4105,8 @@ ada_resolve_function (std::vector<struct block_symbol> &syms,
   else if (m > 1 && !parse_completion)
     {
       gdb_printf (_("Multiple matches for %s\n"), name);
-      user_select_syms (syms.data (), m, 1);
+      syms.resize (m);
+      user_select_syms (syms, 1);
       return 0;
     }
   return 0;

gdb/testsuite/gdb.ada/overload_menu_crash.exp

@@ -32,8 +32,8 @@ runto "main.adb:$bp_location"
 
 set menu [multi_line "Multiple matches for regtest" \
 	      "\\\[0\\\] cancel" \
-	      "\\\[1\\\] pck.inner.regtest at .*pck.ads:\[0-9\]+" \
+	      "\\\[1\\\] pck.regtest .* return boolean at .*pck.adb:\[0-9\]+" \
-	      "\\\[2\\\] pck.regtest .* return boolean at .*pck.adb:\[0-9\]+" \
+	      "\\\[2\\\] pck.inner.regtest at .*pck.ads:\[0-9\]+" \
 	      "> $"]
 gdb_test_multiple "whatis &regtest" "menu does not crash" {
     -re "$menu" {
@@ -43,4 +43,4 @@ gdb_test_multiple "whatis &regtest" "menu does not crash" {
 	fail "$gdb_test_name"
     }
 }
-gdb_test "1" "type = access boolean" "choose from menu"
+gdb_test "2" "type = access boolean" "choose from menu"

gdb/testsuite/gdb.ada/sym_print_name.exp

@@ -29,10 +29,10 @@ set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.adb]
 runto "foo.adb:$bp_location"
 
 set menu [multi_line "Multiple matches for integervar" \
-                     "\\\[0\\\] cancel" \
+	      "\\\[0\\\] cancel" \
-                     "\\\[1\\\] pck\\.first\\.integervar.*" \
+	      "\\\[1\\\] pck\\.first\\.integervar.*" \
-                     "\\\[2\\\] pck\\.second\\.integervar.*" \
+	      "\\\[2\\\] pck\\.second\\.integervar.*" \
-                     "> $" ]
+	      "> $"]
 
 set test_name "multiple matches for symbol integervar"
 gdb_test_multiple "print integervar" "$test_name" \