Re: [PATCH PR gdb/16841] virtual inheritance via typedef cannot find base

[Simon Marchi <simark@simark.ca>]

On 2018-06-15 07:45 PM, Weimin Pan wrote:
> Fix failure to find member of a typedef base class
> 
> The test case below demonstrates the problem, as described in this PR's Comment 5:
> 
> typedef struct {
>         int x;
> } A;
> 
> struct C : A {
>         int y;
> };
> 
> int main()
> {
>         C c;
>         return 55;
> }
> 
> $ gdb a.out
> (gdb) ptype C::x
> Internal error: non-aggregate type to value_struct_elt_for_reference
> 
> In value_struct_elt_for_reference(), need to call check_typedef() on
> the aggregate type to handle the case of *curtype being ptr->typedef.

Thanks, that makes sense.  I noted some nits, you can push the patch after
addressing them.

> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index b2938b1..78771d3 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,9 @@
> +2018-06-12  Weimin Pan  <weimin.pan@oracle.com>
> +
> +	PR gdb/16841
> +	* gdb/testsuite/gdb.cp/typedef-base-ptype.cc: New file.
> +	* gdb/testsuite/gdb.cp/typedef-base-ptype.exp: New file.

Filenames should be relative to the ChangeLog file, so remove "gdb/testsuite/".

> diff --git a/gdb/testsuite/gdb.cp/typedef-base-ptype.cc b/gdb/testsuite/gdb.cp/typedef-base-ptype.cc
> new file mode 100644
> index 0000000..bfbcc4a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/typedef-base-ptype.cc
> @@ -0,0 +1,30 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2018 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +typedef struct {
> +        int x;
> +} A;
> +
> +struct C : A {
> +        int y;
> +};
> +
> +int main()
> +{
> +        C c;
> +        return 55;
> +}

We try to make the test code follow the GNU coding style (unless testing
a particular formatting is the purpose of the test).

> diff --git a/gdb/testsuite/gdb.cp/typedef-base-ptype.exp b/gdb/testsuite/gdb.cp/typedef-base-ptype.exp

I would suggest naming this test "typedef-base.exp", because we may add other tests
than ptype in it.  For example, David's original reproducer in PR 16841 still won't
work after this patch (AFAIK), and would be a good candidate to end up as a test in
this file.

> new file mode 100644
> index 0000000..e588fd3
> --- /dev/null
> +++ b/gdb/testsuite/gdb.cp/typedef-base-ptype.exp
> @@ -0,0 +1,37 @@
> +# Copyright 2018 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.

Can you add a one-liner comment that states the purpose of the test?  Something
like

  # Make sure that inheritance through a typedef is well handled.

Thanks,

Simon