[PATCHv3 2/5] gdb: make struct output_source_filename_data more C++ like

[Andrew Burgess <andrew.burgess@embecosm.com>]

In a future commit I'm going to be making some changes to the 'info
sources' command.  While looking at the code I noticed that things
could be improved by making struct output_source_filename_data more
C++ like (private member variables, and more member functions).
That's what this commit does.

The 'info sources' filename filtering is split out into a separate
class in this commit.  In a future commit this new filter
class (info_sources_filter) will move into the header file and be used
from the MI code.

There should be no user visible changes after this commit.

gdb/ChangeLog:

	* symtab.c (struct info_sources_filter): New.
	(info_sources_filter::info_sources_filter): New function.
	(info_sources_filter::matches): New function.
	(info_sources_filter::print): New function.
	(struct filename_partial_match_opts): Moved to later in the file
	and update the comment.
	(struct output_source_filename_data)
	<output_source_filename_data>: New constructor.  <regexp>: Delete,
	this is now in info_sources_filter.  <c_regexp>: Delete, this is
	now in info_sources_filter.  <reset_output>: New member function.
	<filename_seen_cache>: Rename to m_filename_seen_cache, change
	from being a pointer, to being an actual object.  <first>: Rename
	to m_first.  <print_header>: New member function. <partial_match>:
	Delete.
	(output_source_filename_data::output): Update now
	m_filename_seen_cache is no longer a pointer, and for other member
	variable name changes. Add a header comment.
	(print_info_sources_header): Renamed to...
	(output_source_filename_data::print_header): ...this.  Update now
	it's a member function and to take account of member variable
	renaming.
	(info_sources_command): Add a header comment, delete stack local
	filename_seen_cache, initialization of output_source_filename_data
	is now done by the constructor.  Call print_header member function
	instead of print_info_sources_header, call reset_output member
	function instead of manually performing the reset.
---
 gdb/ChangeLog |  29 +++++
 gdb/symtab.c  | 324 +++++++++++++++++++++++++++++++++-----------------
 2 files changed, 247 insertions(+), 106 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 5ad1c8a817d..2ff79e0cddf 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4200,46 +4200,190 @@ operator_chars (const char *p, const char **end)
 }
 \f
 
-/* What part to match in a file name.  */
-
-struct filename_partial_match_opts
+/* Class used to encapsulate the filename filtering for the "info sources"
+   command.  */
+struct info_sources_filter
 {
-  /* Only match the directory name part.   */
-  bool dirname = false;
+  /* If filename filtering is being used (see M_C_REGEXP) then which part
+     of the filename is being filtered against?  */
+  enum class match_on
+  {
+    /* Match against the full filename.  */
+    FULLNAME,
 
-  /* Only match the basename part.  */
-  bool basename = false;
+    /* Match only against the directory part of the full filename.  */
+    DIRNAME,
+
+    /* Match only against the basename part of the full filename.  */
+    BASENAME
+  };
+
+  /* Create a filter of MATCH_TYPE using regular expression REGEXP.  If
+     REGEXP is nullptr then all files will match the filter and MATCH_TYPE
+     is ignored.
+
+     The string pointed too by REGEXP must remain live and unchanged for
+     this lifetime of this object as the object only retains a copy of the
+     pointer.  */
+  info_sources_filter (match_on match_type, const char *regexp);
+
+  DISABLE_COPY_AND_ASSIGN (info_sources_filter);
+
+  /* Does FULLNAME match the filter defined by this object, return true if
+     it does, otherwise, return false.  If there is no filtering defined
+     then this function will always return true.  */
+  bool matches (const char *fullname) const;
+
+  /* Print a single line describing this filter, used as part of the "info
+     sources" command output.  If there is no filter in place then nothing
+     is printed.  */
+  void print () const;
+
+private:
+
+  /* The type of filtering in place.  */
+  match_on m_match_type;
+
+  /* Points to the original regexp used to create this filter.  */
+  const char *m_regexp;
+
+  /* A compiled version of M_REGEXP.  This object is only given a value if
+     M_REGEXP is not nullptr and is not the empty string.  */
+  gdb::optional<compiled_regex> m_c_regexp;
 };
 
-/* Data structure to maintain printing state for output_source_filename.  */
+/* See class declaration.  */
 
-struct output_source_filename_data
+info_sources_filter::info_sources_filter (match_on match_type,
+                                          const char *regexp)
+  : m_match_type (match_type),
+    m_regexp (regexp)
 {
-  /* 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;
+  /* Setup the compiled regular expression M_C_REGEXP based on M_REGEXP.  */
+  if (m_regexp != nullptr && *m_regexp != '\0')
+    {
+      gdb_assert (m_regexp != nullptr);
 
+      int cflags = REG_NOSUB;
+#ifdef HAVE_CASE_INSENSITIVE_FILE_SYSTEM
+      cflags |= REG_ICASE;
+#endif
+      m_c_regexp.emplace (m_regexp, cflags, _("Invalid regexp"));
+    }
+}
 
