[review v7] gdb/mi: Add -symbol-info-module-{variables,functions}

["Simon Marchi (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>]

Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/268
......................................................................


Patch Set 7:

(2 comments)

| --- gdb/mi/mi-symbol-cmds.c
| +++ gdb/mi/mi-symbol-cmds.c
| @@ -219,0 +268,34 @@ mi_info_module_functions_or_variables (enum search_domain kind,
| +	}
| +    }
| +
| +  std::vector<module_symbol_search> module_symbols
| +    = search_module_symbols (module_regexp, regexp, type_regexp, kind);
| +
| +  struct ui_out *uiout = current_uiout;
| +  ui_out_emit_list all_matching_symbols (uiout, "symbols");
| +
| +  for (int i = 0; i < module_symbols.size (); )

PS7, Line 277:

I would appreciate a few comments in this function, that explains what
each step does, at a high level.  Otherwise I'm not sure of what's the
intention, and therefore if the code what you intended it to do.

| +    {
| +      gdb_assert (module_symbols[i].second.symbol != nullptr);
| +      const symbol *mod_sym = module_symbols[i].first.symbol;
| +      const symbol *first_mod_sym = mod_sym;
| +
| +      ui_out_emit_tuple module_tuple (uiout, nullptr);
| +      uiout->field_string ("module", mod_sym->print_name ());
| +
| +      for (; (i < module_symbols.size ()
| +	      && first_mod_sym == module_symbols[i].first.symbol);)
| +	{
| +	  ui_out_emit_list files_list (uiout, "files");
| +
| +	  for (; (i < module_symbols.size ()
| +		  && first_mod_sym == module_symbols[i].first.symbol);)

PS7, Line 292:

For example, this for loop is identical to the previous for loop.  I
find it a bit odd, but maybe that's intended?

| +	    {
| +	      ui_out_emit_tuple current_file (uiout, nullptr);
| +	      const symbol_search &q = module_symbols[i].second;
| +	      struct symtab *s = symbol_symtab (q.symbol);
| +	      uiout->field_string ("filename",
| +				   symtab_to_filename_for_display (s));
| +	      uiout->field_string ("fullname", symtab_to_fullname (s));
| +	      ui_out_emit_list item_list (uiout, "symbols");
| +	      const symtab *last_symtab = symbol_symtab (q.symbol);

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ic96f12dd14bd7e34774c3cde008fec30a4055bfe
Gerrit-Change-Number: 268
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-CC: Joel Brobecker <brobecker@adacore.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Wed, 27 Nov 2019 04:21:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment