[RFAv2,1/3] New "info sources" args [-dirname | -basename] [--] [REGEXP]

Message ID 20190616203804.26938-2-philippe.waroquiers@skynet.be
State New, archived
Headers

Commit Message

Philippe Waroquiers June 16, 2019, 8:38 p.m. UTC
  gdb/ChangeLog

2019-06-16  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* symtab.c (filename_partial_match_opts): New struct type.
	(struct output_source_filename_data): New members
	regexp, c_regexp, partial_match.
	(output_source_filename): Use new members to decide to print file.
	(info_sources_option_defs): New variable.
	(make_info_sources_options_def_group, print_info_sources_header,
	info_sources_command_completer):
	New functions.
	(info_sources_command): Read new optional arguments.
	(_initialize_symtab): Update info sources help.
---
 gdb/symtab.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 150 insertions(+), 8 deletions(-)
  

Comments

Tom Tromey July 24, 2019, 7:14 p.m. UTC | #1
>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> gdb/ChangeLog
Philippe> 2019-06-16  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

This commit probably needs a bit of intro text.

Philippe> +static void
Philippe> +print_info_sources_header (const char *symbol_msg,
Philippe> +			   const struct output_source_filename_data *data)
Philippe> +{
Philippe> +  printf_filtered ("Source files ");
Philippe> +  if (!data->regexp.empty ())
Philippe> +    {
Philippe> +      if (data->partial_match.dirname)
Philippe> +	printf_filtered ("(dirname ");
Philippe> +      else if (data->partial_match.basename)
Philippe> +	printf_filtered ("(basename ");
Philippe> +      else
Philippe> +	printf_filtered ("(filename ");
Philippe> +      printf_filtered ("matching regular expression \"%s\") ",
Philippe> +		       data->regexp.c_str ());
Philippe> +    }
Philippe> +  printf_filtered ("for which symbols %s:\n\n", symbol_msg);

This sort of approach isn't very i18n-friendly.  I think we've been
trying to avoid introducing new code like this, in hopes that maybe
somebody will provide gdb translations some day.

One idea might be to separate the main text from the parenthetical text,
so like:

  printf_filtered (_("Source files for which symbols have been read in"));
  if (blah)
    printf_filtered (_("(dirname matching regular expression \"%s\")"), dirname);
  ...
  printf_filtered (_(":\n"));

The "if"s could be factored into a helper function.

Otherwise this patch seems good to me.  Thanks for doing this.

Tom
  
Philippe Waroquiers Aug. 3, 2019, 7:42 p.m. UTC | #2
On Wed, 2019-07-24 at 13:14 -0600, Tom Tromey wrote:
> One idea might be to separate the main text from the parenthetical text,
> so like:
> 
>   printf_filtered (_("Source files for which symbols have been read in"));
>   if (blah)
>     printf_filtered (_("(dirname matching regular expression \"%s\")"), dirname);
>   ...
>   printf_filtered (_(":\n"));
> 
> The "if"s could be factored into a helper function.
> 
> Otherwise this patch seems good to me.  Thanks for doing this.
Thanks for the review.

Pushed after fixing the above, adding a copyright header that was missing
in a test file.

Philippe
  

Patch

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 4920d94a24..854af63651 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4114,10 +4114,28 @@  operator_chars (const char *p, const char **end)
 }
 
 
+/* What part to match in a file name.  */
+
+struct filename_partial_match_opts
+{
+  /* Only match the directory name part.   */
+  int dirname = false;
+
+  /* Only match the basename part.  */
+  int basename = false;
+};
+
 /* Data structure to maintain printing state for output_source_filename.  */
 
 struct output_source_filename_data
 {
+  /* Output only filenames matching REGEXP.  */
+  std::string regexp;
+  gdb::optional<compiled_regex> c_regexp;
+  /* Possibly only match a part of the filename.  */
+  filename_partial_match_opts partial_match;
+
+
   /* Cache of what we've seen so far.  */
   struct filename_seen_cache *filename_seen_cache;
 
@@ -4149,7 +4167,27 @@  output_source_filename (const char *name,
       return;
     }
 
-  /* No; print it and reset *FIRST.  */
+  /* Does it match data->regexp?  */
+  if (data->c_regexp.has_value ())
+    {
+      const char *to_match;
+      std::string dirname;
+
+      if (data->partial_match.dirname)
+	{
+	  dirname = ldirname (name);
+	  to_match = dirname.c_str ();
+	}
+      else if (data->partial_match.basename)
+	to_match = lbasename (name);
+      else
+	to_match = name;
+
+      if (data->c_regexp->exec (to_match, 0, NULL, 0) != 0)
+	return;
+    }
+
+  /* Print it and reset *FIRST.  */
   if (! data->first)
     printf_filtered (", ");
   data->first = 0;
@@ -4168,8 +4206,73 @@  output_partial_symbol_filename (const char *filename, const char *fullname,
 			  (struct output_source_filename_data *) data);
 }
 
