Allow 'ptype/o' for assembly

  PR exp/28359 points out that 'ptype/o' does not work when the current
language is "asm".

I tracked this down to a hard-coded list of languages in typeprint.c.
This patch replaces this list with a method on 'language_defn'
instead.  If all languages are ever updated to have this feature, the
method could be removed; but in the meantime this lets each language
control what happens.

I looked at having each print_type method simply modify the flags
itself, but this doesn't work very well with the feature that disables
method-printing by default (but allows it via a flag).
---
 gdb/c-lang.c                             | 28 ++++++++++++++++++++++++
 gdb/d-lang.c                             |  7 ++++++
 gdb/language.h                           |  7 ++++++
 gdb/objc-lang.c                          |  7 ++++++
 gdb/opencl-lang.c                        |  7 ++++++
 gdb/rust-lang.h                          |  7 ++++++
 gdb/testsuite/gdb.base/ptype-offsets.exp | 13 +++++++++++
 gdb/typeprint.c                          |  4 +---
 8 files changed, 77 insertions(+), 3 deletions(-)
  

On 11/8/22 12:14, Tom Tromey via Gdb-patches wrote:
> PR exp/28359 points out that 'ptype/o' does not work when the current
> language is "asm".
> 

I hesitate to bring this up, but...

Why would the current language matter in this case?
[I understand that the print options are set before
the expression is evaluated...]

Since we have the type of the symbol, couldn't we
simply use that? Maybe it's not worth being so
pedantic?

In any case, it LGTM. I recommend you approve your patch.

Keith
  

Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:

> PR exp/28359 points out that 'ptype/o' does not work when the current
> language is "asm".
>
> I tracked this down to a hard-coded list of languages in typeprint.c.
> This patch replaces this list with a method on 'language_defn'
> instead.  If all languages are ever updated to have this feature, the
> method could be removed; but in the meantime this lets each language
> control what happens.
>
> I looked at having each print_type method simply modify the flags
> itself, but this doesn't work very well with the feature that disables
> method-printing by default (but allows it via a flag).

LGTM.

Thanks,
Andrew

> ---
>  gdb/c-lang.c                             | 28 ++++++++++++++++++++++++
>  gdb/d-lang.c                             |  7 ++++++
>  gdb/language.h                           |  7 ++++++
>  gdb/objc-lang.c                          |  7 ++++++
>  gdb/opencl-lang.c                        |  7 ++++++
>  gdb/rust-lang.h                          |  7 ++++++
>  gdb/testsuite/gdb.base/ptype-offsets.exp | 13 +++++++++++
>  gdb/typeprint.c                          |  4 +---
>  8 files changed, 77 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/c-lang.c b/gdb/c-lang.c
> index 36b4d1ae3dd..e15541f8175 100644
> --- a/gdb/c-lang.c
> +++ b/gdb/c-lang.c
> @@ -816,6 +816,13 @@ class c_language : public language_defn
>  
>    /* See language.h.  */
>  
> +  bool can_print_type_offsets () const override
> +  {
> +    return true;
> +  }
> +
> +  /* See language.h.  */
> +
>    void print_type (struct type *type, const char *varstring,
>  		   struct ui_file *stream, int show, int level,
>  		   const struct type_print_options *flags) const override
> @@ -966,6 +973,13 @@ class cplus_language : public language_defn
>  
>    /* See language.h.  */
>  
> +  bool can_print_type_offsets () const override
> +  {
> +    return true;
> +  }
> +
> +  /* See language.h.  */
> +
>    void print_type (struct type *type, const char *varstring,
>  		   struct ui_file *stream, int show, int level,
>  		   const struct type_print_options *flags) const override
> @@ -1066,6 +1080,13 @@ class asm_language : public language_defn
>  
>    /* See language.h.  */
>  
> +  bool can_print_type_offsets () const override
> +  {
> +    return true;
> +  }
> +
> +  /* See language.h.  */
> +
>    void print_type (struct type *type, const char *varstring,
>  		   struct ui_file *stream, int show, int level,
>  		   const struct type_print_options *flags) const override
> @@ -1118,6 +1139,13 @@ class minimal_language : public language_defn
>  
>    /* See language.h.  */
>  
> +  bool can_print_type_offsets () const override
> +  {
> +    return true;
> +  }
> +
> +  /* See language.h.  */
> +
>    void print_type (struct type *type, const char *varstring,
>  		   struct ui_file *stream, int show, int level,
>  		   const struct type_print_options *flags) const override
> diff --git a/gdb/d-lang.c b/gdb/d-lang.c
> index d9591997c87..bb48af6d7c6 100644
> --- a/gdb/d-lang.c
> +++ b/gdb/d-lang.c
> @@ -144,6 +144,13 @@ class d_language : public language_defn
>  
>    /* See language.h.  */
>  
> +  bool can_print_type_offsets () const override
> +  {
> +    return true;
> +  }
> +
> +  /* See language.h.  */
> +
>    void print_type (struct type *type, const char *varstring,
>  		   struct ui_file *stream, int show, int level,
>  		   const struct type_print_options *flags) const override
> diff --git a/gdb/language.h b/gdb/language.h
> index 871b00b3c94..c1c735ad9ae 100644
> --- a/gdb/language.h
> +++ b/gdb/language.h
> @@ -451,6 +451,13 @@ struct language_defn
>      return nullptr;
>    }
>  
> +  /* Return true if this class' implementation of print_type can
> +     handle the /o modifier.  */
> +  virtual bool can_print_type_offsets () const
> +  {
> +    return false;
> +  }
> +
>    /* Print TYPE to STREAM using syntax appropriate for this language.
>       LEVEL is the depth to indent lines by.  VARSTRING, if not NULL or the
>       empty string, is the name of a variable and TYPE should be printed in
> diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
> index 1a1d6215475..3c2cbf401f0 100644
> --- a/gdb/objc-lang.c
> +++ b/gdb/objc-lang.c
> @@ -266,6 +266,13 @@ class objc_language : public language_defn
>  
>    /* See language.h.  */
>  
> +  bool can_print_type_offsets () const override
> +  {
> +    return true;
> +  }
> +
> +  /* See language.h.  */
> +
>    void print_type (struct type *type, const char *varstring,
>  		   struct ui_file *stream, int show, int level,
>  		   const struct type_print_options *flags) const override
> diff --git a/gdb/opencl-lang.c b/gdb/opencl-lang.c
> index e41db3dcbba..f7cf00c6919 100644
> --- a/gdb/opencl-lang.c
> +++ b/gdb/opencl-lang.c
> @@ -953,6 +953,13 @@ class opencl_language : public language_defn
>  
>    /* See language.h.  */
>  
> +  bool can_print_type_offsets () const override
> +  {
> +    return true;
> +  }
> +
> +  /* See language.h.  */
> +
>    void print_type (struct type *type, const char *varstring,
>  		   struct ui_file *stream, int show, int level,
>  		   const struct type_print_options *flags) const override
> diff --git a/gdb/rust-lang.h b/gdb/rust-lang.h
> index f4c7b7fc1c8..a1d10263feb 100644
> --- a/gdb/rust-lang.h
> +++ b/gdb/rust-lang.h
> @@ -107,6 +107,13 @@ class rust_language : public language_defn
>  
>    /* See language.h.  */
>  
> +  bool can_print_type_offsets () const override
> +  {
> +    return true;
> +  }
> +
> +  /* See language.h.  */
> +
>    void print_type (struct type *type, const char *varstring,
>  		   struct ui_file *stream, int show, int level,
>  		   const struct type_print_options *flags) const override;
> diff --git a/gdb/testsuite/gdb.base/ptype-offsets.exp b/gdb/testsuite/gdb.base/ptype-offsets.exp
> index eb41bafb3e6..b42fa4dac43 100644
> --- a/gdb/testsuite/gdb.base/ptype-offsets.exp
> +++ b/gdb/testsuite/gdb.base/ptype-offsets.exp
> @@ -469,3 +469,16 @@ with_test_prefix "with_hex_default" {
>    # restore
>    gdb_test_no_output "set print type hex off"
>  }
> +
> +gdb_test_no_output "set language asm"
> +gdb_test "ptype/o struct tuv" \
> +    [string_to_regexp [multi_line \
> +  "/* offset      |    size */  type = struct tuv \{" \
> +  "/*      0      |       4 */    int a1;" \
> +  "/* XXX  4-byte hole      */" \
> +  "/*      8      |       8 */    signed char *a2;" \
> +  "/*     16      |       4 */    int a3;" \
> +  "/* XXX  4-byte padding   */" \
> +  "" \
> +  "                               /* total size (bytes):   24 */" \
> +  "                             \}"]]
> diff --git a/gdb/typeprint.c b/gdb/typeprint.c
> index 79043122b17..7a7ac6c314e 100644
> --- a/gdb/typeprint.c
> +++ b/gdb/typeprint.c
> @@ -478,9 +478,7 @@ whatis_exp (const char *exp, int show)
>  		    /* Filter out languages which don't implement the
>  		       feature.  */
>  		    if (show > 0
> -			&& (current_language->la_language == language_c
> -			    || current_language->la_language == language_cplus
> -			    || current_language->la_language == language_rust))
> +			&& current_language->can_print_type_offsets ())
>  		      {
>  			flags.print_offsets = 1;
>  			flags.print_typedefs = 0;
> -- 
> 2.34.3
  

>> PR exp/28359 points out that 'ptype/o' does not work when the current
>> language is "asm".

Keith> I hesitate to bring this up, but...

Keith> Why would the current language matter in this case?
Keith> [I understand that the print options are set before
Keith> the expression is evaluated...]

Keith> Since we have the type of the symbol, couldn't we
Keith> simply use that? Maybe it's not worth being so
Keith> pedantic?

Ordinarily this would be fine, but for non-C languages it's sometimes
useful to "set lang c" and see what the C type-printer thinks.  For
example, in Ada there are "fat pointers" that are really just a kind of
struct.  Ada will always print these as ordinary access types, but it
can be handy to look under the hood and see that it's really a struct
type.

Maybe there's some better way to do this kind of thing.  Like we could
add "ptype/c" for "C style printing".

Keith> In any case, it LGTM. I recommend you approve your patch.

Thanks.

Tom
  

On Wed, 2022-11-09 at 08:28 -0700, Tom Tromey via Gdb-patches wrote:
> > > PR exp/28359 points out that 'ptype/o' does not work when the current
> > > language is "asm".
> 
> Keith> I hesitate to bring this up, but...
> 
> Keith> Why would the current language matter in this case?
> Keith> [I understand that the print options are set before
> Keith> the expression is evaluated...]
> 
> Keith> Since we have the type of the symbol, couldn't we
> Keith> simply use that? Maybe it's not worth being so
> Keith> pedantic?
> 
> Ordinarily this would be fine, but for non-C languages it's sometimes
> useful to "set lang c" and see what the C type-printer thinks.  For
> example, in Ada there are "fat pointers" that are really just a kind of
> struct.  Ada will always print these as ordinary access types, but it
> can be handy to look under the hood and see that it's really a struct
> type.
> 
> Maybe there's some better way to do this kind of thing.  Like we could
> add "ptype/c" for "C style printing".
At work, I relatively often use ptype/o for Ada types.
As it only works in C mode, as part of our zillions of site aliases,
we have added:
  alias Wc = with language c -- with annotate 0 --
(where annotate 0 avoids spurious language switch messages in the output).

So, waiting for Adacore to implement ptype/o for Ada (hint hint :)),
we use e.g.  
  (gdb) Wc ptype/o sometype

Of course, as the type will be searched in C mode, we have to use the
"underlying" type name e.g.   This_Is_A_Package__This_Is_The_Ada_Type
instead of This_Is_A_Package.This_Is_The_Ada_Type

Of course, Wc will work with whatever you want to do with C language temporarily set.
(thanks again to Pedro for the "with" implementation).

Thanks
Philippe
  

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> At work, I relatively often use ptype/o for Ada types.
Philippe> As it only works in C mode, as part of our zillions of site aliases,
Philippe> we have added:
Philippe>   alias Wc = with language c -- with annotate 0 --
Philippe> (where annotate 0 avoids spurious language switch messages in the output).

Philippe> So, waiting for Adacore to implement ptype/o for Ada (hint hint :)),
Philippe> we use e.g.  
Philippe>   (gdb) Wc ptype/o sometype

I've considered it, and maybe the basics aren't too hard to do, but I
didn't really know what to do with types that have a dynamic layout.
Here the offsets would maybe have to be symbolic (which I'd suppose is a
pain to implement) and holes couldn't really be determined.

Maybe gdb could just print a warning in this scenario and drop the '/o'
modifier.  That might be easier to do.

Tom
  

On Mon, 2022-11-14 at 06:32 -0700, Tom Tromey wrote:
> > > > > > "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
> 
> Philippe> At work, I relatively often use ptype/o for Ada types.
> Philippe> As it only works in C mode, as part of our zillions of site aliases,
> Philippe> we have added:
> Philippe>   alias Wc = with language c -- with annotate 0 --
> Philippe> (where annotate 0 avoids spurious language switch messages in the output).
> 
> Philippe> So, waiting for Adacore to implement ptype/o for Ada (hint hint :)),
> Philippe> we use e.g.  
> Philippe>   (gdb) Wc ptype/o sometype
> 
> I've considered it, and maybe the basics aren't too hard to do, but I
> didn't really know what to do with types that have a dynamic layout.
> Here the offsets would maybe have to be symbolic (which I'd suppose is a
> pain to implement) and holes couldn't really be determined.
Effectively, Ada types can have a very rich/sophisticated layout.
When I really need to have a in depth look at a complex type representation,
the compiler option -gnatR3 produces this (including the symbolic offsets
where needed, and "formulas" to compute offsets).
> 
> Maybe gdb could just print a warning in this scenario and drop the '/o'
> modifier.  That might be easier to do.
Currently, ptype /o an_ada_type just prints the type, without the offsets.
I have no problem with this behaviour, but producing a warning that /o
is ignored in Ada mode will not harm).

Thanks
Philippe