public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Two quick-function simplifications
@ 2021-03-24 19:38 Tom Tromey
  2021-03-24 19:38 ` [PATCH 1/2] Simplify use of map_matching_symbols in ada-lang.c Tom Tromey
  2021-03-24 19:38 ` [PATCH 2/2] Use function view in quick_symbol_functions::map_symbol_filenames Tom Tromey
  0 siblings, 2 replies; 7+ messages in thread
From: Tom Tromey @ 2021-03-24 19:38 UTC (permalink / raw)
  To: gdb-patches

While working on some patches to simplify quick_symbol_functions, I
came across a couple of things that could be simplified in a
straightforward way.  I'm submitting these separately because they
seem more obviously ok.

Regression tested on x86-64 Fedora 32.

Let me know what you think.

Tom



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] Simplify use of map_matching_symbols in ada-lang.c
  2021-03-24 19:38 [PATCH 0/2] Two quick-function simplifications Tom Tromey
@ 2021-03-24 19:38 ` Tom Tromey
  2021-03-25 19:41   ` Simon Marchi
  2021-03-24 19:38 ` [PATCH 2/2] Use function view in quick_symbol_functions::map_symbol_filenames Tom Tromey
  1 sibling, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2021-03-24 19:38 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

I noticed that ada-lang.c creates a lambda to call
aux_add_nonlocal_symbols.  However, this code can be simplified a bit
by changing match_data to implement operator(), and then simply
passing the object as the callback.  That is what this patch
implements.

2021-03-24  Tom Tromey  <tom@tromey.com>

	* ada-lang.c (struct match_data): Add operator().
	(match_data::operator()): Rename from aux_add_nonlocal_symbols.
	(callback): Remove 'callback'.
---
 gdb/ChangeLog  |  6 ++++++
 gdb/ada-lang.c | 36 ++++++++++++++++--------------------
 2 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index d0374780b98..a7dd88ad86f 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -4970,7 +4970,7 @@ ada_add_local_symbols (std::vector<struct block_symbol> &result,
     add_symbols_from_enclosing_procs (result, lookup_name, domain);
 }
 
-/* An object of this type is used as the user_data argument when
+/* An object of this type is used as the callback argument when
    calling the map_matching_symbols method.  */
 
 struct match_data
@@ -4981,6 +4981,8 @@ struct match_data
   }
   DISABLE_COPY_AND_ASSIGN (match_data);
 
+  bool operator() (struct block_symbol *bsym);
+
   struct objfile *objfile = nullptr;
   std::vector<struct block_symbol> *resultp;
   struct symbol *arg_sym = nullptr;
@@ -4996,33 +4998,32 @@ struct match_data
    marking the end of a block, the argument symbol is added if no
    other has been found.  */
 
-static bool
-aux_add_nonlocal_symbols (struct block_symbol *bsym,
-			  struct match_data *data)
+bool
+match_data::operator() (struct block_symbol *bsym)
 {
   const struct block *block = bsym->block;
   struct symbol *sym = bsym->symbol;
 
   if (sym == NULL)
     {
-      if (!data->found_sym && data->arg_sym != NULL) 
-	add_defn_to_vec (*data->resultp,
-			 fixup_symbol_section (data->arg_sym, data->objfile),
+      if (!found_sym && arg_sym != NULL)
+	add_defn_to_vec (*resultp,
+			 fixup_symbol_section (arg_sym, objfile),
 			 block);
-      data->found_sym = false;
-      data->arg_sym = NULL;
+      found_sym = false;
+      arg_sym = NULL;
     }
   else 
     {
       if (SYMBOL_CLASS (sym) == LOC_UNRESOLVED)
 	return true;
       else if (SYMBOL_IS_ARGUMENT (sym))
-	data->arg_sym = sym;
+	arg_sym = sym;
       else
 	{
-	  data->found_sym = true;
-	  add_defn_to_vec (*data->resultp,
-			   fixup_symbol_section (sym, data->objfile),
+	  found_sym = true;
+	  add_defn_to_vec (*resultp,
+			   fixup_symbol_section (sym, objfile),
 			   block);
 	}
     }
@@ -5194,16 +5195,11 @@ add_nonlocal_symbols (std::vector<struct block_symbol> &result,
 
   bool is_wild_match = lookup_name.ada ().wild_match_p ();
 
-  auto callback = [&] (struct block_symbol *bsym)
-    {
-      return aux_add_nonlocal_symbols (bsym, &data);
-    };
-
   for (objfile *objfile : current_program_space->objfiles ())
     {
       data.objfile = objfile;
 
-      objfile->map_matching_symbols (lookup_name, domain, global, callback,
+      objfile->map_matching_symbols (lookup_name, domain, global, data,
 				     is_wild_match ? NULL : compare_names);
 
       for (compunit_symtab *cu : objfile->compunits ())
@@ -5226,7 +5222,7 @@ add_nonlocal_symbols (std::vector<struct block_symbol> &result,
       for (objfile *objfile : current_program_space->objfiles ())
 	{
 	  data.objfile = objfile;
-	  objfile->map_matching_symbols (name1, domain, global, callback,
+	  objfile->map_matching_symbols (name1, domain, global, data,
 					 compare_names);
 	}
     }      	
-- 
2.26.2


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] Use function view in quick_symbol_functions::map_symbol_filenames
  2021-03-24 19:38 [PATCH 0/2] Two quick-function simplifications Tom Tromey
  2021-03-24 19:38 ` [PATCH 1/2] Simplify use of map_matching_symbols in ada-lang.c Tom Tromey
