[PATCH 0/5] Change some index-cache commands

[PATCH 0/5] Change some index-cache commands

[Simon Marchi]

Reviewing Aaron's debuginfod series (which adds a command to
enable/disable the use of debuginfod) made me reconsider how I
designed the index-cache commands.  The details about that are in the
last two patches.  The first three patches are preparatory cleanups or
improvements.

Simon Marchi (5):
  gdb: pass/return setting setter/getter scalar values by value
  gdb: remove unnecessary cmd_list_element::aliases nullptr checks
  gdb: remove command_class enum class_deprecated
  gdb: add "info index-cache stats", deprecate "show index-cache stats"
  gdb: introduce "set index-cache enabled", deprecate "set index-cache
    on/off"

 gdb/cli/cli-decode.c                         | 112 ++++++++++-------
 gdb/cli/cli-setshow.c                        |   6 +-
 gdb/command.h                                |  85 +++++++------
 gdb/doc/gdb.texinfo                          |  10 +-
 gdb/dwarf2/index-cache.c                     | 126 ++++++++++---------
 gdb/remote.c                                 |   5 +-
 gdb/testsuite/gdb.base/index-cache.exp       |  34 +++--
 gdb/testsuite/gdb.base/maint.exp             |   8 +-
 gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp |   6 +-
 9 files changed, 225 insertions(+), 167 deletions(-)

-- 
2.26.2

[PATCH 1/5] gdb: pass/return setting setter/getter scalar values by value

[Simon Marchi]

From: Simon Marchi <simon.marchi@polymtl.ca>

The getter and setter in struct setting always receive and return values
by const reference.  This is not necessary for scalar values (like bool
and int), but more importantly it makes it a bit annoying to write a
getter, you have to use a scratch static variable or something similar
that you can refer to:

  const bool &
  my_getter ()
  {
    static bool value;
    value = function_returning_bool ();
    return value;
  }

Change the getter and setter function signatures to receive and return
value by value instead of by reference, when the underlying data type is
scalar.  This means that string-based settings will still use
references, but all others will be by value.  The getter above would
then be re-written as:

  bool
  my_getter ()
  {
    return function_returning_bool ();
  }

This is useful for a patch later in this series that defines a boolean
setting with a getter and a setter.

Change-Id: Ieca3a2419fcdb75a6f75948b2c920b548a0af0fd
---
 gdb/cli/cli-decode.c | 52 +++++++++++++--------------
 gdb/command.h        | 84 ++++++++++++++++++++++++++------------------
 gdb/remote.c         |  5 ++-
 3 files changed, 80 insertions(+), 61 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 27a80192c485..396aefb4b011 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -572,8 +572,8 @@ add_setshow_cmd_full (const char *name,
 		      var_types var_type, T *var,
 		      const char *set_doc, const char *show_doc,
 		      const char *help_doc,
-		      setting_setter_ftype<T> set_setting_func,
-		      setting_getter_ftype<T> get_setting_func,
+		      typename the_types<T>::set set_setting_func,
+		      typename the_types<T>::get get_setting_func,
 		      cmd_func_ftype *set_func,
 		      show_value_ftype *show_func,
 		      struct cmd_list_element **set_list,
@@ -628,8 +628,8 @@ set_show_commands
 add_setshow_enum_cmd (const char *name, command_class theclass,
 		      const char *const *enumlist, const char *set_doc,
 		      const char *show_doc, const char *help_doc,
-		      setting_setter_ftype<const char *> set_func,
-		      setting_getter_ftype<const char *> get_func,
+		      typename the_types<const char *>::set set_func,
+		      typename the_types<const char *>::get get_func,
 		      show_value_ftype *show_func,
 		      cmd_list_element **set_list,
 		      cmd_list_element **show_list)
@@ -682,8 +682,8 @@ set_show_commands
 add_setshow_auto_boolean_cmd (const char *name, command_class theclass,
 			      const char *set_doc, const char *show_doc,
 			      const char *help_doc,
-			      setting_setter_ftype<enum auto_boolean> set_func,
-			      setting_getter_ftype<enum auto_boolean> get_func,
+			      typename the_types<enum auto_boolean>::set set_func,
+			      typename the_types<enum auto_boolean>::get get_func,
 			      show_value_ftype *show_func,
 			      cmd_list_element **set_list,
 			      cmd_list_element **show_list)
@@ -737,8 +737,8 @@ set_show_commands
 add_setshow_boolean_cmd (const char *name, command_class theclass,
 			 const char *set_doc, const char *show_doc,
 			 const char *help_doc,
-			 setting_setter_ftype<bool> set_func,
-			 setting_getter_ftype<bool> get_func,
+			 typename the_types<bool>::set set_func,
+			 typename the_types<bool>::get get_func,
 			 show_value_ftype *show_func,
 			 cmd_list_element **set_list,
 			 cmd_list_element **show_list)
@@ -784,8 +784,8 @@ set_show_commands
 add_setshow_filename_cmd (const char *name, command_class theclass,
 			  const char *set_doc, const char *show_doc,
 			  const char *help_doc,
-			  setting_setter_ftype<std::string> set_func,
-			  setting_getter_ftype<std::string> get_func,
+			  typename the_types<std::string>::set set_func,
+			  typename the_types<std::string>::get get_func,
 			  show_value_ftype *show_func,
 			  cmd_list_element **set_list,
 			  cmd_list_element **show_list)
@@ -833,8 +833,8 @@ set_show_commands
 add_setshow_string_cmd (const char *name, command_class theclass,
 			const char *set_doc, const char *show_doc,
 			const char *help_doc,
-			setting_setter_ftype<std::string> set_func,
-			setting_getter_ftype<std::string> get_func,
+			typename the_types<std::string>::set set_func,
+			typename the_types<std::string>::get get_func,
 			show_value_ftype *show_func,
 			cmd_list_element **set_list,
 			cmd_list_element **show_list)
@@ -883,8 +883,8 @@ set_show_commands
 add_setshow_string_noescape_cmd (const char *name, command_class theclass,
 				 const char *set_doc, const char *show_doc,
 				 const char *help_doc,
-				 setting_setter_ftype<std::string> set_func,
-				 setting_getter_ftype<std::string> get_func,
+				 typename the_types<std::string>::set set_func,
+				 typename the_types<std::string>::get get_func,
 				 show_value_ftype *show_func,
 				 cmd_list_element **set_list,
 				 cmd_list_element **show_list)
@@ -933,8 +933,8 @@ set_show_commands
 add_setshow_optional_filename_cmd (const char *name, command_class theclass,
 				   const char *set_doc, const char *show_doc,
 				   const char *help_doc,
-				   setting_setter_ftype<std::string> set_func,
-				   setting_getter_ftype<std::string> get_func,
+				   typename the_types<std::string>::set set_func,
+				   typename the_types<std::string>::get get_func,
 				   show_value_ftype *show_func,
 				   cmd_list_element **set_list,
 				   cmd_list_element **show_list)
@@ -1001,8 +1001,8 @@ set_show_commands
 add_setshow_integer_cmd (const char *name, command_class theclass,
 			 const char *set_doc, const char *show_doc,
 			 const char *help_doc,
-			 setting_setter_ftype<int> set_func,
-			 setting_getter_ftype<int> get_func,
+			 typename the_types<int>::set set_func,
+			 typename the_types<int>::get get_func,
 			 show_value_ftype *show_func,
 			 cmd_list_element **set_list,
 			 cmd_list_element **show_list)
@@ -1050,8 +1050,8 @@ set_show_commands
 add_setshow_uinteger_cmd (const char *name, command_class theclass,
 			  const char *set_doc, const char *show_doc,
 			  const char *help_doc,
-			  setting_setter_ftype<unsigned int> set_func,
-			  setting_getter_ftype<unsigned int> get_func,
+			  typename the_types<unsigned int>::set set_func,
+			  typename the_types<unsigned int>::get get_func,
 			  show_value_ftype *show_func,
 			  cmd_list_element **set_list,
 			  cmd_list_element **show_list)
@@ -1095,8 +1095,8 @@ set_show_commands
 add_setshow_zinteger_cmd (const char *name, command_class theclass,
 			  const char *set_doc, const char *show_doc,
 			  const char *help_doc,
-			  setting_setter_ftype<int> set_func,
-			  setting_getter_ftype<int> get_func,
+			  typename the_types<int>::set set_func,
+			  typename the_types<int>::get get_func,
 			  show_value_ftype *show_func,
 			  cmd_list_element **set_list,
 			  cmd_list_element **show_list)
@@ -1137,8 +1137,8 @@ set_show_commands
 add_setshow_zuinteger_unlimited_cmd (const char *name, command_class theclass,
 				     const char *set_doc, const char *show_doc,
 				     const char *help_doc,
-				     setting_setter_ftype<int> set_func,
-				     setting_getter_ftype<int> get_func,
+				     typename the_types<int>::set set_func,
+				     typename the_types<int>::get get_func,
 				     show_value_ftype *show_func,
 				     cmd_list_element **set_list,
 				     cmd_list_element **show_list)
@@ -1182,8 +1182,8 @@ set_show_commands
 add_setshow_zuinteger_cmd (const char *name, command_class theclass,
 			   const char *set_doc, const char *show_doc,
 			   const char *help_doc,
-			   setting_setter_ftype<unsigned int> set_func,
-			   setting_getter_ftype<unsigned int> get_func,
+			   typename the_types<unsigned int>::set set_func,
+			   typename the_types<unsigned int>::get get_func,
 			   show_value_ftype *show_func,
 			   cmd_list_element **set_list,
 			   cmd_list_element **show_list)
diff --git a/gdb/command.h b/gdb/command.h
index 9afe70cf66a6..ef3c52949dca 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -181,15 +181,31 @@ inline bool var_type_uses<const char *> (var_types t)
   return t == var_enum;
 }
 