-  /* Cache of what we've seen so far.  */
-  struct filename_seen_cache *filename_seen_cache;
+/* See class declaration.  */
 
-  /* Flag of whether we're printing the first one.  */
-  int first;
+bool
+info_sources_filter::matches (const char *fullname) const
+{
+  /* Does it match regexp?  */
+  if (m_c_regexp.has_value ())
+    {
+      const char *to_match;
+      std::string dirname;
+
+      switch (m_match_type)
+        {
+        case match_on::DIRNAME:
+          dirname = ldirname (fullname);
+          to_match = dirname.c_str ();
+          break;
+        case match_on::BASENAME:
+          to_match = lbasename (fullname);
+          break;
+        case match_on::FULLNAME:
+          to_match = fullname;
+          break;
+        }
+
+      if (m_c_regexp->exec (to_match, 0, NULL, 0) != 0)
+        return false;
+    }
+
+  return true;
+}
+
+/* See class declaration.  */
+
+void
+info_sources_filter::print () const
+{
+  if (m_c_regexp.has_value ())
+    {
+      gdb_assert (m_regexp != nullptr);
+
+      switch (m_match_type)
+	{
+	case match_on::DIRNAME:
+	  printf_filtered (_("(dirname matching regular expression \"%s\")"),
+			   m_regexp);
+	  break;
+	case match_on::BASENAME:
+	  printf_filtered (_("(basename matching regular expression \"%s\")"),
+			   m_regexp);
+	  break;
+	case match_on::FULLNAME:
+	  printf_filtered (_("(filename matching regular expression \"%s\")"),
+			   m_regexp);
+	  break;
+	}
+    }
+}
+
+/* Data structure to maintain the state used for printing the results of
+   the 'info sources' command.  */
+
+struct output_source_filename_data
+{
+  /* Create an object for displaying the results of the 'info sources'
+     command.  FILTER must remain valid and unchanged for the lifetime of
+     this object as this object retains a reference to FILTER.  */
+  output_source_filename_data (const info_sources_filter &filter)
+    : m_filter (filter)
+  { /* Nothing.  */ }
+
+  DISABLE_COPY_AND_ASSIGN (output_source_filename_data);
+
+  /* Reset enough state of this object so we can match against a new set of
+     files.  The existing regular expression is retained though.  */
+  void reset_output ()
+  {
+    m_first = true;
+    m_filename_seen_cache.clear ();
+  }
 
-  /* Worker for sources_info.  Force line breaks at ,'s.
-     NAME is the name to print.  */
+  /* Worker for sources_info.  Force line breaks at ,'s.  NAME is the name
+     to print.  */
   void output (const char *name);
 
+  /* Prints the header messages for the source files that will be printed
+     with the matching info present in the current object state.
+     SYMBOL_MSG is a message that describes what will or has been done with
+     the symbols of the matching source files.  */
+  void print_header (const char *symbol_msg);
+
   /* An overload suitable for use as a callback to
      quick_symbol_functions::map_symbol_filenames.  */
   void operator() (const char *filename, const char *fullname)
   {
     output (fullname != nullptr ? fullname : filename);
   }
+
+private:
+
+  /* Flag of whether we're printing the first one.  */
+  bool m_first = true;
+
+  /* Cache of what we've seen so far.  */
+  filename_seen_cache m_filename_seen_cache;
+
+  /* How source filename should be filtered.  */
+  const info_sources_filter &m_filter;
 };
 
+/* See comment in class declaration above.  */
+
 void
 output_source_filename_data::output (const char *name)
 {
@@ -4252,42 +4396,45 @@ output_source_filename_data::output (const char *name)
      situation.  I'm not sure whether this can also happen for
      symtabs; it doesn't hurt to check.  */
 
-  /* Was NAME already seen?  */
-  if (filename_seen_cache->seen (name))
-    {
-      /* Yes; don't print it again.  */
-      return;
-    }
-
-  /* Does it match regexp?  */
-  if (c_regexp.has_value ())
-    {
-      const char *to_match;
-      std::string dirname;
-
-      if (partial_match.dirname)
-	{
-	  dirname = ldirname (name);
-	  to_match = dirname.c_str ();
-	}
-      else if (partial_match.basename)
-	to_match = lbasename (name);
-      else
-	to_match = name;
+  /* Was NAME already seen?  If so, then don't print it again.  */
+  if (m_filename_seen_cache.seen (name))
+    return;
 
-      if (c_regexp->exec (to_match, 0, NULL, 0) != 0)
-	return;
-    }
+  /* If the filter rejects this file then don't print it.  */
+  if (!m_filter.matches (name))
+    return;
 
   /* Print it and reset *FIRST.  */
-  if (! first)
+  if (!m_first)
     printf_filtered (", ");
-  first = 0;
+  m_first = false;
 
   wrap_here ("");
   fputs_styled (name, file_name_style.style (), gdb_stdout);
 }
 