@ 2021-03-24 19:38 ` Tom Tromey
  2021-03-25 19:49   ` Simon Marchi
  1 sibling, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2021-03-24 19:38 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes quick_symbol_functions::map_symbol_filenames to use a
function_view, and updates all the uses.  It also changes the final
parameter to 'bool'.  A couple of spots are further updated to use
operator() rather than a lambda.

2021-03-24  Tom Tromey  <tom@tromey.com>

	* symtab.c (struct output_source_filename_data): Add 'output'
	method and operator().
	(output_source_filename_data::output): Rename from
	output_source_filename.
	(output_partial_symbol_filename): Remove.
	(info_sources_command): Update.
	(struct add_partial_filename_data): Add operator().
	(add_partial_filename_data::operator()): Rename from
	maybe_add_partial_symtab_filename.
	(make_source_files_completion_list): Update.
	* symfile.c (quick_symbol_functions): Update.
	* symfile-debug.c (objfile::map_symbol_filenames): Update.
	* quick-symbol.h (symbol_filename_ftype): Change type of 'fun' and
	'need_fullname'.  Remove 'data' parameter.
	(struct quick_symbol_functions) <map_symbol_filenames>: Likewise.
	* psymtab.c (psymbol_functions::map_symbol_filenames): Update.
	* psympriv.h (struct psymbol_functions) <map_symbol_filenames>:
	Change type of 'fun' and 'need_fullname'.  Remove 'data'
	parameter.
	* objfiles.h (struct objfile) <map_symbol_filenames>: Change type
	of 'fun' and 'need_fullname'.  Remove 'data' parameter.
	* mi/mi-cmd-file.c (print_partial_file_name): Remove 'ignore'
	parameter.
	(mi_cmd_file_list_exec_source_files): Update.
	* dwarf2/read.c
	(dwarf2_base_index_functions::map_symbol_filenames): Update.
---
 gdb/ChangeLog        | 29 ++++++++++++++++
 gdb/dwarf2/read.c    | 14 ++++----
 gdb/mi/mi-cmd-file.c |  6 ++--
 gdb/objfiles.h       |  4 +--
 gdb/psympriv.h       |  4 +--
 gdb/psymtab.c        | 10 +++---
 gdb/quick-symbol.h   |  9 ++---
 gdb/symfile-debug.c  | 11 +++---
 gdb/symfile.c        |  6 ++--
 gdb/symfile.h        |  4 +--
 gdb/symtab.c         | 80 ++++++++++++++++++++------------------------
 11 files changed, 99 insertions(+), 78 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 2bfb13d6d0e..d6c1e6b33c1 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2230,8 +2230,8 @@ struct dwarf2_base_index_functions : public quick_symbol_functions
   }
 
   void map_symbol_filenames (struct objfile *objfile,
-			     symbol_filename_ftype *fun, void *data,
-			     int need_fullname) override;
+			     gdb::function_view<symbol_filename_ftype> fun,
+			     bool need_fullname) override;
 };
 
 struct dwarf2_gdb_index : public dwarf2_base_index_functions
