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

[Joel Brobecker <brobecker@adacore.com>]

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

> To summarize again, this patch introduces a new set/show command set:
> 
>         (gdb) set multiple-symbols (ask|all|cancel)
>         (gdb) show multiple-symbols

Ping?

> 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