[RFAv2,1/3] Use function_name_style to print Ada and C function names

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

Commit Message

Philippe Waroquiers Jan. 12, 2019, 10:28 p.m. UTC
  Note that ada-typeprint.c print_func_type is called with
types representing functions and is also called to print
a function NAME and its type.  In such a case, the function
name will be printed using function name style.

Similarly, c_print_type_1 is called to print a type, optionally
with the name of an object of this type in the VARSTRING arg.
So, c_print_type_1 uses function name style to print varstring
when the type code indicates that c_print_type_1 TYPE is some
'real code'.

gdb/ChangeLog
2019-01-12  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* ada-typeprint.c (print_func_type): Print function name
	style to print function name.
	* c-typeprint.c (c_print_type_1): Likewise.
---
 gdb/ada-typeprint.c | 6 +++++-
 gdb/c-typeprint.c   | 8 ++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)
  

Comments

Tom Tromey Jan. 17, 2019, 10:21 p.m. UTC | #1
>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> Note that ada-typeprint.c print_func_type is called with
Philippe> types representing functions and is also called to print
Philippe> a function NAME and its type.  In such a case, the function
Philippe> name will be printed using function name style.

In this particular spot it still isn't clear to me if this will
sometimes style a type name.  So, I'm going to defer to Joel on the Ada
bits.

I'm actually in favor of styling types, but I think they should have a
separate setting from functions.

Philippe> 	* ada-typeprint.c (print_func_type): Print function name
Philippe> 	style to print function name.
Philippe> 	* c-typeprint.c (c_print_type_1): Likewise.

The c-typeprint.c change is ok.  I re-read your last note on the other
thread and I think I finally understand.

Tom
  
Philippe Waroquiers Jan. 19, 2019, 12:11 p.m. UTC | #2
Thanks for the review.

On Thu, 2019-01-17 at 15:21 -0700, Tom Tromey wrote:
> > > > > > "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
> 
> Philippe> Note that ada-typeprint.c print_func_type is called with
> Philippe> types representing functions and is also called to print
> Philippe> a function NAME and its type.  In such a case, the function
> Philippe> name will be printed using function name style.
> 
> In this particular spot it still isn't clear to me if this will
> sometimes style a type name.  So, I'm going to defer to Joel on the Ada
> bits.

Would be nice to have Joel confirming, but I am quite confident that
this will only style function names and not type names.

If that can help, here is some more detailed investigations:
First, in Ada, after the keywords procedure/function, and before
the arg list (as printed by print_func_type), you must put
a procedure/function name, no type name can be put there,
so GDB would produce real strange/invalid Ada such as:
   procedure Some_Ada_Type_Name (Some_Arg : ...);
This explanation matches the comment of print_func_type:
/* Print function or procedure type TYPE on STREAM.  Make it a header
   for function or procedure NAME if NAME is not null.  */

Also, the type name of an Ada type is accessed via ada_type_name,
that accesses a member of TYPE containing the type name.

To the contrary: the NAME arg of print_func_type is received from
ada_print_type (the single caller of print_func_type),
that passes as NAME its VARSTRING arg.

There are only a few places where ada_print_type is called
with a non null VARSTRING arg, and these are for the names
of fields of record/unchecked unions.

ada_print_type is called (indirectly) by all calls to
print_type (dispatching on the language).
Doing a grep, I found only symtab.c:print_symbol_info
that calls type_print with a non
NULL/non "" VARSTRING arg,
and this piece of code gives "" when the symbol
is a TYPEDEF.

So, I really do not see how that piece of code could style
something else than a function/procedure name.

Thanks

Philippe
  
Joel Brobecker Jan. 26, 2019, 6:21 a.m. UTC | #3
> > Philippe> Note that ada-typeprint.c print_func_type is called with
> > Philippe> types representing functions and is also called to print
> > Philippe> a function NAME and its type.  In such a case, the function
> > Philippe> name will be printed using function name style.
> > 
> > In this particular spot it still isn't clear to me if this will
> > sometimes style a type name.  So, I'm going to defer to Joel on the Ada
> > bits.
> 
> Would be nice to have Joel confirming, but I am quite confident that
> this will only style function names and not type names.

I reviewed the situation, and I think Philippe is right in the sense
that when this function is called with a name, it's an actual function's
name.

When one declares a function type in Ada, it has to be an access
type (the Ada equivalent of a pointer). And the target (function)
declaration is anonymous. This is what it looks like:

   type FA is access procedure (A : System.Address);

So, while the access type has a name ("FA"), the function type
itself does not. From there, the only way I could see that
the function might be called with a name is if it is given
a symbol which is a procedure (not an access to that symbol).
In that case, it would be legitimate to stylize the name as
a function name. But in practice, it's hard to make GDB call
that function with a name, because you can't really put
the function name within an Ada expression without taking
it's address (in Ada, this is a parameterless function call).

The only way I could find that allowed me to trigger this function
call with a non-NULL name was with "maintenance print symbols FILENAME".
In that situation, we're back to the reasoning above that function
types have no name unless they are tied to a real function, and
thus "name" seems to always represent a function name.