@@ -4947,10 +4947,10 @@ dwarf2_base_index_functions::find_pc_sect_compunit_symtab
 }
 
 void
-dwarf2_base_index_functions::map_symbol_filenames (struct objfile *objfile,
-						   symbol_filename_ftype *fun,
-						   void *data,
-						   int need_fullname)
+dwarf2_base_index_functions::map_symbol_filenames
+     (struct objfile *objfile,
+      gdb::function_view<symbol_filename_ftype> fun,
+      bool need_fullname)
 {
   dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
 
@@ -5011,7 +5011,7 @@ dwarf2_base_index_functions::map_symbol_filenames (struct objfile *objfile,
 
       if (need_fullname)
 	this_real_name = gdb_realpath (filename);
-      (*fun) (filename, this_real_name.get (), data);
+      fun (filename, this_real_name.get ());
     });
 }
 
diff --git a/gdb/mi/mi-cmd-file.c b/gdb/mi/mi-cmd-file.c
index 55fd94fb9a7..430449c919e 100644
--- a/gdb/mi/mi-cmd-file.c
+++ b/gdb/mi/mi-cmd-file.c
@@ -65,8 +65,7 @@ mi_cmd_file_list_exec_source_file (const char *command, char **argv, int argc)
 /* A callback for map_partial_symbol_filenames.  */
 
 static void
-print_partial_file_name (const char *filename, const char *fullname,
-			 void *ignore)
+print_partial_file_name (const char *filename, const char *fullname)
 {
   struct ui_out *uiout = current_uiout;
 
@@ -108,8 +107,7 @@ mi_cmd_file_list_exec_source_files (const char *command, char **argv, int argc)
 	}
     }
 
-  map_symbol_filenames (print_partial_file_name, NULL,
-			1 /*need_fullname*/);
+  map_symbol_filenames (print_partial_file_name, true /*need_fullname*/);
 
   uiout->end (ui_out_type_list);
 }
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index 41f8fc913d8..4cd392bd7f0 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -590,8 +590,8 @@ struct objfile
      int warn_if_readin);
 
   /* See quick_symbol_functions.  */
-  void map_symbol_filenames (symbol_filename_ftype *fun, void *data,
-			     int need_fullname);
+  void map_symbol_filenames (gdb::function_view<symbol_filename_ftype> fun,
+			     bool need_fullname);
 
   /* See quick_symbol_functions.  */
   struct compunit_symtab *find_compunit_symtab_by_address (CORE_ADDR address);
