[RFA] new set/show multiple-choice-auto-select command (take 3)

[Joel Brobecker <brobecker@adacore.com>]

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

Hello,

This is my third take on introducing this new set/show command.
The change since the last time is that the default had been changed
to "all". This is not what used to be the default behavior in Ada,
but it better matches the direction taken for C/C++, so it's all
consistent across languages (and will make it easier later, when
we manage to merge the ada code with the core code).

So, to summarize, this patch introduces a new set/show command:

        (gdb) set multiple-choice-auto-select (off|all|cancel)
        (gdb) show multiple-choice-auto-select

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.
  - "off": 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 we have done
recently with multiple-location breakpoints. In order to preserve
the current testcases, I simply forced multiple-choice-auto-select
to "off" to reproduce the previous behavior.

I also added some testing of the cases when multiple-choice-auto-select
is set to "cancel" and "all". I still haven't written an Ada testcase
but that's very easy. I'll do that next, together with the documentation
if this patch is approved.

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

        * symtab.c (auto_select_off, auto_select_all, auto_select_cancel):
        New constants.
        (auto_select_modes, auto_select_mode): New static globals.
        (multiple_choice_auto_select_mode): New function.
        (_initialize_symtab): Add new multiple-choice-auto-select command.
        * symtab.h (auto_select_off, auto_select_all, auto_select_cancel)
        (multiple_choice_auto_select_mode): Add declarations.
        * ada-lang.c (user_select_syms): Add handling of new
        multiple-choice-auto-select setting.
        * linespec.c (decode_line_2): Likewise.

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

        * gdb.cp/ovldbreak.cc: Add missing bodies for methods foo::foofunc.
        * gdb.cp/ovldbreak.exp: Set multiple-choice-auto-select to "off"
        as this is the value that this test was assuming when it was
        written, before this setting got introduced.
        Add a couple of tests that verify the behavior when the new setting
        is "cancel" and "all".
        * gdb.cp/method2.exp, gdb.cp/templates.exp: Set
        multiple-choice- auto-select to "off" before we start the testing.

Tested on x86-linux.
OK to apply?

Thanks,
-- 
Joel

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

Index: symtab.c
===================================================================
--- symtab.c	(revision 141)
+++ symtab.c	(revision 142)
@@ -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 auto_select_off[] = "off";
+const char const auto_select_all[] = "all";
+const char const auto_select_cancel[] = "cancel";
+static const char *auto_select_modes[] =
+{
+  auto_select_off,
+  auto_select_all,
+  auto_select_cancel,
+  NULL
+};
+static const char *auto_select_mode = auto_select_all;
+
+/* Read-only accessor to AUTO_SELECT_MODE.  */
+
+const char *
+multiple_choice_auto_select_mode (void)
+{
+  return auto_select_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-choice-auto-select", no_class,
+                        auto_select_modes, &auto_select_mode,
+                        _("\
+Set the debugger behavior when part of a command is ambiguous and\n\
+a multiple-choice menu would normally be printed."), _("\
+Show how the debugger handles ambiguities in commands."), _("\
+Valid values are \"off\", \"all\", \"cancel\", and the default is \"off\"."),
+                        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 141)
+++ symtab.h	(revision 142)
@@ -1005,6 +1005,12 @@ extern int asm_demangle;
 
 /* symtab.c lookup functions */
 
+extern const char const auto_select_off[];
+extern const char const auto_select_all[];
+extern const char const auto_select_cancel[];
+
+const char *multiple_choice_auto_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 141)
+++ ada-lang.c	(revision 142)
@@ -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 *auto_select = multiple_choice_auto_select_mode ();
 
   if (max_results < 1)
     error (_("Request to select 0 symbols!"));
   if (nsyms <= 1)
     return nsyms;
 
+  if (auto_select == auto_select_cancel)
+    error (_("\
+canceled because the command is ambiguous and the multiple-choice menu\n\
+has been deactivated.  See set/show multiple-choice-auto-select."));
+  
+  /* If auto_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 (auto_select == auto_select_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 141)
+++ linespec.c	(revision 142)
@@ -492,7 +492,13 @@ decode_line_2 (struct symbol *sym_arr[],
   char *symname;
   struct cleanup *old_chain;
   char **canonical_arr = (char **) NULL;
+  const char *auto_select = multiple_choice_auto_select_mode ();
 
+  if (auto_select == auto_select_cancel)
+    error (_("\
+canceled because the command is ambiguous and the multiple-choice menu\n\
+has been deactivated.  See set/show multiple-choice-auto-select."));
+  
   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 auto_select_mode is "all", then do not print the multiple-choice
+     menu and act as if the user had chosen choice "1" (all).  */
+  if (auto_select == auto_select_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: mcas-tc.diff --]
[-- Type: text/plain, Size: 3809 bytes --]

Index: gdb.cp/ovldbreak.exp
===================================================================
--- gdb.cp/ovldbreak.exp	(revision 139)
+++ gdb.cp/ovldbreak.exp	(revision 140)
@@ -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-choice-auto-select to "off", to allow us to test the use
+# of the multiple-choice menu when breaking on an overloaded method.
+gdb_test "set multiple-choice-auto-select off" ""
 
 # Set breakpoints on foo::overload1arg, one by one.
 
@@ -350,7 +352,18 @@ 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-choice-auto-select
+# is set to "cancel"
+gdb_test "set multiple-choice-auto-select cancel" ""
+gdb_test "break foo::foofunc" \
+         "canceled.*"
+
+# Test breaking on an overloaded function when multiple-choice-auto-select
+# is set to "all"
+gdb_test "set multiple-choice-auto-select 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\\."
+         #"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\\.Use the \"delete\" command to delete unwanted breakpoints\\."
 
 # That's all, folks.
 
Index: gdb.cp/ovldbreak.cc
===================================================================
--- gdb.cp/ovldbreak.cc	(revision 139)
+++ gdb.cp/ovldbreak.cc	(revision 140)
@@ -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 139)
+++ gdb.cp/method2.exp	(revision 140)
@@ -63,6 +63,10 @@ proc test_break { lang } {
     }
 }
 
+# We want in this test to double-check the contents of the multiple-choice
+# menu, so we have to turn the multiple-choice-auto-select feature off first.
+gdb_test "set multiple-choice-auto-select off" ""
+
 test_break "c"
 test_break "c++"
 
Index: gdb.cp/templates.exp
===================================================================
--- gdb.cp/templates.exp	(revision 139)
+++ gdb.cp/templates.exp	(revision 140)
@@ -210,6 +210,10 @@ proc do_tests {} {
     gdb_reinitialize_dir $srcdir/$subdir
     gdb_load $binfile
 
+    # Change the multiple-choice-auto-select value to "off" in order to
+    # get the multiple-choice menu when breaking on overloaded methods.
+    gdb_test "set multiple-choice-auto-select off" ""
+
     runto_main
 
     test_ptype_of_templates