-/* Function signature for a callback used to get a value from a setting.  */
+template<bool is_scalar, typename T> struct the_types2;
 
 template<typename T>
-using setting_getter_ftype = const T &(*) ();
+struct the_types2<true, T>
+{
+  using type = T;
+  using set = void (*) (type);
+  using get = type (*) ();
+};
 
-/* Function signature for a callback used to set a value to a setting.  */
+template<typename T>
+struct the_types2<false, T>
+{
+  using type = const T &;
+  using set = void (*) (type);
+  using get = type (*) ();
+};
 
 template<typename T>
-using setting_setter_ftype = void (*) (const T &);
+struct the_types
+{
+  using type = typename the_types2<std::is_scalar<T>::value, T>::type;
+  using set = typename the_types2<std::is_scalar<T>::value, T>::set;
+  using get = typename the_types2<std::is_scalar<T>::value, T>::get;
+};
 
 /* Generic/type-erased function pointer.  */
 
@@ -225,8 +241,8 @@ struct setting
   template<typename T>
   static erased_args erase_args (var_types var_type,
 				 T *var,
-				 setting_setter_ftype<T> set_setting_func,
-				 setting_getter_ftype<T> get_setting_func)
+				 typename the_types<T>::set set_setting_func,
+				 typename the_types<T>::get get_setting_func)
   {
     gdb_assert (var_type_uses<T> (var_type));
   /* The getter and the setter must be both provided or both omitted.  */
@@ -260,8 +276,8 @@ struct setting
      Type T must match the var type VAR_TYPE (see VAR_TYPE_USES).  */
   template<typename T>
   setting (var_types var_type,
-	   setting_setter_ftype<T> setter,
-	   setting_getter_ftype<T> getter)
+	   typename the_types<T>::set setter,
+	   typename the_types<T>::get getter)
     : m_var_type (var_type)
   {
     gdb_assert (var_type_uses<T> (var_type));
@@ -285,14 +301,14 @@ struct setting
      The template parameter T is the type of the variable used to store the
      setting.  */
   template<typename T>
-  const T &get () const
+  typename the_types<T>::type get () const
   {
     gdb_assert (var_type_uses<T> (m_var_type));
 
     if (m_var == nullptr)
       {
 	gdb_assert (m_getter != nullptr);
-	auto getter = reinterpret_cast<setting_getter_ftype<T>> (m_getter);
+	auto getter = reinterpret_cast<typename the_types<T>::get> (m_getter);
 	return getter ();
       }
     else
@@ -322,7 +338,7 @@ struct setting
     if (m_var == nullptr)
       {
 	gdb_assert (m_setter != nullptr);
-	auto setter = reinterpret_cast<setting_setter_ftype<T>> (m_setter);
+	auto setter = reinterpret_cast<typename the_types<T>::set> (m_setter);
 	setter (v);
       }
     else
@@ -644,8 +660,8 @@ extern set_show_commands add_setshow_enum_cmd
 extern set_show_commands add_setshow_enum_cmd
   (const char *name, command_class theclass, const char *const *enumlist,
    const char *set_doc, const char *show_doc,
-   const char *help_doc, setting_setter_ftype<const char *> set_func,
-   setting_getter_ftype<const char *> get_func, show_value_ftype *show_func,
+   const char *help_doc, typename the_types<const char *>::set set_func,
+   typename the_types<const char *>::get get_func, show_value_ftype *show_func,
    cmd_list_element **set_list, cmd_list_element **show_list);
 
 extern set_show_commands add_setshow_auto_boolean_cmd
@@ -657,8 +673,8 @@ extern set_show_commands add_setshow_auto_boolean_cmd
 extern set_show_commands add_setshow_auto_boolean_cmd
   (const char *name, command_class theclass, const char *set_doc,
    const char *show_doc, const char *help_doc,
-   setting_setter_ftype<enum auto_boolean> set_func,
-   setting_getter_ftype<enum auto_boolean> get_func,
+   typename the_types<enum auto_boolean>::set set_func,
+   typename the_types<enum auto_boolean>::get get_func,
    show_value_ftype *show_func, cmd_list_element **set_list,
    cmd_list_element **show_list);
 
@@ -671,8 +687,8 @@ extern set_show_commands add_setshow_boolean_cmd
 extern set_show_commands add_setshow_boolean_cmd
   (const char *name, command_class theclass, const char *set_doc,
    const char *show_doc, const char *help_doc,
-   setting_setter_ftype<bool> set_func,
-   setting_getter_ftype<bool> get_func, show_value_ftype *show_func,
+   typename the_types<bool>::set set_func,
+   typename the_types<bool>::get get_func, show_value_ftype *show_func,
    cmd_list_element **set_list, cmd_list_element **show_list);
 
 extern set_show_commands add_setshow_filename_cmd
@@ -684,8 +700,8 @@ extern set_show_commands add_setshow_filename_cmd
 extern set_show_commands add_setshow_filename_cmd
   (const char *name, command_class theclass, const char *set_doc,
    const char *show_doc, const char *help_doc,
-   setting_setter_ftype<std::string> set_func,
-   setting_getter_ftype<std::string> get_func, show_value_ftype *show_func,
+   typename the_types<std::string>::set set_func,
+   typename the_types<std::string>::get get_func, show_value_ftype *show_func,
    cmd_list_element **set_list, cmd_list_element **show_list);
 
 extern set_show_commands add_setshow_string_cmd
@@ -697,8 +713,8 @@ extern set_show_commands add_setshow_string_cmd
 extern set_show_commands add_setshow_string_cmd
   (const char *name, command_class theclass, const char *set_doc,
    const char *show_doc, const char *help_doc,
-   setting_setter_ftype<std::string> set_func,
-   setting_getter_ftype<std::string> get_func,
+   typename the_types<std::string>::set set_func,
+   typename the_types<std::string>::get get_func,
    show_value_ftype *show_func, cmd_list_element **set_list,
    cmd_list_element **show_list);
 
@@ -711,8 +727,8 @@ extern set_show_commands add_setshow_string_noescape_cmd
 extern set_show_commands add_setshow_string_noescape_cmd
   (const char *name, command_class theclass, const char *set_doc,
    const char *show_doc, const char *help_doc,
-   setting_setter_ftype<std::string> set_func,
-   setting_getter_ftype<std::string> get_func, show_value_ftype *show_func,
+   typename the_types<std::string>::set set_func,
+   typename the_types<std::string>::get get_func, show_value_ftype *show_func,
    cmd_list_element **set_list, cmd_list_element **show_list);
 
 extern set_show_commands add_setshow_optional_filename_cmd
@@ -724,8 +740,8 @@ extern set_show_commands add_setshow_optional_filename_cmd
 extern set_show_commands add_setshow_optional_filename_cmd
   (const char *name, command_class theclass, const char *set_doc,
    const char *show_doc, const char *help_doc,
-   setting_setter_ftype<std::string> set_func,
-   setting_getter_ftype<std::string> get_func,
+   typename the_types<std::string>::set set_func,
+   typename the_types<std::string>::get get_func,
    show_value_ftype *show_func, cmd_list_element **set_list,
    cmd_list_element **show_list);
 
@@ -738,8 +754,8 @@ extern set_show_commands add_setshow_integer_cmd
 extern set_show_commands add_setshow_integer_cmd
   (const char *name, command_class theclass, const char *set_doc,
    const char *show_doc, const char *help_doc,
-   setting_setter_ftype<int> set_func,
-   setting_getter_ftype<int> get_func, show_value_ftype *show_func,
+   typename the_types<int>::set set_func,
+   typename the_types<int>::get get_func, show_value_ftype *show_func,
    cmd_list_element **set_list, cmd_list_element **show_list);
 
 extern set_show_commands add_setshow_uinteger_cmd
@@ -751,8 +767,8 @@ extern set_show_commands add_setshow_uinteger_cmd
 extern set_show_commands add_setshow_uinteger_cmd
   (const char *name, command_class theclass, const char *set_doc,
    const char *show_doc, const char *help_doc,
-   setting_setter_ftype<unsigned int> set_func,
-   setting_getter_ftype<unsigned int> get_func, show_value_ftype *show_func,
+   typename the_types<unsigned int>::set set_func,
+   typename the_types<unsigned int>::get get_func, show_value_ftype *show_func,
    cmd_list_element **set_list, cmd_list_element **show_list);
 
 extern set_show_commands add_setshow_zinteger_cmd
@@ -764,8 +780,8 @@ extern set_show_commands add_setshow_zinteger_cmd
 extern set_show_commands add_setshow_zinteger_cmd
   (const char *name, command_class theclass, const char *set_doc,
    const char *show_doc, const char *help_doc,
-   setting_setter_ftype<int> set_func,
-   setting_getter_ftype<int> get_func, show_value_ftype *show_func,
+   typename the_types<int>::set set_func,
+   typename the_types<int>::get get_func, show_value_ftype *show_func,
    cmd_list_element **set_list, cmd_list_element **show_list);
 
 extern set_show_commands add_setshow_zuinteger_cmd
@@ -777,8 +793,8 @@ extern set_show_commands add_setshow_zuinteger_cmd
 extern set_show_commands add_setshow_zuinteger_cmd
   (const char *name, command_class theclass, const char *set_doc,
    const char *show_doc, const char *help_doc,
-   setting_setter_ftype<unsigned int> set_func,
-   setting_getter_ftype<unsigned int> get_func, show_value_ftype *show_func,
+   typename the_types<unsigned int>::set set_func,
+   typename the_types<unsigned int>::get get_func, show_value_ftype *show_func,
    cmd_list_element **set_list, cmd_list_element **show_list);
 
 extern set_show_commands add_setshow_zuinteger_unlimited_cmd
@@ -790,7 +806,7 @@ extern set_show_commands add_setshow_zuinteger_unlimited_cmd
 extern set_show_commands add_setshow_zuinteger_unlimited_cmd
   (const char *name, command_class theclass, const char *set_doc,
    const char *show_doc, const char *help_doc,
-   setting_setter_ftype<int> set_func, setting_getter_ftype<int> get_func,
+   typename the_types<int>::set set_func, typename the_types<int>::get get_func,
    show_value_ftype *show_func, cmd_list_element **set_list,
    cmd_list_element **show_list);
 
diff --git a/gdb/remote.c b/gdb/remote.c
index 0fb427535960..f17e571efa93 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1873,6 +1873,8 @@ struct packet_config
        have an associated command always have this set to auto.  */
     enum auto_boolean detect;
 
+    cmd_list_element *show_cmd;
+
     /* Does the target support this packet?  */
     enum packet_support support;
   };
@@ -1936,6 +1938,7 @@ add_packet_config_cmd (struct packet_config *config, const char *name,
 				    NULL,
 				    show_remote_protocol_packet_cmd,
 				    &remote_set_cmdlist, &remote_show_cmdlist);
+  config->show_cmd = cmds.show;
 
   /* The command code copies the documentation strings.  */
   xfree (set_doc);
@@ -2245,7 +2248,7 @@ show_remote_protocol_packet_cmd (struct ui_file *file, int from_tty,
        packet < &remote_protocol_packets[PACKET_MAX];
        packet++)
     {
-      if (&packet->detect == &c->var->get<enum auto_boolean> ())
+      if (c == packet->show_cmd)
 	{
 	  show_packet_config_cmd (packet);
 	  return;
-- 
2.26.2

[PATCH 2/5] gdb: remove unnecessary cmd_list_element::aliases nullptr checks

[Simon Marchi]

From: Simon Marchi <simon.marchi@polymtl.ca>

Remove two unnecessary nullptr checks.  If aliases is nullptr, then the
for loops will simply be skipped.

Change-Id: I9132063bb17798391f8d019af305383fa8e0229f
---
 gdb/cli/cli-decode.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 396aefb4b011..de90198dfa71 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -1351,16 +1351,11 @@ static void
 fput_aliases_definition_styled (struct cmd_list_element *cmd,
 				struct ui_file *stream)
 {
-  if (cmd->aliases != nullptr)
-    {
-      for (cmd_list_element *iter = cmd->aliases;
-	   iter;
-	   iter = iter->alias_chain)
-	{
-	  if (!iter->default_args.empty ())
-	    fput_alias_definition_styled (iter, stream);
-	}
-    }
+  for (cmd_list_element *iter = cmd->aliases;
+       iter != nullptr;
+       iter = iter->alias_chain)
+    if (!iter->default_args.empty ())
+      fput_alias_definition_styled (iter, stream);
 }
 
 
@@ -1377,15 +1372,14 @@ fput_command_names_styled (struct cmd_list_element *c,
 {
   if (always_fput_c_name ||  c->aliases != nullptr)
     fput_command_name_styled (c, stream);
-  if (c->aliases != nullptr)
+
+  for (cmd_list_element *iter = c->aliases; iter; iter = iter->alias_chain)
     {
-      for (cmd_list_element *iter = c->aliases; iter; iter = iter->alias_chain)
-	{
-	  fputs_filtered (", ", stream);
-	  wrap_here ("   ");
-	  fput_command_name_styled (iter, stream);
-	}
+      fputs_filtered (", ", stream);
+      wrap_here ("   ");
+      fput_command_name_styled (iter, stream);
     }
+
   if (always_fput_c_name ||  c->aliases != nullptr)
     fputs_filtered (postfix, stream);
 }
-- 
2.26.2

[PATCH 3/5] gdb: remove command_class enum class_deprecated

[Simon Marchi]

From: Simon Marchi <simon.marchi@polymtl.ca>

The class_deprecated enumerator isn't assigned anywhere, so remove it.
Commands that are deprecated have cmd_list_element::cmd_deprecated set
instead.

Change-Id: Ib35e540915c52aa65f13bfe9b8e4e22e6007903c
---
 gdb/cli/cli-setshow.c | 6 +-----
 gdb/command.h         | 1 -
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index 42c2b15b3928..dcb50caf5e33 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -37,11 +37,7 @@ notify_command_param_changed_p (bool param_changed, struct cmd_list_element *c)
   if (!param_changed)
     return false;
 
-  if (c->theclass == class_maintenance || c->theclass == class_deprecated
-      || c->theclass == class_obscure)
-    return false;
-
-  return true;
+  return c->theclass != class_maintenance && c->theclass != class_obscure;
 }
 
 \f
diff --git a/gdb/command.h b/gdb/command.h
index ef3c52949dca..a8a92d797f8a 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -46,7 +46,6 @@ enum command_class
      Note that help accepts unambiguous abbreviated class names.  */
 
   /* Special classes to help_list */
-  class_deprecated = -3,
   all_classes = -2,  /* help without <classname> */
   all_commands = -1, /* all */
 
-- 
2.26.2

[PATCH 4/5] gdb: add "info index-cache stats", deprecate "show index-cache stats"

[Simon Marchi]

From: Simon Marchi <simon.marchi@polymtl.ca>

The "show index-cache stats" command is used to show some statistics
about the index-cache usage.  I believe this is not a good use of a show
command, as it is not a setting.

For example, it is listed by "info set", in a way that doesn't make
sense:

    ...
    host-charset:  The host character set is "auto; currently UTF-8".
    index-cache directory:  The directory of the index cache is "/home/simark/.cache/gdb".
    index-cache stats:    Cache hits (this session): 0
    Cache misses (this session): 0
    inferior-tty:  Terminal for future runs of program being debugged is "".
    ...

When using the -gdb-show MI command, the result isn't useful either:

    (gdb) interpreter-exec mi "-gdb-show index-cache stats"
    ~"  Cache hits (this session): 0\n"
    ~"Cache misses (this session): 0\n"
    ^done

I think it makes more sense to have an info command for this kind of
thing, and that set/show commands should be kept for actual settings.
Introduce "info index-cache stats", and make "show index-cache stats" a
deprecated alias of that.

Just with that change, the output of the "info index-cache" prefix
included the deprecated "show index-cache stats" alias:

    (gdb) info index-cache
    List of info index-cache subcommands:

    info index-cache stats, show index-cache stats -- Print some statistics about the index cache.

When listing sub-commands, I don't think we should show the deprecated
aliases.  They are kept for backwards compatibility, but we don't really
want to advertise them.  Change fput_command_names_styled to exclude
aliases marked as deprecated.

Update tests using "show index-cache stats" to use the new command, and
update the doc.

Change-Id: I13be3c6f98dec9bb4159013b2f7a5df234f6af32
---
 gdb/cli/cli-decode.c                   | 36 ++++++++++++++---
 gdb/doc/gdb.texinfo                    |  2 +-
 gdb/dwarf2/index-cache.c               | 53 +++++++++++++-------------
 gdb/testsuite/gdb.base/index-cache.exp |  7 ++--
 gdb/testsuite/gdb.base/maint.exp       |  2 +-
 5 files changed, 62 insertions(+), 38 deletions(-)

diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index de90198dfa71..ab06954de5da 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -1359,9 +1359,11 @@ fput_aliases_definition_styled (struct cmd_list_element *cmd,
 }
 
 
-/* If C has one or more aliases, style print the name of C and
-   the name of its aliases, separated by commas.
+/* If C has one or more non-deprecated aliases, style print the name of C and
+   the name of its non-deprecated aliases, separated by commas.
+
    If ALWAYS_FPUT_C_NAME, print the name of C even if it has no aliases.
+
    If one or more names are printed, POSTFIX is printed after the last name.
 */
 
@@ -1370,17 +1372,39 @@ fput_command_names_styled (struct cmd_list_element *c,
 			   bool always_fput_c_name, const char *postfix,
 			   struct ui_file *stream)
 {
-  if (always_fput_c_name ||  c->aliases != nullptr)
+  /* Return true if ALIAS should be printed.  */
+  auto print_alias = [] (cmd_list_element *alias)
+    {
+      return !alias->cmd_deprecated;
+    };
+
+  /* See if we are going to print something, if either ALWAYS_FPUT_C_NAME is true
+     or there exists at least one non-deprecated alias.  */
+  bool print_something = always_fput_c_name;
+  for (cmd_list_element *alias = c->aliases;
+       alias != nullptr && !print_something;
+       alias = alias->alias_chain)
+    {
+      if (!print_alias (alias))
+	continue;
+
+      print_something = true;
+    }
+
+  if (print_something)
     fput_command_name_styled (c, stream);
 
-  for (cmd_list_element *iter = c->aliases; iter; iter = iter->alias_chain)
+  for (cmd_list_element *alias = c->aliases; alias; alias = alias->alias_chain)
     {
+      if (!print_alias (alias))
+	continue;
+
       fputs_filtered (", ", stream);
       wrap_here ("   ");
-      fput_command_name_styled (iter, stream);
+      fput_command_name_styled (alias, stream);
     }
 
-  if (always_fput_c_name ||  c->aliases != nullptr)
+  if (print_something)
     fputs_filtered (postfix, stream);
 }
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 308a97aeb82f..c435bc655b0d 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -21820,7 +21820,7 @@ differ according to local convention.
 There is no limit on the disk space used by index cache.  It is perfectly safe
 to delete the content of that directory to free up disk space.
 
-@item show index-cache stats
+@item info index-cache stats
 Print the number of cache hits and misses since the launch of @value{GDBN}.
 
 @end table
diff --git a/gdb/dwarf2/index-cache.c b/gdb/dwarf2/index-cache.c
index f439b37db5ae..4a83213a6250 100644
--- a/gdb/dwarf2/index-cache.c
+++ b/gdb/dwarf2/index-cache.c
@@ -46,6 +46,9 @@ index_cache global_index_cache;
 static cmd_list_element *set_index_cache_prefix_list;
 static cmd_list_element *show_index_cache_prefix_list;
 
+/* info index-cache commands.  */
+static cmd_list_element *info_index_cache_prefix_list;
+
 /* Default destructor of index_cache_resource.  */
 index_cache_resource::~index_cache_resource () = default;
 
@@ -246,18 +249,11 @@ index_cache::make_index_filename (const bfd_build_id *build_id,
   return m_dir + SLASH_STRING + build_id_str + suffix;
 }
 
-/* True when we are executing "show index-cache".  This is used to improve the
-   printout a little bit.  */
-static bool in_show_index_cache_command = false;
-
 /* "show index-cache" handler.  */
 
 static void
 show_index_cache_command (const char *arg, int from_tty)
 {
-  /* Note that we are executing "show index-cache".  */
-  auto restore_flag = make_scoped_restore (&in_show_index_cache_command, true);
-
   /* Call all "show index-cache" subcommands.  */
   cmd_show_list (show_index_cache_prefix_list, from_tty);
 
@@ -296,25 +292,15 @@ set_index_cache_directory_command (const char *arg, int from_tty,
   global_index_cache.set_directory (index_cache_directory);
 }
 
-/* "show index-cache stats" handler.  */
+/* "info index-cache stats" handler.  */
 
 static void
-show_index_cache_stats_command (const char *arg, int from_tty)
+info_index_cache_stats_command (const char *arg, int from_tty)
 {
-  const char *indent = "";
-
-  /* If this command is invoked through "show index-cache", make the display a
-     bit nicer.  */
-  if (in_show_index_cache_command)
-    {
-      indent = "  ";
-      printf_unfiltered ("\n");
-    }
-
-  printf_unfiltered (_("%s  Cache hits (this session): %u\n"),
-		     indent, global_index_cache.n_hits ());
-  printf_unfiltered (_("%sCache misses (this session): %u\n"),
-		     indent, global_index_cache.n_misses ());
+  printf_unfiltered (_("Cache hits (this session): %u\n"),
+		     global_index_cache.n_hits ());
+  printf_unfiltered (_("Cache misses (this session): %u\n"),
+		     global_index_cache.n_misses ());
 }
 
 void _initialize_index_cache ();
@@ -359,10 +345,23 @@ _initialize_index_cache ()
 			    &set_index_cache_prefix_list,
 			    &show_index_cache_prefix_list);
 
-  /* show index-cache stats */
-  add_cmd ("stats", class_files, show_index_cache_stats_command,
-	   _("Show some stats about the index cache."),
-	   &show_index_cache_prefix_list);
+  /* info index-cache */
+  add_basic_prefix_cmd ("index-cache", class_files,
+			"Get information about the index-cache.",
+			&info_index_cache_prefix_list, 0, &infolist);
+
+  /* info index-cache stats */
+  cmd_list_element *info_index_cache_stats
+    = add_cmd ("stats", class_files, info_index_cache_stats_command,
+	       _("Print some statistics about the index cache."),
+	       &info_index_cache_prefix_list);
+
+  /* "show index-cache stats", deprecated in favor of
+     "info index-cache stats".  */
+  cmd_list_element *show_index_cache_stats
+    = add_alias_cmd ("stats", info_index_cache_stats, class_files, 0,
+		     &show_index_cache_prefix_list);
+  deprecate_cmd (show_index_cache_stats, "info index-cache stats");
 
   /* set debug index-cache */
   add_setshow_boolean_cmd ("index-cache", class_maintenance,
diff --git a/gdb/testsuite/gdb.base/index-cache.exp b/gdb/testsuite/gdb.base/index-cache.exp
index 8dd35ad80bd7..b6e2c3ec0223 100644
--- a/gdb/testsuite/gdb.base/index-cache.exp
+++ b/gdb/testsuite/gdb.base/index-cache.exp
@@ -61,16 +61,17 @@ proc ls_host { dir } {
     return [list 0 $filtered]
 }
 
-# Execute "show index-cache stats" and verify the output against expected
+# Execute "info index-cache stats" and verify the output against expected
 # values.
 
 proc check_cache_stats { expected_hits expected_misses } {
     set re [multi_line \
-	"  Cache hits .this session.: $expected_hits" \
+	"Cache hits .this session.: $expected_hits" \
 	"Cache misses .this session.: $expected_misses" \
     ]
 
-    gdb_test "show index-cache stats" $re "check index-cache stats"
+    gdb_test "info index-cache stats" $re "check index-cache stats"
+    gdb_test "show index-cache stats" $re "check index-cache stats using deprecated command"
 }
 
 # Run CODE using a fresh GDB configured based on the other parameters.
diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
index 0a0cc0f813c8..7d1f462f9739 100644
--- a/gdb/testsuite/gdb.base/maint.exp
+++ b/gdb/testsuite/gdb.base/maint.exp
@@ -124,7 +124,7 @@ set have_gdb_index [ exec_has_index_section ${binfile} ]
 # We can detect this by looking if the index-cache is enabled and if the number
 # of cache misses is 0.
 set index_cache_misses -1
-gdb_test_multiple "show index-cache stats" "check index cache stats" {
+gdb_test_multiple "info index-cache stats" "check index cache stats" {
     -re ".*Cache misses \\(this session\\): (\\d+)\r\n.*$gdb_prompt $" {
 	set index_cache_misses $expect_out(1,string)
     }
-- 
2.26.2

[PATCH 5/5] gdb: introduce "set index-cache enabled", deprecate "set index-cache on/off"

[Simon Marchi]

From: Simon Marchi <simon.marchi@polymtl.ca>

The "set index-cache" command is used at the same time as a prefix
command (prefix for "set index-cache directory", for example), and a
boolean setting for turning the index-cache on and off.  Even though I
did introduce that, I now don't think it's a good idea to do something
non-standard like this.

First, there's no dedicated CLI command to show whether the index-cache
is enabled, so it has to be custom output in the "show index-cache
handler".  Also, it means there's no good way a MI frontend can find out
if the index-cache is enabled.  "-gdb-show index-cache" doesn't show it
in the MI output record:

    (gdb) interpreter-exec mi "-gdb-show index-cache"
    ~"\n"
    ~"The index cache is currently disabled.\n"
    ^done,showlist={option={name="directory",value="/home/simark/.cache/gdb"}}

Fix this by introducing "set/show index-cache enabled on/off", regular
boolean setting commands.  Keep commands "set index-cache on" and "set
index-cache off" as deprecated aliases of "set index-cache enabled",
with respectively the default arguments "on" and "off".

Update tests using "set index-cache on/off" to use the new command.
Update the regexps in gdb.base/maint.exp to figure out whether the
index-cache is enabled or not.  Update the doc to mention the new
commands.

Change-Id: I7d5aaaf7fd22bf47bd03e0023ef4fbb4023b37b3
---
 gdb/doc/gdb.texinfo                          |  8 +--
 gdb/dwarf2/index-cache.c                     | 73 ++++++++++++--------
 gdb/testsuite/gdb.base/index-cache.exp       | 27 ++++++--
 gdb/testsuite/gdb.base/maint.exp             |  6 +-
 gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp |  6 +-
 5 files changed, 73 insertions(+), 47 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index c435bc655b0d..ed9018b3c0c4 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -21795,14 +21795,14 @@ Indices only work when using DWARF debugging information, not stabs.
 @cindex automatic symbol index cache
 It is possible for @value{GDBN} to automatically save a copy of this index in a
 cache on disk and retrieve it from there when loading the same binary in the
-future.  This feature can be turned on with @kbd{set index-cache on}.  The
-following commands can be used to tweak the behavior of the index cache.
+future.  This feature can be turned on with @kbd{set index-cache enabled on}.
+The following commands can be used to tweak the behavior of the index cache.
 
 @table @code
 
 @kindex set index-cache
-@item set index-cache on
-@itemx set index-cache off
+@item set index-cache enabled on
+@itemx set index-cache enabled off
 Enable or disable the use of the symbol index cache.
 
 @item set index-cache directory @var{directory}
diff --git a/gdb/dwarf2/index-cache.c b/gdb/dwarf2/index-cache.c
index 4a83213a6250..a56d1485dcb3 100644
--- a/gdb/dwarf2/index-cache.c
+++ b/gdb/dwarf2/index-cache.c
@@ -22,6 +22,7 @@
 
 #include "build-id.h"
 #include "cli/cli-cmds.h"
+#include "cli/cli-decode.h"
 #include "command.h"
 #include "gdbsupport/scoped_mmap.h"
 #include "gdbsupport/pathstuff.h"
@@ -249,34 +250,32 @@ index_cache::make_index_filename (const bfd_build_id *build_id,
   return m_dir + SLASH_STRING + build_id_str + suffix;
 }
 
-/* "show index-cache" handler.  */
+/* "set/show index-cache enabled" set callback.  */
 
 static void
-show_index_cache_command (const char *arg, int from_tty)
+set_index_cache_enabled_command (bool value)
 {
-  /* Call all "show index-cache" subcommands.  */
-  cmd_show_list (show_index_cache_prefix_list, from_tty);
-
-  printf_unfiltered ("\n");
-  printf_unfiltered
-    (_("The index cache is currently %s.\n"),
-     global_index_cache.enabled () ? _("enabled") : _("disabled"));
+  if (value)
+    global_index_cache.enable ();
+  else
+    global_index_cache.disable ();
 }
 
-/* "set index-cache on" handler.  */
+/* "set/show index-cache enabled" get callback.  */
 
-static void
-set_index_cache_on_command (const char *arg, int from_tty)
+static bool
+get_index_cache_enabled_command ()
 {
-  global_index_cache.enable ();
+  return global_index_cache.enabled ();
 }
 
-/* "set index-cache off" handler.  */
+/* "set/show index-cache enabled" show callback.  */
 
 static void
-set_index_cache_off_command (const char *arg, int from_tty)
+show_index_cache_enabled_command (ui_file *stream, int from_tty,
+				  cmd_list_element *cmd, const char *value)
 {
-  global_index_cache.disable ();
+  fprintf_filtered (stream, _("The index cache is %s.\n"), value);
 }
 
 /* "set index-cache directory" handler.  */
@@ -317,24 +316,38 @@ _initialize_index_cache ()
   else
     warning (_("Couldn't determine a path for the index cache directory."));
 
-  /* set index-cache */
-  add_basic_prefix_cmd ("index-cache", class_files,
-			_("Set index-cache options."),
-			&set_index_cache_prefix_list,
-			false, &setlist);
-
-  /* show index-cache */
-  add_prefix_cmd ("index-cache", class_files, show_index_cache_command,
-		  _("Show index-cache options."), &show_index_cache_prefix_list,
-		  false, &showlist);
+  /* set/show index-cache */
+  add_setshow_prefix_cmd ("index-cache", class_files,
+			  _("Set index-cache options."),
+			  _("Show index-cache options."),
+			  &set_index_cache_prefix_list,
+			  &show_index_cache_prefix_list,
+			  &setlist, &showlist);
+
+  set_show_commands setshow_index_cache_enabled_cmds
+    = add_setshow_boolean_cmd ("enabled", class_files,
+			       _("Enable the index cache."),
+			       _("Show whether the index cache is enabled."),
+			       _("help doc"),
+			       set_index_cache_enabled_command,
+			       get_index_cache_enabled_command,
+			       show_index_cache_enabled_command,
+			       &set_index_cache_prefix_list,
+			       &show_index_cache_prefix_list);
 
   /* set index-cache on */
-  add_cmd ("on", class_files, set_index_cache_on_command,
-	   _("Enable the index cache."), &set_index_cache_prefix_list);
+  cmd_list_element *set_index_cache_on_cmd
+    = add_alias_cmd ("on", setshow_index_cache_enabled_cmds.set, class_files,
+		     false, &set_index_cache_prefix_list);
+  deprecate_cmd (set_index_cache_on_cmd, "set index-cache enabled on");
+  set_index_cache_on_cmd->default_args = "on";
 
   /* set index-cache off */
-  add_cmd ("off", class_files, set_index_cache_off_command,
-	   _("Disable the index cache."), &set_index_cache_prefix_list);
+  cmd_list_element *set_index_cache_off_cmd
+    = add_alias_cmd ("off", setshow_index_cache_enabled_cmds.set, class_files,
+		     false, &set_index_cache_prefix_list);
+  deprecate_cmd (set_index_cache_off_cmd, "set index-cache enabled off");
+  set_index_cache_off_cmd->default_args = "off";
 
   /* set index-cache directory */
   add_setshow_filename_cmd ("directory", class_files, &index_cache_directory,
diff --git a/gdb/testsuite/gdb.base/index-cache.exp b/gdb/testsuite/gdb.base/index-cache.exp
index b6e2c3ec0223..626f88bd5a0a 100644
--- a/gdb/testsuite/gdb.base/index-cache.exp
+++ b/gdb/testsuite/gdb.base/index-cache.exp
@@ -81,7 +81,7 @@ proc run_test_with_flags { cache_dir cache_enabled code } {
 
     save_vars { GDBFLAGS } {
 	set GDBFLAGS "$GDBFLAGS -iex \"set index-cache directory $cache_dir\""
-	set GDBFLAGS "$GDBFLAGS -iex \"set index-cache $cache_enabled\""
+	set GDBFLAGS "$GDBFLAGS -iex \"set index-cache enabled $cache_enabled\""
 
 	clean_restart ${testfile}
 
@@ -98,17 +98,30 @@ proc_with_prefix test_basic_stuff { } {
 
     # Check that the index cache is disabled by default.
     gdb_test \
-	"show index-cache" \
-	" is currently disabled." \
+	"show index-cache enabled" \
+	"The index cache is off." \
 	"index-cache is disabled by default"
 
-    # Test that we can enable it and "show index-cache" reflects that.
-    gdb_test_no_output "set index-cache on" "enable index cache"
+    # Test that we can enable it and "show index-cache enabled" reflects that.
+    gdb_test_no_output "set index-cache enabled on" "enable index cache"
     gdb_test \
-	"show index-cache" \
-	" is currently enabled." \
+	"show index-cache enabled" \
+	"The index cache is on." \
 	"index-cache is now enabled"
 
+    with_test_prefix "deprecated commands" {
+        gdb_test "set index-cache off" ".*is deprecated.*" "disable index cache"
+	gdb_test \
+	    "show index-cache enabled" \
+	    "The index cache is off." \
+	    "index-cache is now disabled"
+        gdb_test "set index-cache on" ".*is deprecated.*" "enable index cache"
+	gdb_test \
+	    "show index-cache enabled" \
+	    "The index cache is on." \
+	    "index-cache is now enabled"
+    }
+
     # Test the "set/show index-cache directory" commands.
     gdb_test "set index-cache directory" "Argument required.*" "set index-cache directory without arg"
     gdb_test_no_output "set index-cache directory /tmp" "change the index cache directory"
diff --git a/gdb/testsuite/gdb.base/maint.exp b/gdb/testsuite/gdb.base/maint.exp
index 7d1f462f9739..34fc9e1090ed 100644
--- a/gdb/testsuite/gdb.base/maint.exp
+++ b/gdb/testsuite/gdb.base/maint.exp
@@ -131,11 +131,11 @@ gdb_test_multiple "info index-cache stats" "check index cache stats" {
 }
 
 set using_index_cache 0
-gdb_test_multiple "show index-cache" "check index cache status" {
-    -re ".*is currently disabled.\r\n$gdb_prompt $" {
+gdb_test_multiple "show index-cache enabled" "check index cache status" {
+    -re ".*is off.\r\n$gdb_prompt $" {
 	set using_index_cache 0
     }
-    -re ".*is currently enabled.\r\n$gdb_prompt $" {
+    -re ".*is on.\r\n$gdb_prompt $" {
 	set using_index_cache 1
     }
 }
diff --git a/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp b/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp
index 6891cd774465..47c92d9a26f2 100644
--- a/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp
+++ b/gdb/testsuite/gdb.dwarf2/per-bfd-sharing.exp
@@ -45,7 +45,7 @@ with_test_prefix "populate index cache" {
 
     gdb_test_no_output "set index-cache directory $cache_dir" \
 	"set index-cache directory"
-    gdb_test_no_output "set index-cache on"
+    gdb_test_no_output "set index-cache enabled on"
     gdb_test "file $binfile" "Reading symbols from .*" "file"
 }
 
@@ -56,9 +56,9 @@ proc load_binary { method } {
     if { $method == "standard" } {
 	gdb_test "file $binfile" "Reading symbols from .*" "file"
     } elseif { $method == "index" } {
-	gdb_test_no_output "set index-cache on"
+	gdb_test_no_output "set index-cache enabled on"
 	gdb_test "file $binfile" "Reading symbols from .*" "file index"
-	gdb_test_no_output "set index-cache off"
+	gdb_test_no_output "set index-cache enabled off"
     } elseif { $method == "readnow" } {
 	gdb_test "file -readnow $binfile" \
 	    "Reading symbols from .*Expanding full symbols from .*" \
-- 
2.26.2

Re: [PATCH 1/5] gdb: pass/return setting setter/getter scalar values by value

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> The getter and setter in struct setting always receive and return values
Simon> by const reference.  This is not necessary for scalar values (like bool
Simon> and int), but more importantly it makes it a bit annoying to write a
Simon> getter, you have to use a scratch static variable or something similar
Simon> that you can refer to:

Seems reasonable to me.

Simon> -		      setting_setter_ftype<const char *> set_func,
Simon> -		      setting_getter_ftype<const char *> get_func,
Simon> +		      typename the_types<const char *>::set set_func,
Simon> +		      typename the_types<const char *>::get get_func,

Is "typename" really needed in spots like this?

Tom

Re: [PATCH 2/5] gdb: remove unnecessary cmd_list_element::aliases nullptr checks

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> From: Simon Marchi <simon.marchi@polymtl.ca>
Simon> Remove two unnecessary nullptr checks.  If aliases is nullptr, then the
Simon> for loops will simply be skipped.

Looks good, obvious even.  Thank you.

Tom

Re: [PATCH 3/5] gdb: remove command_class enum class_deprecated

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> From: Simon Marchi <simon.marchi@polymtl.ca>
Simon> The class_deprecated enumerator isn't assigned anywhere, so remove it.
Simon> Commands that are deprecated have cmd_list_element::cmd_deprecated set
Simon> instead.

Looks good, thanks.

Tom

Re: [PATCH 4/5] gdb: add "info index-cache stats", deprecate "show index-cache stats"

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> The "show index-cache stats" command is used to show some statistics
Simon> about the index-cache usage.  I believe this is not a good use of a show
Simon> command, as it is not a setting.

I think "show" is generally used for any information about the debugger,
not just settings.

A long, long time ago there was some effort to split 'info' and 'show'
this way.  The details of this discussion are long lost, as far as I
know, but the outcome lives on in the help and manual:

'info'
     This command (abbreviated 'i') is for describing the state of your
     program.

'show'
     In contrast to 'info', 'show' is for describing the state of GDB
     itself.  You can change most of the things you can 'show', by using [...]

So I tend to think "show index-cache stats" is more correct.

Tom

Re: [PATCH 5/5] gdb: introduce "set index-cache enabled", deprecate "set index-cache on/off"

[Tom Tromey]

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> Fix this by introducing "set/show index-cache enabled on/off", regular
Simon> boolean setting commands.  Keep commands "set index-cache on" and "set
Simon> index-cache off" as deprecated aliases of "set index-cache enabled",
Simon> with respectively the default arguments "on" and "off".

This seems fine to me.  Thank you.

Tom

Re: [PATCH 1/5] gdb: pass/return setting setter/getter scalar values by value

[Simon Marchi]

On 2021-11-04 14:27, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> The getter and setter in struct setting always receive and return values
> Simon> by const reference.  This is not necessary for scalar values (like bool
> Simon> and int), but more importantly it makes it a bit annoying to write a
> Simon> getter, you have to use a scratch static variable or something similar
> Simon> that you can refer to:
> 
> Seems reasonable to me.
> 
> Simon> -		      setting_setter_ftype<const char *> set_func,
> Simon> -		      setting_getter_ftype<const char *> get_func,
> Simon> +		      typename the_types<const char *>::set set_func,
> Simon> +		      typename the_types<const char *>::get get_func,
> 
> Is "typename" really needed in spots like this?

Hmm no, actually.  Everywhere where the type doesn't depend on a
template type, I could remove it.  Fixed locally.

Also, that made me realize that I didn't change the names "the_types"
and "the_types2", which were work-in-progress names.  I changed them
locally to setting_func_types and setting_func_types_1, respectively.

Simon

Re: [PATCH 4/5] gdb: add "info index-cache stats", deprecate "show index-cache stats"

[Simon Marchi]

On 2021-11-04 14:33, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> The "show index-cache stats" command is used to show some statistics
> Simon> about the index-cache usage.  I believe this is not a good use of a show
> Simon> command, as it is not a setting.
> 
> I think "show" is generally used for any information about the debugger,
> not just settings.
> 
> A long, long time ago there was some effort to split 'info' and 'show'
> this way.  The details of this discussion are long lost, as far as I
> know, but the outcome lives on in the help and manual:
> 
> 'info'
>      This command (abbreviated 'i') is for describing the state of your
>      program.
> 
> 'show'
>      In contrast to 'info', 'show' is for describing the state of GDB
>      itself.  You can change most of the things you can 'show', by using [...]
> 
> So I tend to think "show index-cache stats" is more correct.

Ah, I didn't know about that, thanks.

Then I still wonder what to do about:

    (gdb) interpreter-exec mi "-gdb-show index-cache"
    ~"\n"
    ~"    Cache hits (this session): 0\n"
    ~"  Cache misses (this session): 0\n"
    ~"\n"
    ~"The index cache is currently disabled.\n"
    ^done

The answer is probably to output those numbers using uiout fields in the
command implementation.  If I hack that, I get:

    ...
    ^done,hits="0",misses="0"

which looks good.

What's nice about "show" is that we don't need to add a new MI command
for everything: -gdb-show just works with any child of the "show"
command.  It would be nice to have the same thing with info, have an
"-info" MI command that works with "info" command children.  Of course,
these commands would have to be MI-aware (emit information using uiout
fields), but that's much easier than adding new MI commands for each.

So, I'll drop this patch then, and consider changing the "show
index-cache stats" implementation to use uiout fields (although I am not
aware of any frontend interested in using the the index-cache stats
data).

Simon

Re: [PATCH 2/5] gdb: remove unnecessary cmd_list_element::aliases nullptr checks

[Simon Marchi]

On 2021-11-04 14:29, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> From: Simon Marchi <simon.marchi@polymtl.ca>
> Simon> Remove two unnecessary nullptr checks.  If aliases is nullptr, then the
> Simon> for loops will simply be skipped.
> 
> Looks good, obvious even.  Thank you.
> 
> Tom
> 

Pushed, thanks.

Simon

Re: [PATCH 3/5] gdb: remove command_class enum class_deprecated

[Simon Marchi]

On 2021-11-04 14:30, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> From: Simon Marchi <simon.marchi@polymtl.ca>
> Simon> The class_deprecated enumerator isn't assigned anywhere, so remove it.
> Simon> Commands that are deprecated have cmd_list_element::cmd_deprecated set
> Simon> instead.
> 
> Looks good, thanks.
> 
> Tom
> 

Pushed, thanks.

Simon

Re: [PATCH 1/5] gdb: pass/return setting setter/getter scalar values by value

[Simon Marchi]

On 2021-11-04 14:59, Simon Marchi wrote:
> On 2021-11-04 14:27, Tom Tromey wrote:
>>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
>>
>> Simon> The getter and setter in struct setting always receive and return values
>> Simon> by const reference.  This is not necessary for scalar values (like bool
>> Simon> and int), but more importantly it makes it a bit annoying to write a
>> Simon> getter, you have to use a scratch static variable or something similar
>> Simon> that you can refer to:
>>
>> Seems reasonable to me.
>>
>> Simon> -		      setting_setter_ftype<const char *> set_func,
>> Simon> -		      setting_getter_ftype<const char *> get_func,
>> Simon> +		      typename the_types<const char *>::set set_func,
>> Simon> +		      typename the_types<const char *>::get get_func,
>>
>> Is "typename" really needed in spots like this?
> 
> Hmm no, actually.  Everywhere where the type doesn't depend on a
> template type, I could remove it.  Fixed locally.
> 
> Also, that made me realize that I didn't change the names "the_types"
> and "the_types2", which were work-in-progress names.  I changed them
> locally to setting_func_types and setting_func_types_1, respectively.
> 
> Simon
> 

Pushed with these fixes.

Simon

Re: [PATCH 5/5] gdb: introduce "set index-cache enabled", deprecate "set index-cache on/off"

[Simon Marchi]

On 2021-11-04 14:36, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> Fix this by introducing "set/show index-cache enabled on/off", regular
> Simon> boolean setting commands.  Keep commands "set index-cache on" and "set
> Simon> index-cache off" as deprecated aliases of "set index-cache enabled",
> Simon> with respectively the default arguments "on" and "off".
> 
> This seems fine to me.  Thank you.
> 
> Tom
> 

Pushed, thanks.

Simon