diff --git a/gdb/psympriv.h b/gdb/psympriv.h
index bbae2fc90e4..193d64fbcf2 100644
--- a/gdb/psympriv.h
+++ b/gdb/psympriv.h
@@ -556,8 +556,8 @@ struct psymbol_functions : public quick_symbol_functions
     (struct objfile *objfile, CORE_ADDR address) override;
 
   void map_symbol_filenames (struct objfile *objfile,
-			     symbol_filename_ftype *fun, void *data,
-			     int need_fullname) override;
+			     gdb::function_view<symbol_filename_ftype> fun,
+			     bool need_fullname) override;
 
   void relocated () override
   {
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 597817269c1..88136a760fb 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -1092,10 +1092,10 @@ psymbol_functions::expand_symtabs_with_fullname (struct objfile *objfile,
    the definition of quick_symbol_functions in symfile.h.  */
 
 void
-psymbol_functions::map_symbol_filenames (struct objfile *objfile,
-					 symbol_filename_ftype *fun,
-					 void *data,
-					 int need_fullname)
+psymbol_functions::map_symbol_filenames
+     (struct objfile *objfile,
+      gdb::function_view<symbol_filename_ftype> fun,
+      bool need_fullname)
 {
   for (partial_symtab *ps : require_partial_symbols (objfile))
     {
@@ -1118,7 +1118,7 @@ psymbol_functions::map_symbol_filenames (struct objfile *objfile,
 	fullname = psymtab_to_fullname (ps);
       else
 	fullname = NULL;
-      (*fun) (ps->filename, fullname, data);
+      fun (ps->filename, fullname);
     }
 }
 
diff --git a/gdb/quick-symbol.h b/gdb/quick-symbol.h
index dd2b896fd5f..b0f8bc3f2b2 100644
--- a/gdb/quick-symbol.h
+++ b/gdb/quick-symbol.h
@@ -28,7 +28,7 @@ typedef int (symbol_compare_ftype) (const char *string1,
 /* Callback for quick_symbol_functions->map_symbol_filenames.  */
 
 typedef void (symbol_filename_ftype) (const char *filename,
-				      const char *fullname, void *data);
+				      const char *fullname);
 
 /* Callback for quick_symbol_functions->expand_symtabs_matching
    to match a file name.  */
@@ -227,9 +227,10 @@ struct quick_symbol_functions
      not already read in.  FUN is the callback.  It is passed the file's
      FILENAME, the file's FULLNAME (if need_fullname is non-zero), and
      the DATA passed to this function.  */
-  virtual void map_symbol_filenames (struct objfile *objfile,
-				     symbol_filename_ftype *fun, void *data,
-				     int need_fullname) = 0;
+  virtual void map_symbol_filenames
+       (struct objfile *objfile,
+	gdb::function_view<symbol_filename_ftype> fun,
+	bool need_fullname) = 0;
 
   /* This is called when the objfile is relocated.  It can be used to
      clean up any internal caches.  */
diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c
index 3daede88292..7136968d450 100644
--- a/gdb/symfile-debug.c
+++ b/gdb/symfile-debug.c
@@ -323,19 +323,18 @@ objfile::find_pc_sect_compunit_symtab (struct bound_minimal_symbol msymbol,
 }
 
 void
-objfile::map_symbol_filenames (symbol_filename_ftype *fun, void *data,
-			       int need_fullname)
+objfile::map_symbol_filenames (gdb::function_view<symbol_filename_ftype> fun,
+			       bool need_fullname)
 {
   if (debug_symfile)
     fprintf_filtered (gdb_stdlog,
-		      "qf->map_symbol_filenames (%s, %s, %s, %d)\n",
+		      "qf->map_symbol_filenames (%s, %s, %d)\n",
 		      objfile_debug_name (this),
-		      host_address_to_string (fun),
-		      host_address_to_string (data),
+		      host_address_to_string (&fun),
 		      need_fullname);
 
   for (const auto &iter : qf)
-    iter->map_symbol_filenames (this, fun, data, need_fullname);
+    iter->map_symbol_filenames (this, fun, need_fullname);
 }
 
 struct compunit_symtab *
diff --git a/gdb/symfile.c b/gdb/symfile.c
index adcdc169306..adb24fb5bd6 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -3739,11 +3739,11 @@ expand_symtabs_matching
    See quick_symbol_functions.map_symbol_filenames for details.  */
 
 void
-map_symbol_filenames (symbol_filename_ftype *fun, void *data,
-		      int need_fullname)
+map_symbol_filenames (gdb::function_view<symbol_filename_ftype> fun,
+		      bool need_fullname)
 {
   for (objfile *objfile : current_program_space->objfiles ())
-    objfile->map_symbol_filenames (fun, data, need_fullname);
+    objfile->map_symbol_filenames (fun, need_fullname);
 }
 
 #if GDB_SELF_TEST
diff --git a/gdb/symfile.h b/gdb/symfile.h
index bae2a798703..bda1c578773 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -328,8 +328,8 @@ void expand_symtabs_matching
    gdb::function_view<expand_symtabs_exp_notify_ftype> expansion_notify,
    enum search_domain kind);
 
-void map_symbol_filenames (symbol_filename_ftype *fun, void *data,
-			   int need_fullname);
+void map_symbol_filenames (gdb::function_view<symbol_filename_ftype> fun,
+			   bool need_fullname);
 
 /* Target-agnostic function to load the sections of an executable into memory.
 
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 084e8ecc2e8..592ac19c09e 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4227,15 +4227,22 @@ struct output_source_filename_data
 
   /* Flag of whether we're printing the first one.  */
   int first;
-};
 
-/* Slave routine for sources_info.  Force line breaks at ,'s.
-   NAME is the name to print.
-   DATA contains the state for printing and watching for duplicates.  */
+  /* Worker for sources_info.  Force line breaks at ,'s.
+     NAME is the name to print.
+     DATA contains the state for printing and watching for duplicates.  */
+  void output (const char *name);
 
-static void
-output_source_filename (const char *name,
-			struct output_source_filename_data *data)
+  /* 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);
+  }
+};
+
+void
+output_source_filename_data::output (const char *name)
 {
   /* Since a single source file can result in several partial symbol
      tables, we need to avoid printing it more than once.  Note: if
@@ -4247,51 +4254,41 @@ output_source_filename (const char *name,
      symtabs; it doesn't hurt to check.  */
 
   /* Was NAME already seen?  */
-  if (data->filename_seen_cache->seen (name))
+  if (filename_seen_cache->seen (name))
     {
       /* Yes; don't print it again.  */
       return;
     }
 
-  /* Does it match data->regexp?  */
-  if (data->c_regexp.has_value ())
+  /* Does it match regexp?  */
+  if (c_regexp.has_value ())
     {
       const char *to_match;
       std::string dirname;
 
-      if (data->partial_match.dirname)
+      if (partial_match.dirname)
 	{
 	  dirname = ldirname (name);
 	  to_match = dirname.c_str ();
 	}
-      else if (data->partial_match.basename)
+      else if (partial_match.basename)
 	to_match = lbasename (name);
       else
 	to_match = name;
 
-      if (data->c_regexp->exec (to_match, 0, NULL, 0) != 0)
+      if (c_regexp->exec (to_match, 0, NULL, 0) != 0)
 	return;
     }
 
   /* Print it and reset *FIRST.  */
-  if (! data->first)
+  if (! first)
     printf_filtered (", ");
-  data->first = 0;
+  first = 0;
 
   wrap_here ("");
   fputs_styled (name, file_name_style.style (), gdb_stdout);
 }
 