+using isrc_flag_option_def
+  = gdb::option::flag_option_def<filename_partial_match_opts>;
+
+static const gdb::option::option_def info_sources_option_defs[] = {
+
+  isrc_flag_option_def {
+    "dirname",
+    [] (filename_partial_match_opts *opts) { return &opts->dirname; },
+    N_("Show only the files having a dirname matching REGEXP."),
+  },
+
+  isrc_flag_option_def {
+    "basename",
+    [] (filename_partial_match_opts *opts) { return &opts->basename; },
+    N_("Show only the files having a basename matching REGEXP."),
+  },
+
+};
+
+/* Create an option_def_group for the "info sources" options, with
+   ISRC_OPTS as context.  */
+
+static inline gdb::option::option_def_group
+make_info_sources_options_def_group (filename_partial_match_opts *isrc_opts)
+{
+  return {{info_sources_option_defs}, isrc_opts};
+}
+
+/* Prints the header message for the source files that will be printed
+   with the matching info present in DATA.  SYMBOL_MSG is a message
+   that tells what will or has been done with the symbols of the
+   matching source files.  */
+
+static void
+print_info_sources_header (const char *symbol_msg,
+			   const struct output_source_filename_data *data)
+{
+  printf_filtered ("Source files ");
+  if (!data->regexp.empty ())
+    {
+      if (data->partial_match.dirname)
+	printf_filtered ("(dirname ");
+      else if (data->partial_match.basename)
+	printf_filtered ("(basename ");
+      else
+	printf_filtered ("(filename ");
+      printf_filtered ("matching regular expression \"%s\") ",
+		       data->regexp.c_str ());
+    }
+  printf_filtered ("for which symbols %s:\n\n", symbol_msg);
+}
+
+/* Completer for "info sources".  */
+
+static void
+info_sources_command_completer (cmd_list_element *ignore,
+				completion_tracker &tracker,
+				const char *text, const char *word)
+{
+  const auto group = make_info_sources_options_def_group (nullptr);
+  if (gdb::option::complete_options
+      (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group))
+    return;
+}
+
 static void
-info_sources_command (const char *ignore, int from_tty)
+info_sources_command (const char *args, int from_tty)
 {
   struct output_source_filename_data data;
 
@@ -4180,11 +4283,37 @@  info_sources_command (const char *ignore, int from_tty)
 
   filename_seen_cache filenames_seen;
 
-  data.filename_seen_cache = &filenames_seen;
+  auto group = make_info_sources_options_def_group (&data.partial_match);
+
+  gdb::option::process_options
+    (&args, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, group);
 
-  printf_filtered ("Source files for which symbols have been read in:\n\n");
+  if (args != NULL && *args != '\000')
+    data.regexp = args;
 
+  data.filename_seen_cache = &filenames_seen;
   data.first = 1;
+
+  if (data.partial_match.dirname && data.partial_match.basename)
+    error (_("You cannot give both -basename and -dirname to 'info sources'."));
+  if ((data.partial_match.dirname || data.partial_match.basename)
+      && data.regexp.empty ())
+     error (_("Missing REGEXP for 'info sources'."));
+
+  if (data.regexp.empty ())
+    data.c_regexp.reset ();
+  else
+    {
+      int cflags = REG_NOSUB;
+#ifdef HAVE_CASE_INSENSITIVE_FILE_SYSTEM
+      cflags |= REG_ICASE;
+#endif
+      data.c_regexp.emplace (data.regexp.c_str (), cflags,
+			     _("Invalid regexp"));
+    }
+
+  print_info_sources_header ("have been read in", &data);
+
   for (objfile *objfile : current_program_space->objfiles ())
     {
       for (compunit_symtab *cu : objfile->compunits ())
@@ -4199,8 +4328,7 @@  info_sources_command (const char *ignore, int from_tty)
     }
   printf_filtered ("\n\n");
 
-  printf_filtered ("Source files for which symbols "
-		   "will be read in on demand:\n\n");
+  print_info_sources_header ("will be read in on demand", &data);
 
   filenames_seen.clear ();
   data.first = 1;
@@ -6008,6 +6136,8 @@  symbol_set_symtab (struct symbol *symbol, struct symtab *symtab)
 void
 _initialize_symtab (void)
 {
+  struct cmd_list_element *c;
+
   initialize_ordinary_address_classes ();
 
   add_info ("variables", info_variables_command,
@@ -6042,8 +6172,20 @@  Prints the functions.\n"),
   add_info ("types", info_types_command,
 	    _("All type names, or those matching REGEXP."));
 
-  add_info ("sources", info_sources_command,
-	    _("Source files in the program."));
+  const auto info_sources_opts = make_info_sources_options_def_group (nullptr);
+
+  static std::string info_sources_help
+    = gdb::option::build_help (_("\
+All source files in the program or those matching REGEXP.\n\
+Usage: info sources [OPTION]... [REGEXP]\n\
+By default, REGEXP is used to match anywhere in the filename.\n\
+\n\
+Options:\n\
+%OPTIONS%"),
+			       info_sources_opts);
+
+  c = add_info ("sources", info_sources_command, info_sources_help.c_str ());
+  set_cmd_completer_handle_brkchars (c, info_sources_command_completer);
 
   add_com ("rbreak", class_breakpoint, rbreak_command,
 	   _("Set a breakpoint for all functions matching REGEXP."));