[RFA,2/5] Use scoped_switch_auto_to_sym_language in symtab.c to switch language.

Message ID 20181028144614.14149-3-philippe.waroquiers@skynet.be
State New, archived
Headers

Commit Message

Philippe Waroquiers Oct. 28, 2018, 2:46 p.m. UTC
  Use scoped_switch_auto_to_sym_language in treg_matches_sym_type_name to
replace the local logic that was doing the same as the new class
scoped_switch_auto_to_sym_language.

Use scoped_switch_auto_to_sym_language inside print_symbol_info, so
that symbol information is printed in the symbol language when
language mode is auto.
This modifies the behaviour of the test dw2-case-insensitive.exp,
as the function FUNC_lang is now printed with the Fortran syntax
(as declared in the .S file).

gdb/ChangeLog
2018-10-27  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* symtab.c (treg_matches_sym_type_name): Use
	scoped_switch_auto_to_sym_language instead of local logic.
	(print_symbol_info): Use scoped_switch_auto_to_sym_language
	to switch to SYM language when language mode is auto.

gdb/testsuite/ChangeLOg
2018-10-27  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.dwarf2/dw2-case-insensitive.exp: Update due to auto switch to
	FUNC_lang language syntax.
---
 gdb/symtab.c                                      | 12 +++++-------
 gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.exp |  4 +++-
 2 files changed, 8 insertions(+), 8 deletions(-)
  

Comments

Pedro Alves Nov. 16, 2018, 6:28 p.m. UTC | #1
On 10/28/2018 02:46 PM, Philippe Waroquiers wrote:
> Use scoped_switch_auto_to_sym_language in treg_matches_sym_type_name to
> replace the local logic that was doing the same as the new class
> scoped_switch_auto_to_sym_language.
> 
> Use scoped_switch_auto_to_sym_language inside print_symbol_info, so
> that symbol information is printed in the symbol language when
> language mode is auto.
> This modifies the behaviour of the test dw2-case-insensitive.exp,
> as the function FUNC_lang is now printed with the Fortran syntax
> (as declared in the .S file).
> 