-/* A callback for map_partial_symbol_filenames.  */
-
-static void
-output_partial_symbol_filename (const char *filename, const char *fullname,
-				void *data)
-{
-  output_source_filename (fullname ? fullname : filename,
-			  (struct output_source_filename_data *) data);
-}
-
 using isrc_flag_option_def
   = gdb::option::flag_option_def<filename_partial_match_opts>;
 
@@ -4410,7 +4407,7 @@ info_sources_command (const char *args, int from_tty)
 	    {
 	      const char *fullname = symtab_to_fullname (s);
 
-	      output_source_filename (fullname, &data);
+	      data.output (fullname);
 	    }
 	}
     }
@@ -4421,8 +4418,7 @@ info_sources_command (const char *args, int from_tty)
 
   filenames_seen.clear ();
   data.first = 1;
-  map_symbol_filenames (output_partial_symbol_filename, &data,
-			1 /*need_fullname*/);
+  map_symbol_filenames (data, true /*need_fullname*/);
   printf_filtered ("\n");
 }
 
@@ -5959,7 +5955,7 @@ not_interesting_fname (const char *fname)
   return 0;
 }
 
-/* An object of this type is passed as the user_data argument to
+/* An object of this type is passed as the callback argument to
    map_partial_symbol_filenames.  */
 struct add_partial_filename_data
 {
@@ -5968,34 +5964,33 @@ struct add_partial_filename_data
   const char *word;
   int text_len;
   completion_list *list;
+
+  void operator() (const char *filename, const char *fullname);
 };
 
 /* A callback for map_partial_symbol_filenames.  */
 
-static void
-maybe_add_partial_symtab_filename (const char *filename, const char *fullname,
-				   void *user_data)
+void
+add_partial_filename_data::operator() (const char *filename,
+				       const char *fullname)
 {
-  struct add_partial_filename_data *data
-    = (struct add_partial_filename_data *) user_data;
-
   if (not_interesting_fname (filename))
     return;
-  if (!data->filename_seen_cache->seen (filename)
-      && filename_ncmp (filename, data->text, data->text_len) == 0)
+  if (!filename_seen_cache->seen (filename)
+      && filename_ncmp (filename, text, text_len) == 0)
     {
       /* This file matches for a completion; add it to the
 	 current list of matches.  */
-      add_filename_to_list (filename, data->text, data->word, data->list);
+      add_filename_to_list (filename, text, word, list);
     }
   else
     {
       const char *base_name = lbasename (filename);
 
       if (base_name != filename
-	  && !data->filename_seen_cache->seen (base_name)
-	  && filename_ncmp (base_name, data->text, data->text_len) == 0)
-	add_filename_to_list (base_name, data->text, data->word, data->list);
+	  && !filename_seen_cache->seen (base_name)
+	  && filename_ncmp (base_name, text, text_len) == 0)
+	add_filename_to_list (base_name, text, word, list);
     }
 }
 
@@ -6052,8 +6047,7 @@ make_source_files_completion_list (const char *text, const char *word)
   datum.word = word;
   datum.text_len = text_len;
   datum.list = &list;
-  map_symbol_filenames (maybe_add_partial_symtab_filename, &datum,
-			0 /*need_fullname*/);
+  map_symbol_filenames (datum, false /*need_fullname*/);
 
   return list;
 }
-- 
2.26.2


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] Simplify use of map_matching_symbols in ada-lang.c
  2021-03-24 19:38 ` [PATCH 1/2] Simplify use of map_matching_symbols in ada-lang.c Tom Tromey
@ 2021-03-25 19:41   ` Simon Marchi
  2021-03-26 19:40     ` Tom Tromey
  0 siblings, 1 reply; 7+ messages in thread
From: Simon Marchi @ 2021-03-25 19:41 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches



On 2021-03-24 3:38 p.m., Tom Tromey wrote:
> I noticed that ada-lang.c creates a lambda to call
> aux_add_nonlocal_symbols.  However, this code can be simplified a bit
> by changing match_data to implement operator(), and then simply
> passing the object as the callback.  That is what this patch
> implements.
> 
> 2021-03-24  Tom Tromey  <tom@tromey.com>
> 
> 	* ada-lang.c (struct match_data): Add operator().
> 	(match_data::operator()): Rename from aux_add_nonlocal_symbols.
> 	(callback): Remove 'callback'.
> ---
>  gdb/ChangeLog  |  6 ++++++
>  gdb/ada-lang.c | 36 ++++++++++++++++--------------------
>  2 files changed, 22 insertions(+), 20 deletions(-)
> 
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index d0374780b98..a7dd88ad86f 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -4970,7 +4970,7 @@ ada_add_local_symbols (std::vector<struct block_symbol> &result,
>      add_symbols_from_enclosing_procs (result, lookup_name, domain);
>  }
>  
> -/* An object of this type is used as the user_data argument when
> +/* An object of this type is used as the callback argument when
>     calling the map_matching_symbols method.  */
>  
>  struct match_data
> @@ -4981,6 +4981,8 @@ struct match_data
>    }
>    DISABLE_COPY_AND_ASSIGN (match_data);
>  
> +  bool operator() (struct block_symbol *bsym);
> +
>    struct objfile *objfile = nullptr;
>    std::vector<struct block_symbol> *resultp;
>    struct symbol *arg_sym = nullptr;
> @@ -4996,33 +4998,32 @@ struct match_data
>     marking the end of a block, the argument symbol is added if no
>     other has been found.  */
>  
> -static bool
> -aux_add_nonlocal_symbols (struct block_symbol *bsym,
> -			  struct match_data *data)
> +bool
> +match_data::operator() (struct block_symbol *bsym)

The documentation above that needs to be updated.

Otherwise, LGTM.

Simon

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] Use function view in quick_symbol_functions::map_symbol_filenames
  2021-03-24 19:38 ` [PATCH 2/2] Use function view in quick_symbol_functions::map_symbol_filenames Tom Tromey
@ 2021-03-25 19:49   ` Simon Marchi
  2021-03-26 19:43     ` Tom Tromey
  0 siblings, 1 reply; 7+ messages in thread
From: Simon Marchi @ 2021-03-25 19:49 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches

> diff --git a/gdb/quick-symbol.h b/gdb/quick-symbol.h
> index dd2b896fd5f..b0f8bc3f2b2 100644
> --- a/gdb/quick-symbol.h
> +++ b/gdb/quick-symbol.h
> @@ -28,7 +28,7 @@ typedef int (symbol_compare_ftype) (const char *string1,
>  /* Callback for quick_symbol_functions->map_symbol_filenames.  */
>  
>  typedef void (symbol_filename_ftype) (const char *filename,
> -				      const char *fullname, void *data);
> +				      const char *fullname);
>  
>  /* Callback for quick_symbol_functions->expand_symtabs_matching
>     to match a file name.  */
> @@ -227,9 +227,10 @@ struct quick_symbol_functions
>       not already read in.  FUN is the callback.  It is passed the file's
>       FILENAME, the file's FULLNAME (if need_fullname is non-zero), and
>       the DATA passed to this function.  */
> -  virtual void map_symbol_filenames (struct objfile *objfile,
> -				     symbol_filename_ftype *fun, void *data,
> -				     int need_fullname) = 0;
> +  virtual void map_symbol_filenames
> +       (struct objfile *objfile,
> +	gdb::function_view<symbol_filename_ftype> fun,
> +	bool need_fullname) = 0;

The doc would need to be updated.

> diff --git a/gdb/symfile-debug.c b/gdb/symfile-debug.c
> index 3daede88292..7136968d450 100644
> --- a/gdb/symfile-debug.c
> +++ b/gdb/symfile-debug.c
> @@ -323,19 +323,18 @@ objfile::find_pc_sect_compunit_symtab (struct bound_minimal_symbol msymbol,
>  }
>  
>  void
> -objfile::map_symbol_filenames (symbol_filename_ftype *fun, void *data,
> -			       int need_fullname)
> +objfile::map_symbol_filenames (gdb::function_view<symbol_filename_ftype> fun,
> +			       bool need_fullname)
>  {
>    if (debug_symfile)
>      fprintf_filtered (gdb_stdlog,
> -		      "qf->map_symbol_filenames (%s, %s, %s, %d)\n",
> +		      "qf->map_symbol_filenames (%s, %s, %d)\n",
>  		      objfile_debug_name (this),
> -		      host_address_to_string (fun),
> -		      host_address_to_string (data),
> +		      host_address_to_string (&fun),

I'm not sure this is very useful, as it prints a stack address.  I don't
know what else we could do that would be more useful though, so I don't
really mind.  I'm mentioning just in case you have an idea.

> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 084e8ecc2e8..592ac19c09e 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -4227,15 +4227,22 @@ struct output_source_filename_data
>  
>    /* Flag of whether we're printing the first one.  */
>    int first;
> -};
>  
> -/* Slave routine for sources_info.  Force line breaks at ,'s.
> -   NAME is the name to print.
> -   DATA contains the state for printing and watching for duplicates.  */
> +  /* Worker for sources_info.  Force line breaks at ,'s.
> +     NAME is the name to print.
> +     DATA contains the state for printing and watching for duplicates.  *

DATA isn't relevant anymore.

Otherwise, LGTM.

Simon

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] Simplify use of map_matching_symbols in ada-lang.c
  2021-03-25 19:41   ` Simon Marchi
@ 2021-03-26 19:40     ` Tom Tromey
  0 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2021-03-26 19:40 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Tom Tromey, gdb-patches

>> +bool
>> +match_data::operator() (struct block_symbol *bsym)

Simon> The documentation above that needs to be updated.

I did this, and I'm going to check it in soon.

Tom

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] Use function view in quick_symbol_functions::map_symbol_filenames
  2021-03-25 19:49   ` Simon Marchi
@ 2021-03-26 19:43     ` Tom Tromey
  0 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2021-03-26 19:43 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Tom Tromey, gdb-patches

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

>> +  virtual void map_symbol_filenames
>> +       (struct objfile *objfile,
>> +	gdb::function_view<symbol_filename_ftype> fun,
>> +	bool need_fullname) = 0;

Simon> The doc would need to be updated.

Done.

>> objfile_debug_name (this),
>> -		      host_address_to_string (fun),
>> -		      host_address_to_string (data),
>> +		      host_address_to_string (&fun),

Simon> I'm not sure this is very useful, as it prints a stack address.  I don't
Simon> know what else we could do that would be more useful though, so I don't
Simon> really mind.  I'm mentioning just in case you have an idea.

Yeah, I dropped it and just stuck "..." in the output to represent the
un-printed parameter.

>> +  /* Worker for sources_info.  Force line breaks at ,'s.
>> +     NAME is the name to print.
>> +     DATA contains the state for printing and watching for duplicates.  *

Simon> DATA isn't relevant anymore.

Thanks, fixed.
I'm going to check this in.

Tom

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-03-26 19:43 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-24 19:38 [PATCH 0/2] Two quick-function simplifications Tom Tromey
2021-03-24 19:38 ` [PATCH 1/2] Simplify use of map_matching_symbols in ada-lang.c Tom Tromey
2021-03-25 19:41   ` Simon Marchi
2021-03-26 19:40     ` Tom Tromey
2021-03-24 19:38 ` [PATCH 2/2] Use function view in quick_symbol_functions::map_symbol_filenames Tom Tromey
2021-03-25 19:49   ` Simon Marchi
2021-03-26 19:43     ` Tom Tromey

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).