With all that, I'm wondering if Philippe had other examples
where he can demonstrate the usefulness of this patch.
At the moment, if the maintenance command is the only case,
knowing that the output gets sent to a file, rather than
stdout, and thus should not be stylized, are there other
situations I couldn't think of where this patch would be
useful?
  
Philippe Waroquiers Jan. 26, 2019, 11:04 a.m. UTC | #4
On Sat, 2019-01-26 at 10:21 +0400, Joel Brobecker wrote:
> > > Philippe> Note that ada-typeprint.c print_func_type is called with
> > > Philippe> types representing functions and is also called to print
> > > Philippe> a function NAME and its type.  In such a case, the function
> > > Philippe> name will be printed using function name style.
> > > 
> > > In this particular spot it still isn't clear to me if this will
> > > sometimes style a type name.  So, I'm going to defer to Joel on the Ada
> > > bits.
> > 
> > Would be nice to have Joel confirming, but I am quite confident that
> > this will only style function names and not type names.
> 
> I reviewed the situation, and I think Philippe is right in the sense
> that when this function is called with a name, it's an actual function's
> name.
> 
> When one declares a function type in Ada, it has to be an access
> type (the Ada equivalent of a pointer). And the target (function)
> declaration is anonymous. This is what it looks like:
> 
>    type FA is access procedure (A : System.Address);
Yes, effectively, that is the reasoning.

> With all that, I'm wondering if Philippe had other examples
> where he can demonstrate the usefulness of this patch.
> At the moment, if the maintenance command is the only case,
> knowing that the output gets sent to a file, rather than
> stdout, and thus should not be stylized, are there other
> situations I couldn't think of where this patch would be
> useful?

This part of the patch ensures that the function names in the output of
  'info functions [-q] [-t TYPEREGEXP] [NAMEREGEXP]'
are stylized.

Thanks

Philippe
  
Philippe Waroquiers Jan. 29, 2019, 8:34 p.m. UTC | #5
On Sat, 2019-01-26 at 12:04 +0100, Philippe Waroquiers wrote:
> > With all that, I'm wondering if Philippe had other examples
> > where he can demonstrate the usefulness of this patch.
> > At the moment, if the maintenance command is the only case,
> > knowing that the output gets sent to a file, rather than
> > stdout, and thus should not be stylized, are there other
> > situations I couldn't think of where this patch would be
> > useful?
> 
> This part of the patch ensures that the function names in the output of
>   'info functions [-q] [-t TYPEREGEXP] [NAMEREGEXP]'
> are stylized.
Any additional comments ?
Ok to push ?

Thanks

Philippe
  

Patch

diff --git a/gdb/ada-typeprint.c b/gdb/ada-typeprint.c
index 2b6cdaf264..8c42e8140d 100644
--- a/gdb/ada-typeprint.c
+++ b/gdb/ada-typeprint.c
@@ -30,6 +30,7 @@ 
 #include "language.h"
 #include "demangle.h"
 #include "c-lang.h"
+#include "cli/cli-style.h"
 #include "typeprint.h"
 #include "target-float.h"
 #include "ada-lang.h"
@@ -779,7 +780,10 @@  print_func_type (struct type *type, struct ui_file *stream, const char *name,
     fprintf_filtered (stream, "function");
 
   if (name != NULL && name[0] != '\0')
-    fprintf_filtered (stream, " %s", name);
+    {
+      fputs_filtered (" ", stream);
+      fputs_styled (name, function_name_style.style (), stream);
+    }
 
   if (len > 0)
     {
diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index 5e7e672e02..6690ca53bc 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -28,6 +28,7 @@ 
 #include "language.h"
 #include "demangle.h"
 #include "c-lang.h"
+#include "cli/cli-style.h"
 #include "typeprint.h"
 #include "cp-abi.h"
 #include "cp-support.h"
@@ -115,6 +116,7 @@  c_print_type_1 (struct type *type,
     type = check_typedef (type);
 
   local_name = typedef_hash_table::find_typedef (flags, type);
+  code = TYPE_CODE (type);
   if (local_name != NULL)
     {
       fputs_filtered (local_name, stream);
@@ -124,7 +126,6 @@  c_print_type_1 (struct type *type,
   else
     {
       c_type_print_base_1 (type, stream, show, level, language, flags, podata);
-      code = TYPE_CODE (type);
       if ((varstring != NULL && *varstring != '\0')
 	  /* Need a space if going to print stars or brackets;
 	     but not if we will print just a type name.  */
@@ -144,7 +145,10 @@  c_print_type_1 (struct type *type,
 
   if (varstring != NULL)
     {
-      fputs_filtered (varstring, stream);
+      if (code == TYPE_CODE_FUNC || code == TYPE_CODE_METHOD)
+	fputs_styled (varstring, function_name_style.style (), stream);
+      else
+	fputs_filtered (varstring, stream);
 
       /* For demangled function names, we have the arglist as part of
          the name, so don't print an additional pair of ()'s.  */