+/* See comment is class declaration above.  */
+
+void
+output_source_filename_data::print_header (const char *symbol_msg)
+{
+  puts_filtered (symbol_msg);
+  m_filter.print ();
+  puts_filtered ("\n");
+}
+
+/* For the 'info sources' command, what part of the file names should we be
+   matching the user supplied regular expression against?  */
+
+struct filename_partial_match_opts
+{
+  /* Only match the directory name part.   */
+  bool dirname = false;
+
+  /* Only match the basename part.  */
+  bool basename = false;
+};
+
 using isrc_flag_option_def
   = gdb::option::flag_option_def<filename_partial_match_opts>;
 
@@ -4316,31 +4463,6 @@ 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)
-{
-  puts_filtered (symbol_msg);
-  if (!data->regexp.empty ())
-    {
-      if (data->partial_match.dirname)
-	printf_filtered (_("(dirname matching regular expression \"%s\")"),
-			 data->regexp.c_str ());
-      else if (data->partial_match.basename)
-	printf_filtered (_("(basename matching regular expression \"%s\")"),
-			 data->regexp.c_str ());
-      else
-	printf_filtered (_("(filename matching regular expression \"%s\")"),
-			 data->regexp.c_str ());
-    }
-  puts_filtered ("\n");
-}
-
 /* Completer for "info sources".  */
 
 static void
@@ -4354,49 +4476,41 @@ info_sources_command_completer (cmd_list_element *ignore,
     return;
 }
 
+/* Implement the 'info sources' command.  */
+
 static void
 info_sources_command (const char *args, int from_tty)
 {
-  struct output_source_filename_data data;
-
   if (!have_full_symbols () && !have_partial_symbols ())
-    {
-      error (_("No symbol table is loaded.  Use the \"file\" command."));
-    }
-
-  filename_seen_cache filenames_seen;
-
-  auto group = make_info_sources_options_def_group (&data.partial_match);
+    error (_("No symbol table is loaded.  Use the \"file\" command."));
 
+  filename_partial_match_opts match_opts;
+  auto group = make_info_sources_options_def_group (&match_opts);
   gdb::option::process_options
     (&args, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_ERROR, group);
 
-  if (args != NULL && *args != '\000')
-    data.regexp = args;
+  if (match_opts.dirname && match_opts.basename)
+    error (_("You cannot give both -basename and -dirname to 'info sources'."));
 
-  data.filename_seen_cache = &filenames_seen;
-  data.first = 1;
+  const char *regex = nullptr;
+  if (args != nullptr && *args != '\000')
+    regex = args;
 
-  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 ((match_opts.dirname || match_opts.basename) && regex == nullptr)
+    error (_("Missing REGEXP for 'info sources'."));
 
-  if (data.regexp.empty ())
-    data.c_regexp.reset ();
+  info_sources_filter::match_on match_type;
+  if (match_opts.dirname)
+    match_type = info_sources_filter::match_on::DIRNAME;
+  else if (match_opts.basename)
+    match_type = info_sources_filter::match_on::BASENAME;
   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"));
-    }
+    match_type = info_sources_filter::match_on::FULLNAME;
+
+  info_sources_filter filter (match_type, regex);
+  output_source_filename_data data (filter);
 
-  print_info_sources_header
-    (_("Source files for which symbols have been read in:\n"), &data);
+  data.print_header (_("Source files for which symbols have been read in:\n"));
 
   for (objfile *objfile : current_program_space->objfiles ())
     {
@@ -4412,11 +4526,9 @@ info_sources_command (const char *args, int from_tty)
     }
   printf_filtered ("\n\n");
 
-  print_info_sources_header
-    (_("Source files for which symbols will be read in on demand:\n"), &data);
+  data.print_header (_("Source files for which symbols will be read in on demand:\n"));
 
-  filenames_seen.clear ();
-  data.first = 1;
+  data.reset_output ();
   map_symbol_filenames (data, true /*need_fullname*/);
   printf_filtered ("\n");
 }
-- 
2.25.4