> +
>  
>    if (symbol_lookup_debug > 1)
>      {
> @@ -4601,6 +4598,7 @@ print_symbol_info (enum search_domain kind,
>  		   struct symbol *sym,
>  		   int block, const char *last)
>  {
> +  scoped_switch_auto_to_sym_language l (sym);

Sounds unnecessarily inefficient to use the scoped switch here
instead of at the caller to avoid the back and forth for each
symbol, but probably this isn't in any hot path, so it's fine.

>    struct symtab *s = symbol_symtab (sym);
>  
>    if (last != NULL)
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.exp b/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.exp
> index b15dcafa00..2e3397865a 100644
> --- a/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.exp
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.exp
> @@ -42,8 +42,10 @@ gdb_test "info functions fUnC_lang" \
>  gdb_test "set case-sensitive off" {warning: the current case sensitivity setting does not match the language\.}
>  
>  # The dot-leading symbol is for ppc64 function descriptors.
> +# Note that info functions gives the FUNC_lang result using the fortran syntax
> +# as specific in dw-case-insensitive-debug.S DW_AT_language.
>  gdb_test "info functions fUnC_lang" \
> -	 "All functions matching regular expression \"fUnC_lang\":\[\r\n\]+File file1.txt:\r\n\tfoo FUNC_lang\\(void\\);(\r\n\r\nNon-debugging symbols:\r\n0x\[0-9a-f\]+ +\\.FUNC_lang)?" \
> +	 "All functions matching regular expression \"fUnC_lang\":\[\r\n\]+File file1.txt:\r\n\tfoo FUNC_lang\\(\\);(\r\n\r\nNon-debugging symbols:\r\n0x\[0-9a-f\]+ +\\.FUNC_lang)?" \
>  	 "regexp case-sensitive off"

OOC, the actual name & type matching respect the symbol's language, right?

Patch is OK.

Thanks,
Pedro Alves
  
Philippe Waroquiers Nov. 17, 2018, 12:24 p.m. UTC | #2
On Fri, 2018-11-16 at 18:28 +0000, Pedro Alves wrote:
> On 10/28/2018 02:46 PM, Philippe Waroquiers wrote:
> > 
> >    if (symbol_lookup_debug > 1)
> >      {
> > @@ -4601,6 +4598,7 @@ print_symbol_info (enum search_domain kind,
> >  		   struct symbol *sym,
> >  		   int block, const char *last)
> >  {
> > +  scoped_switch_auto_to_sym_language l (sym);
> 
> Sounds unnecessarily inefficient to use the scoped switch here
> instead of at the caller to avoid the back and forth for each
> symbol, but probably this isn't in any hot path, so it's fine.
Yes, I do not think this is performance critical,
and also, switching of language is not very expensive :
it implies to construct/destroy scoped_switch_auto_to_sym_language l,
which do calls to functions setting enumeration values for
the language and case settings.
 
> 
> >    struct symtab *s = symbol_symtab (sym);
> >  
> >    if (last != NULL)
> > diff --git a/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.exp b/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.exp
> > index b15dcafa00..2e3397865a 100644
> > --- a/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.exp
> > +++ b/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.exp
> > @@ -42,8 +42,10 @@ gdb_test "info functions fUnC_lang" \
> >  gdb_test "set case-sensitive off" {warning: the current case sensitivity setting does not match the language\.}
> >  
> >  # The dot-leading symbol is for ppc64 function descriptors.
> > +# Note that info functions gives the FUNC_lang result using the fortran syntax
> > +# as specific in dw-case-insensitive-debug.S DW_AT_language.
> >  gdb_test "info functions fUnC_lang" \
> > -	 "All functions matching regular expression \"fUnC_lang\":\[\r\n\]+File file1.txt:\r\n\tfoo FUNC_lang\\(void\\);(\r\n\r\nNon-debugging symbols:\r\n0x\[0-9a-f\]+ +\\.FUNC_lang)?" \
> > +	 "All functions matching regular expression \"fUnC_lang\":\[\r\n\]+File file1.txt:\r\n\tfoo FUNC_lang\\(\\);(\r\n\r\nNon-debugging symbols:\r\n0x\[0-9a-f\]+ +\\.FUNC_lang)?" \
> >  	 "regexp case-sensitive off"
> 
> OOC, the actual name & type matching respect the symbol's language, right?
Yes, effectively, the previous patch series that introduced
e.g. 'info function -t TYPEREGEXP' was already switching to the entity language
to do the matching of the type.

The only somewhat (strange ? non intuitive ?) behavior remaining is that the
entity names are printed (and matched) not according to the language setting,
but according to the 'set print demangle' setting.

For example, in the below, you see that with set lang c, the var name
is still printed with a . after global_pack, while intuitively, one
would have expected the 'raw' name with __. The __ are shown
when demangling is off.
 
  (gdb) set lang c
  Warning: the current language does not match this frame.
  (gdb) info var some_struct_in_ada
  All variables matching regular expression "some_struct_in_ada":

  File /bd/home/philippe/gdb/git/info_t/gdb/testsuite/gdb.ada/info_auto_lang/global_pack.ads:
  24:	struct global_pack__some_type_in_ada global_pack.some_struct_in_ada;
  (gdb) set print demangle off
  (gdb) info var some_struct_in_ada
  All variables matching regular expression "some_struct_in_ada":

  File /bd/home/philippe/gdb/git/info_t/gdb/testsuite/gdb.ada/info_auto_lang/global_pack.ads:
  24:	struct global_pack__some_type_in_ada global_pack__some_struct_in_ada;
  (gdb) 

I guess that changing this would mean to have demangling be driven by language
setting (not too sure how the demangling logic works now).


Thanks for the review, I think I handled all your comments/suggestions
in RFAv2.

Philippe
  

Patch

diff --git a/gdb/symtab.c b/gdb/symtab.c
index cd27a75e8c..0f9d562c84 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4288,15 +4288,12 @@  treg_matches_sym_type_name (const compiled_regex &treg,
   if (sym_type == NULL)
     return false;
 
-  if (language_mode == language_mode_auto)
-    {
-      scoped_restore_current_language l;
+  {
+    scoped_switch_auto_to_sym_language l (sym);
 
-      set_language (SYMBOL_LANGUAGE (sym));
-      printed_sym_type_name = type_to_string (sym_type);
-    }
-  else
     printed_sym_type_name = type_to_string (sym_type);
+  }
+
 
   if (symbol_lookup_debug > 1)
     {
@@ -4601,6 +4598,7 @@  print_symbol_info (enum search_domain kind,
 		   struct symbol *sym,
 		   int block, const char *last)
 {
+  scoped_switch_auto_to_sym_language l (sym);
   struct symtab *s = symbol_symtab (sym);
 
   if (last != NULL)
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.exp b/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.exp
index b15dcafa00..2e3397865a 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-case-insensitive.exp
@@ -42,8 +42,10 @@  gdb_test "info functions fUnC_lang" \
 gdb_test "set case-sensitive off" {warning: the current case sensitivity setting does not match the language\.}
 
 # The dot-leading symbol is for ppc64 function descriptors.
+# Note that info functions gives the FUNC_lang result using the fortran syntax
+# as specific in dw-case-insensitive-debug.S DW_AT_language.
 gdb_test "info functions fUnC_lang" \
-	 "All functions matching regular expression \"fUnC_lang\":\[\r\n\]+File file1.txt:\r\n\tfoo FUNC_lang\\(void\\);(\r\n\r\nNon-debugging symbols:\r\n0x\[0-9a-f\]+ +\\.FUNC_lang)?" \
+	 "All functions matching regular expression \"fUnC_lang\":\[\r\n\]+File file1.txt:\r\n\tfoo FUNC_lang\\(\\);(\r\n\r\nNon-debugging symbols:\r\n0x\[0-9a-f\]+ +\\.FUNC_lang)?" \
 	 "regexp case-sensitive off"
 
 gdb_test "p fuNC_lang" { = {foo \(void\)} 0x[0-9a-f]+ <FUNC_lang>}