Re: [PATCH 1/1] gdb: Print cv qualifiers if class attributes are substituted

[Andrew Burgess <aburgess@redhat.com>]

* Christina Schimpe via Gdb-patches <gdb-patches@sourceware.org> [2021-11-04 14:30:48 +0100]:

> Make ptype print const/volatile qualifiers when template or typedef
> attributes are substituted.
> 
> For a programm like
> ~~~
> template<typename DataT>
> class Cfoo
> {
>   typedef float myfloat;
> public:
>   DataT me0;
>   const DataT me1=1;
>   const myfloat me2=2.0;
> };
> 
> int main()
> {
>   Cfoo<int> cfoo;
>   return 0;
> }
> ~~~
> 
> gdb outputs the following type for cfoo's attributes:
> 
> ~~~
> (gdb) b 14
> Breakpoint 1 at 0x1170: file tmp.cc, line 14.
> (gdb) run
> Starting program: /tmp
> 
> Breakpoint 1, main () at tmp.cc:14
> 14        return 0;
> (gdb) ptype cfoo
> type = class Cfoo<int> [with DataT = int] {
>   public:
>     DataT me0;
>     DataT me1;
>     myfloat me2;
> 
>   private:
>     typedef float myfloat;
> }
> 
> ~~~
> 
> The cv qualifiers (const in this case) are ignored for me1 and me2.
> 
> After:
> ~~~
> (gdb) ptype cfoo
> type = class Cfoo<int> [with DataT = int] {
>   public:
>     DataT me0;
>     const DataT me1;
>     const myfloat me2;
> 
>   private:
>     typedef float myfloat;
> }
> ~~~
> 
> gdb/ChangeLog:
> 2021-10-25  Christina Schimpe  <christina.schimpe@intel.com>
> 
> 	* gdb/c-typeprint.c: Print cv qualifiers in case of parameter
> 	  substitution.
> 
> gdb/testsuite/ChangeLog:
> 2021-10-25  Christina Schimpe  <christina.schimpe@intel.com>
> 
> 	* gdb.cp/ptype-cv-cp.cc: New const and volatile typdef
> 	  variables.
> 	* gdb.cp/ptype-cv-cp.exp: Add new tests for new variables.
> 	* gdb.cp/templates.cc: 	New template class Cfoo with const,
> 	  template, typdef and integer attributes.
> 	* gdb.cp/templates.exp: Add new test using ptype and ptype/r
> 	  commmands for template class CFoo.
> 
> Signed-off-by: Christina Schimpe <christina.schimpe@intel.com>
> ---
>  gdb/c-typeprint.c                    |  1 +
>  gdb/testsuite/gdb.cp/ptype-cv-cp.cc  |  8 ++++--
>  gdb/testsuite/gdb.cp/ptype-cv-cp.exp |  6 ++++
>  gdb/testsuite/gdb.cp/templates.cc    | 14 +++++++++
>  gdb/testsuite/gdb.cp/templates.exp   | 43 ++++++++++++++++++++++++++--
>  5 files changed, 66 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
> index 5f20233c78..a6228248e9 100644
> --- a/gdb/c-typeprint.c
> +++ b/gdb/c-typeprint.c
> @@ -119,6 +119,7 @@ c_print_type_1 (struct type *type,
>    code = type->code ();
>    if (local_name != NULL)
>      {
> +      c_type_print_modifier (type, stream, 0, 1, language);
>        fputs_filtered (local_name, stream);
>        if (varstring != NULL && *varstring != '\0')
>  	fputs_filtered (" ", stream);
> diff --git a/gdb/testsuite/gdb.cp/ptype-cv-cp.cc b/gdb/testsuite/gdb.cp/ptype-cv-cp.cc
> index 9560dfa80b..0c5e10cd84 100644
> --- a/gdb/testsuite/gdb.cp/ptype-cv-cp.cc
> +++ b/gdb/testsuite/gdb.cp/ptype-cv-cp.cc
> @@ -23,9 +23,11 @@ typedef const volatile_my_int const_volatile_my_int;
>  
>  my_int v_my_int (0);
>  __attribute__((used)) const_my_int v_const_my_int (1);
> -volatile_my_int v_volatile_my_int (2);
> -const_volatile_my_int v_const_volatile_my_int (3);
> -volatile_const_my_int v_volatile_const_my_int (4);
> +__attribute__((used)) const my_int v2_const_my_int (2);
> +volatile_my_int v_volatile_my_int (3);
> +volatile my_int v2_volatile_my_int (4);
> +const_volatile_my_int v_const_volatile_my_int (5);
> +volatile_const_my_int v_volatile_const_my_int (6);
>  
>  int
>  main ()

All of these tests pass both before and after this patch, and I don't
think these pass through the code you modified.

I'm happy for these tests to go in, but I think they should be in a
separate commit to make it clear that they are not directly related to
the change in this commit.

It also seemed odd that you'd not covered:

  const volatile my_int v_const_volatile_my_int (7);
  volatile const my_int v_volatile_const_my_int (8);

> diff --git a/gdb/testsuite/gdb.cp/ptype-cv-cp.exp b/gdb/testsuite/gdb.cp/ptype-cv-cp.exp
> index b3574cbd34..8214e89350 100644
> --- a/gdb/testsuite/gdb.cp/ptype-cv-cp.exp
> +++ b/gdb/testsuite/gdb.cp/ptype-cv-cp.exp
> @@ -41,3 +41,9 @@ if {[test_compiler_info {gcc-[0-3]-*}]
>      setup_xfail "gcc/45997" "*-*-*"
>  }
>  gdb_test "ptype v_volatile_const_my_int" "type = const volatile int"
> +
> +gdb_test "ptype v2_const_my_int" "type = const int"
> +gdb_test "whatis v2_const_my_int" "type = const my_int"
> +
> +gdb_test "ptype v2_volatile_my_int" "type = volatile int"
> +gdb_test "whatis v2_volatile_my_int" "type = volatile my_int"
> diff --git a/gdb/testsuite/gdb.cp/templates.cc b/gdb/testsuite/gdb.cp/templates.cc
> index 49cf6b66ef..5e418ed272 100644
> --- a/gdb/testsuite/gdb.cp/templates.cc
> +++ b/gdb/testsuite/gdb.cp/templates.cc
> @@ -690,6 +690,18 @@ int gf2 (int a) {
>  
>  char string[3];
>  
> +// Template class with typdefs and const attributes

Comments should end with '.' unless the test requires otherwise.

> +template<typename DataT>
> +class Cfoo
> +{
> +  typedef float myfloat;
> +public:
> +  DataT me0;
> +  const DataT me1=1;
> +  const myfloat me2=2.0;
> +  const int me3=0;
> +};
> +
>  
>  // Template for nested instantiations

And here.

>  
> @@ -778,6 +790,8 @@ int main()
>    sic.spec ('c');
>    siip.spec (&x);
>  
> +  Cfoo<double> cfoo;
> +
>    Garply<int> f;
>    Garply<char> fc;
>    f.x = 13;
> diff --git a/gdb/testsuite/gdb.cp/templates.exp b/gdb/testsuite/gdb.cp/templates.exp
> index 388c4f268c..2ca154e4f5 100644
> --- a/gdb/testsuite/gdb.cp/templates.exp
> +++ b/gdb/testsuite/gdb.cp/templates.exp
> @@ -289,12 +289,12 @@ do_tests
>  # weren't breakpointing past a point where the below expressions were
>  # initialized in the actual source. - djb
>  
> -gdb_test "b 770" \
> -    "Breakpoint .* at .*, line 770."
> +gdb_test "b 782" \
> +    "Breakpoint .* at .*, line 782."
>  
>  gdb_test "c" \
>      "Continuing.*Breakpoint .*" \
> -    "continue to line 770"
> +    "continue to line 782"

I've pushed a patch to master to replace this use of a hard-coded line
number with gdb_get_line_number.  If you rebase your patch you should
find this change is no longer needed.

>  
>  gdb_test "print fint" \
>     "\\$\[0-9\]* = \\{x = 0, t = 0\\}"
> @@ -475,6 +475,43 @@ gdb_test_multiple "ptype/r bazint" "ptype bazint" {
>     }
>  }
>  
> +# Check cv qualifiers and substitute parameters

Add '.' at the end of your comments please.

> +
> +if {[test_compiler_info {clang-*}]} {
> +	setup_kfail "llvm/52262 " "*-*-*"
> +}
> +gdb_test "ptype cfoo" [multi_line \
> +"type = (class |)Cfoo<double> \\\[with DataT = double\\\] \\{" \
> +  "\[ \t\]*public:" \
> +    "\[ \t\]*DataT me0;" \
> +    "\[ \t\]*const DataT me1;" \
> +    "\[ \t\]*const myfloat me2;" \
> +    "\[ \t\]*const int me3;" \
> +    "" \
> +  "\[ \t\]*private:" \
> +    "\[ \t\]*typedef float myfloat;" \
> +"\\}" \
> +] "print type of cfoo"
> +
> +# Check cv qualifiers and do not substitute

And here.

Thanks,
Andrew


> +
> +if {[test_compiler_info {clang-*}]} {
> +	setup_kfail "llvm/52262 " "*-*-*"
> +}
> +gdb_test "ptype/r cfoo" [multi_line \
> +"type = (class |)Cfoo<double> \\{" \
> +  "\[ \t\]*public:" \
> +    "\[ \t\]*double me0;" \
> +    "\[ \t\]*const double me1;" \
> +    "\[ \t\]*const Cfoo<double>::myfloat me2;" \
> +    "\[ \t\]*const int me3;" \
> +    "" \
> +  "\[ \t\]*private:" \
> +    "\[ \t\]*typedef float myfloat;" \
> +"\\}" \
> +] "print raw type of cfoo"
> +
> +
>  # ptype Baz<char, 'a'>
>  
>  gdb_test_multiple "ptype/r bazint2" "ptype bazint2" {
> -- 
> 2.25.1
> 
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928
>