[RFA] new set/show multiple-symbols command (take 4)

[Joel Brobecker <brobecker@adacore.com>]

[-- Attachment #1: Type: text/plain, Size: 2769 bytes --]

Following a suggestion from Daniel, I changed the name of the new
setting to something more explicit and yet shorter.  This is the only
different between this patch and take 3.

To summarize again, this patch introduces a new set/show command set:

        (gdb) set multiple-symbols (ask|all|cancel)
        (gdb) show multiple-symbols

For everyone's convenience, I'm repeating here the context for this
new command, to avoid so that a cross-reference is not necessary.

The idea is that some expressions are ambiguous, and that the debugger
normally has several options: Select all matching symbols, ask the user
to select one or more of them, or error out.  This new option allows
the user to configure how the debugger should behave in that situation:

  - "all" (default): auto-select all matching symbols.
  - "ask": Auto-selection is off, so display a menu with all possible
           choices and ask the user to choose one or more of them.
  - "cancel": Abort the command explaining why.

After the new setting was introduced, I modified ada-lang.c and
linespec.c to take it into account. That resulted in some behavior
changes which I think I desirable and consistent with what we have done
recently with multiple-location breakpoints. In order to preserve
the current testcases, I simply forced multiple-symbols to "ask"
to reproduce the previous behavior.

I also added some testing of the cases when multiple-symbols is set to
"cancel" and "all". I still haven't written an Ada testcase but that's
very easy. I'll do that next, as well as sending a documentation update
if this patch is approved (including a NEWS entry).

2008-02-03  Joel Brobecker  <brobecker@adacore.com>

        * symtab.c (multiple_symbols_ask, multiple_symbols_all)
        (multiple_symbols_cancel): New constants.
        (multiple_symbols_modes, multiple_symbols_mode): New static globals.
        (multiple_symbols_select_mode): New function.
        (_initialize_symtab): Add new set/show multiple-symbols commands.
        * symtab.h (multiple_symbols_ask, multiple_symbols_all)
        (multiple_symbols_cancel, multiple_symbols_select_mode): Declare.
        * ada-lang.c (user_select_syms): Add handling of new multiple-symbols
        setting.
        * linespec.c (decode_line_2): Likewise.

2008-02-03  Joel Brobecker  <brobecker@adacore.com>

        * gdb.cp/ovldbreak.cc: Add missing bodies for methods foo::foofunc.
        * gdb.cp/ovldbreak.exp: Set multiple-symbols to "ask".
        Add a couple of tests that verify the behavior when the new setting
        is set to "cancel" and "all".
        * gdb.cp/method2.exp, gdb.cp/templates.exp: Set multiple-symbols to
        "ask" before we start the testing.

Tested on x86-linux.
OK to apply?

Thanks,
-- 
Joel

[-- Attachment #2: ms.diff --]
[-- Type: text/plain, Size: 6508 bytes --]

Index: symtab.c
===================================================================
--- symtab.c	(revision 155)
+++ symtab.c	(revision 156)
@@ -124,6 +124,30 @@ void _initialize_symtab (void);
 
 /* */
 
+/* Allow the user to configure the debugger behavior with respect
+   to multiple-choice menus when more than one symbol matches during
+   a symbol lookup.  */
+
+const char const multiple_symbols_ask[] = "ask";
+const char const multiple_symbols_all[] = "all";
+const char const multiple_symbols_cancel[] = "cancel";
+static const char *multiple_symbols_modes[] =
+{
+  multiple_symbols_ask,
+  multiple_symbols_all,
+  multiple_symbols_cancel,
+  NULL
+};
+static const char *multiple_symbols_mode = multiple_symbols_all;
+
+/* Read-only accessor to AUTO_SELECT_MODE.  */
+
+const char *
+multiple_symbols_select_mode (void)
+{
+  return multiple_symbols_mode;
+}
+
 /* The single non-language-specific builtin type */
 struct type *builtin_type_error;
 
@@ -4417,6 +4441,15 @@ All global and static variable names, or
 All global and static variable names, or those matching REGEXP."));
     }
 
+  add_setshow_enum_cmd ("multiple-symbols", no_class,
+                        multiple_symbols_modes, &multiple_symbols_mode,
+                        _("\
+Set the debugger behavior when more than one symbol are possible matches\n\
+in an expression."), _("\
+Show how the debugger handles ambiguities in expressions."), _("\
+Valid values are \"ask\", \"all\", \"cancel\", and the default is \"all\"."),
+                        NULL, NULL, &setlist, &showlist);
+
   /* Initialize the one built-in type that isn't language dependent... */
   builtin_type_error = init_type (TYPE_CODE_ERROR, 0, 0,
 				  "<unknown type>", (struct objfile *) NULL);
Index: symtab.h
===================================================================
--- symtab.h	(revision 155)
+++ symtab.h	(revision 156)
@@ -1005,6 +1005,12 @@ extern int asm_demangle;
 
 /* symtab.c lookup functions */
 
+extern const char const multiple_symbols_ask[];
+extern const char const multiple_symbols_all[];
+extern const char const multiple_symbols_cancel[];
+
+const char *multiple_symbols_select_mode (void);
+
 /* lookup a symbol table by source file name */
 
 extern struct symtab *lookup_symtab (const char *);
Index: ada-lang.c
===================================================================
--- ada-lang.c	(revision 155)
+++ ada-lang.c	(revision 156)
@@ -3362,12 +3362,24 @@ user_select_syms (struct ada_symbol_info
   int *chosen = (int *) alloca (sizeof (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)
     return nsyms;
 
+  if (select_mode == multiple_symbols_cancel)
+    error (_("\
+canceled because the command is ambiguous\n\
+See set/show multiple-symbol."));
+  
+  /* If select_mode is "all", then return all possible symbols.
+     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;
+
   printf_unfiltered (_("[0] cancel\n"));
   if (max_results > 1)
     printf_unfiltered (_("[1] all\n"));
Index: linespec.c
===================================================================
--- linespec.c	(revision 155)
+++ linespec.c	(revision 156)
@@ -492,7 +492,13 @@ decode_line_2 (struct symbol *sym_arr[],
   char *symname;
   struct cleanup *old_chain;
   char **canonical_arr = (char **) NULL;
+  const char *select_mode = multiple_symbols_select_mode ();
 
+  if (select_mode == multiple_symbols_cancel)
+    error (_("\
+canceled because the command is ambiguous\n\
+See set/show multiple-symbol."));
+  
   values.sals = (struct symtab_and_line *)
     alloca (nelts * sizeof (struct symtab_and_line));
   return_values.sals = (struct symtab_and_line *)
@@ -508,38 +514,52 @@ decode_line_2 (struct symbol *sym_arr[],
     }
 
   i = 0;
-  printf_unfiltered (_("[0] cancel\n[1] all\n"));
   while (i < nelts)
     {
       init_sal (&return_values.sals[i]);	/* Initialize to zeroes.  */
       init_sal (&values.sals[i]);
       if (sym_arr[i] && SYMBOL_CLASS (sym_arr[i]) == LOC_BLOCK)
-	{
-	  values.sals[i] = find_function_start_sal (sym_arr[i], funfirstline);
-	  if (values.sals[i].symtab)
-	    printf_unfiltered ("[%d] %s at %s:%d\n",
-			       (i + 2),
-			       SYMBOL_PRINT_NAME (sym_arr[i]),
-			       values.sals[i].symtab->filename,
-			       values.sals[i].line);
-	  else
-	    printf_unfiltered (_("[%d] %s at ?FILE:%d [No symtab? Probably broken debug info...]\n"),
-			       (i + 2),
-			       SYMBOL_PRINT_NAME (sym_arr[i]),
-			       values.sals[i].line);
-
-	}
-      else
-	printf_unfiltered (_("?HERE\n"));
+	values.sals[i] = find_function_start_sal (sym_arr[i], funfirstline);
       i++;
     }
 
-  prompt = getenv ("PS2");
-  if (prompt == NULL)
+  /* If select_mode is "all", then do not print the multiple-choice
+     menu and act as if the user had chosen choice "1" (all).  */
+  if (select_mode == multiple_symbols_all)
+    args = "1";
+  else
     {
-      prompt = "> ";
+      i = 0;
+      printf_unfiltered (_("[0] cancel\n[1] all\n"));
+      while (i < nelts)
+        {
+          if (sym_arr[i] && SYMBOL_CLASS (sym_arr[i]) == LOC_BLOCK)
+            {
+              if (values.sals[i].symtab)
+                printf_unfiltered ("[%d] %s at %s:%d\n",
+                                   (i + 2),
+                                   SYMBOL_PRINT_NAME (sym_arr[i]),
+                                   values.sals[i].symtab->filename,
+                                   values.sals[i].line);
+              else
+                printf_unfiltered (_("[%d] %s at ?FILE:%d [No symtab? Probably broken debug info...]\n"),
+                                   (i + 2),
+                                   SYMBOL_PRINT_NAME (sym_arr[i]),
+                                   values.sals[i].line);
+
+            }
+          else
+            printf_unfiltered (_("?HERE\n"));
+          i++;
+        }
+
+      prompt = getenv ("PS2");
+      if (prompt == NULL)
+        {
+          prompt = "> ";
+        }
+      args = command_line_input (prompt, 0, "overload-choice");
     }
-  args = command_line_input (prompt, 0, "overload-choice");
 
   if (args == 0 || *args == 0)
     error_no_arg (_("one or more choice numbers"));

[-- Attachment #3: ms-tc.diff --]
[-- Type: text/plain, Size: 3483 bytes --]

Index: gdb.cp/ovldbreak.exp
===================================================================
--- gdb.cp/ovldbreak.exp	(revision 154)
+++ gdb.cp/ovldbreak.exp	(revision 155)
@@ -131,7 +131,9 @@ proc set_bp_overloaded {name expectedmen
 
 set menu_overload1arg "\\\[0\\\] cancel\r\n\\\[1\\\] all\r\n\\\[2\\\] foo::overload1arg\\(double\\) at.*$srcfile:121\r\n\\\[3\\\] foo::overload1arg\\(float\\) at.*$srcfile:120\r\n\\\[4\\\] foo::overload1arg\\(unsigned long\\) at.*$srcfile:119\r\n\\\[5\\\] foo::overload1arg\\(long\\) at.*$srcfile:118\r\n\\\[6\\\] foo::overload1arg\\((unsigned int|unsigned)\\) at.*$srcfile:117\r\n\\\[7\\\] foo::overload1arg\\(int\\) at.*$srcfile:116\r\n\\\[8\\\] foo::overload1arg\\(unsigned short\\) at.*$srcfile:115\r\n\\\[9\\\] foo::overload1arg\\(short\\) at.*$srcfile:114\r\n\\\[10\\\] foo::overload1arg\\(unsigned char\\) at.*$srcfile:113\r\n\\\[11\\\] foo::overload1arg\\(signed char\\) at.*$srcfile:112\r\n\\\[12\\\] foo::overload1arg\\(char\\) at.*$srcfile:111\r\n\\\[13\\\] foo::overload1arg\\((void|)\\) at.*$srcfile:110\r\n> $"
 
-
+# Set multiple-symbols to "ask", to allow us to test the use
+# of the multiple-choice menu when breaking on an overloaded method.
+gdb_test "set multiple-symbols ask" ""
 
 # Set breakpoints on foo::overload1arg, one by one.
 
@@ -350,7 +352,17 @@ continue_to_bp_overloaded 0 16 "unsigned
 continue_to_bp_overloaded 0 15 "float" "arg=100"
 continue_to_bp_overloaded 1 14 "double" "arg=200"
 
-
+# Test breaking on an overloaded function when multiple-symbols
+# is set to "cancel"
+gdb_test "set multiple-symbols cancel" ""
+gdb_test "break foo::foofunc" \
+         "canceled.*"
+
+# Test breaking on an overloaded function when multiple-symbols
+# is set to "all"
+gdb_test "set multiple-symbols all" ""
+gdb_test "break foo::foofunc" \
+         "Breakpoint \[0-9\]+ at ${hex}: file .*ovldbreak\\.cc, line \[0-9\]+\\.\r\nBreakpoint \[0-9\]+ at ${hex}: file .*ovldbreak\\.cc, line \[0-9\]+\\.\r\nwarning: Multiple breakpoints were set\\.\r\nUse the \"delete\" command to delete unwanted breakpoints\\."
 
 # That's all, folks.
 
Index: gdb.cp/ovldbreak.cc
===================================================================
--- gdb.cp/ovldbreak.cc	(revision 154)
+++ gdb.cp/ovldbreak.cc	(revision 155)
@@ -174,4 +174,11 @@ int foo::overloadargs (int a1, int a2, i
                           a10 = a11 = 0; return 11;}
 
 
+void foo::foofunc (int a)
+{
+}
+
+void foo::foofunc (int b, signed char *c)
+{
+}
 
Index: gdb.cp/method2.exp
===================================================================
--- gdb.cp/method2.exp	(revision 154)
+++ gdb.cp/method2.exp	(revision 155)
@@ -63,6 +63,11 @@ proc test_break { lang } {
     }
 }
 
+# We want in this test to double-check the contents of the multiple-choice
+# menu that's printed when a breakpoint location is ambiguous.  So we need
+# to set multiple-symbols to "ask" first.
+gdb_test "set multiple-symbols ask" ""
+
 test_break "c"
 test_break "c++"
 
Index: gdb.cp/templates.exp
===================================================================
--- gdb.cp/templates.exp	(revision 154)
+++ gdb.cp/templates.exp	(revision 155)
@@ -210,6 +210,10 @@ proc do_tests {} {
     gdb_reinitialize_dir $srcdir/$subdir
     gdb_load $binfile
 
+    # Change multiple-symbols to "ask" in order to get the multiple-choice
+    # menu when breaking on overloaded methods.
+    gdb_test "set multiple-symbols ask" ""
+
     runto_main
 
     test_ptype_of_templates