public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@efficios.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 5/5] gdb: introduce "set index-cache enabled", deprecate "set index-cache on/off"
Date: Mon,  1 Nov 2021 11:50:09 -0400	[thread overview]
Message-ID: <20211101155009.457224-6-simon.marchi@efficios.com> (raw)
In-Reply-To: <20211101155009.457224-1-simon.marchi@efficios.com>

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


  parent reply	other threads:[~2021-11-01 15:50 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-01 15:50 [PATCH 0/5] Change some index-cache commands Simon Marchi
2021-11-01 15:50 ` [PATCH 1/5] gdb: pass/return setting setter/getter scalar values by value Simon Marchi
2021-11-04 18:27   ` Tom Tromey
2021-11-04 18:59     ` Simon Marchi
2021-11-04 19:51       ` Simon Marchi
2021-11-01 15:50 ` [PATCH 2/5] gdb: remove unnecessary cmd_list_element::aliases nullptr checks Simon Marchi
2021-11-04 18:29   ` Tom Tromey
2021-11-04 19:45     ` Simon Marchi
2021-11-01 15:50 ` [PATCH 3/5] gdb: remove command_class enum class_deprecated Simon Marchi
2021-11-04 18:30   ` Tom Tromey
2021-11-04 19:45     ` Simon Marchi
2021-11-01 15:50 ` [PATCH 4/5] gdb: add "info index-cache stats", deprecate "show index-cache stats" Simon Marchi
2021-11-04 18:33   ` Tom Tromey
2021-11-04 19:19     ` Simon Marchi
2021-11-01 15:50 ` Simon Marchi [this message]
2021-11-04 18:36   ` [PATCH 5/5] gdb: introduce "set index-cache enabled", deprecate "set index-cache on/off" Tom Tromey
2021-11-04 19:51     ` Simon Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211101155009.457224-6-simon.marchi@efficios.com \
    --to=simon.marchi@efficios.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).