Fix accessing a function's fields (parameters) from Python (PR 18073)

  Since 7.4, gdb doesn't allow calling .fields() on a function type, even
though the documentation states it should return a list corresponding to
the function's parameters.  This patch restores the intended behaviour
and adds a test for it.

Reg-tested on Arch Linux x86-64.

gdb/ChangeLog:

	PR python/18073
	* python/py-type.c (typy_get_composite): Allow returning a
	function type.

gdb/testsuite/ChangeLog:

	PR python/18073
	* gdb.python/py-type.c (C::a_method): New.
	(C::a_const_method): New.
	(C::a_static_method): New.
	(a_function): New.
	* gdb.python/py-type.exp (test_fields): Test getting fields
	from function and method.
---
 gdb/python/py-type.c                 |  5 +++--
 gdb/testsuite/gdb.python/py-type.c   | 34 ++++++++++++++++++++++++++++++++--
 gdb/testsuite/gdb.python/py-type.exp | 28 ++++++++++++++++++++++++----
 3 files changed, 59 insertions(+), 8 deletions(-)
  

On 28 September 2015 at 00:43, Simon Marchi <simon.marchi@polymtl.ca> wrote:
> Since 7.4, gdb doesn't allow calling .fields() on a function type, even
> though the documentation states it should return a list corresponding to
> the function's parameters.  This patch restores the intended behaviour
> and adds a test for it.
>
> Reg-tested on Arch Linux x86-64.
>
> gdb/ChangeLog:
>
>         PR python/18073
>         * python/py-type.c (typy_get_composite): Allow returning a
>         function type.
>
> gdb/testsuite/ChangeLog:
>
>         PR python/18073
>         * gdb.python/py-type.c (C::a_method): New.
>         (C::a_const_method): New.
>         (C::a_static_method): New.
>         (a_function): New.
>         * gdb.python/py-type.exp (test_fields): Test getting fields
>         from function and method.
> ---
>  gdb/python/py-type.c                 |  5 +++--
>  gdb/testsuite/gdb.python/py-type.c   | 34 ++++++++++++++++++++++++++++++++--
>  gdb/testsuite/gdb.python/py-type.exp | 28 ++++++++++++++++++++++++----
>  3 files changed, 59 insertions(+), 8 deletions(-)
>
> diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
> index 7b3f8f9..ef4e719 100644
> --- a/gdb/python/py-type.c
> +++ b/gdb/python/py-type.c
> @@ -495,10 +495,11 @@ typy_get_composite (struct type *type)
>       exception.  */
>    if (TYPE_CODE (type) != TYPE_CODE_STRUCT
>        && TYPE_CODE (type) != TYPE_CODE_UNION
> -      && TYPE_CODE (type) != TYPE_CODE_ENUM)
> +      && TYPE_CODE (type) != TYPE_CODE_ENUM
> +      && TYPE_CODE (type) != TYPE_CODE_FUNC)
>      {
>        PyErr_SetString (PyExc_TypeError,
> -                      "Type is not a structure, union, or enum type.");
> +                      "Type is not a structure, union, enum, or function type.");
>        return NULL;
>      }
>
> diff --git a/gdb/testsuite/gdb.python/py-type.c b/gdb/testsuite/gdb.python/py-type.c
> index e3901cc..36ce1d8 100644
> --- a/gdb/testsuite/gdb.python/py-type.c
> +++ b/gdb/testsuite/gdb.python/py-type.c
> @@ -35,6 +35,24 @@ struct C
>  {
>    int c;
>    int d;
> +
> +  int
> +  a_method (int x, char y)
> +    {
> +      return x + y;
> +    }
> +
> +  int
> +  a_const_method (int x, char y) const
> +    {
> +      return x + y;
> +    }
> +
> +  static int
> +  a_static_method (int x, char y)
> +    {
> +      return x + y;
> +    }
>  };
>
>  struct D : C
> @@ -59,6 +77,12 @@ enum E
>  struct s vec_data_1 = {1, 1};
>  struct s vec_data_2 = {1, 2};
>
> +static int
> +a_function (int x, char y)
> +{
> +  return x + y;
> +}
> +
>  int
>  main ()
>  {
> @@ -72,15 +96,21 @@ main ()
>    D d;
>    d.e = 3;
>    d.f = 4;
> +
> +  c.a_method (0, 1);
> +  c.a_const_method (0, 1);
> +  C::a_static_method (0, 1);
>  #endif
>    enum E e;
> -
> +
>    st.a = 3;
>    st.b = 5;
>
>    e = v2;
>
>    ss.x = 100;
> -
> +
> +  a_function (0, 1);
> +
>    return 0;      /* break to inspect struct and array.  */
>  }
> diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
> index 58a2394..26126eb 100644
> --- a/gdb/testsuite/gdb.python/py-type.exp
> +++ b/gdb/testsuite/gdb.python/py-type.exp
> @@ -75,6 +75,21 @@ proc test_fields {lang} {
>        gdb_test "python print (c.type == gdb.parse_and_eval('d').type)" "False"
>        gdb_test "python print (c.type == gdb.parse_and_eval('d').type.fields()\[0\].type)" \
>           "True"
> +
> +      # Test fields of a method (its parameters)
> +      gdb_test "python print (len (gdb.parse_and_eval ('C::a_method').type.fields ()))" "3"
> +      gdb_test "python print (gdb.parse_and_eval ('C::a_method').type.fields ()\[0\].type)" "C \\* const"
> +      gdb_test "python print (gdb.parse_and_eval ('C::a_method').type.fields ()\[1\].type)" "int"
> +      gdb_test "python print (gdb.parse_and_eval ('C::a_method').type.fields ()\[2\].type)" "char"
> +
> +      gdb_test "python print (len (gdb.parse_and_eval ('C::a_const_method').type.fields ()))" "3"
> +      gdb_test "python print (gdb.parse_and_eval ('C::a_const_method').type.fields ()\[0\].type)" "const C \\* const"
> +      gdb_test "python print (gdb.parse_and_eval ('C::a_const_method').type.fields ()\[1\].type)" "int"
> +      gdb_test "python print (gdb.parse_and_eval ('C::a_const_method').type.fields ()\[2\].type)" "char"
> +
> +      gdb_test "python print (len (gdb.parse_and_eval ('C::a_static_method').type.fields ()))" "2"
> +      gdb_test "python print (gdb.parse_and_eval ('C::a_static_method').type.fields ()\[0\].type)" "int"
> +      gdb_test "python print (gdb.parse_and_eval ('C::a_static_method').type.fields ()\[1\].type)" "char"
>      }
>
>      # Test normal fields usage in structs.
> @@ -109,10 +124,10 @@ proc test_fields {lang} {
>      gdb_test "python print (not not st.type)" "True" "Check conversion to bool"
>
>      # Test rejection of mapping operations on scalar types
> -    gdb_test "python print (len (st.type\['a'\].type))" "TypeError: Type is not a structure, union, or enum type.*"
> -    gdb_test "python print (st.type\['a'\].type.has_key ('x'))" "TypeError: Type is not a structure, union, or enum type.*"
> -    gdb_test "python print (st.type\['a'\].type.keys ())" "TypeError: Type is not a structure, union, or enum type.*"
> -    gdb_test "python print (st.type\['a'\].type\['x'\])" "TypeError: Type is not a structure, union, or enum type.*"
> +    gdb_test "python print (len (st.type\['a'\].type))" "TypeError: Type is not a structure, union, enum, or function type.*"
> +    gdb_test "python print (st.type\['a'\].type.has_key ('x'))" "TypeError: Type is not a structure, union, enum, or function type.*"
> +    gdb_test "python print (st.type\['a'\].type\['x'\])" "TypeError: Type is not a structure, union, enum, or function type.*"
> +    gdb_test "python print (st.type\['a'\].type.keys ())" "TypeError: Type is not a structure, union, enum, or function type.*"
>
>      # Test conversion to bool on scalar types
>      gdb_test "python print (not not st.type\['a'\].type)" "True"
> @@ -148,6 +163,11 @@ proc test_fields {lang} {
>      gdb_test "python print (vec1 == vec2)" "True"
>      gdb_py_test_silent_cmd "python vec3 = vec_data_2.cast(ar\[0\].type.vector(1))" "set vec3" 1
>      gdb_test "python print (vec1 == vec3)" "False"
> +
> +    # Test fields of a function (its parameters)
> +    gdb_test "python print (len (gdb.parse_and_eval ('a_function').type.fields ()))" "2"
> +    gdb_test "python print (gdb.parse_and_eval ('a_function').type.fields ()\[0\].type)" "int"
> +    gdb_test "python print (gdb.parse_and_eval ('a_function').type.fields ()\[1\].type)" "char"
>    }
>  }
>
> --
> 2.5.3
>

Ping.
  

Hi Simon,

> > Since 7.4, gdb doesn't allow calling .fields() on a function type, even
> > though the documentation states it should return a list corresponding to
> > the function's parameters.  This patch restores the intended behaviour
> > and adds a test for it.
> >
> > Reg-tested on Arch Linux x86-64.
> >
> > gdb/ChangeLog:
> >
> >         PR python/18073
> >         * python/py-type.c (typy_get_composite): Allow returning a
> >         function type.
> >
> > gdb/testsuite/ChangeLog:
> >
> >         PR python/18073
> >         * gdb.python/py-type.c (C::a_method): New.
> >         (C::a_const_method): New.
> >         (C::a_static_method): New.
> >         (a_function): New.
> >         * gdb.python/py-type.exp (test_fields): Test getting fields
> >         from function and method.

The patch looks good to me, although I am not one of the main Python
maintainers. Normally, I'd ask the author to wait an extra week for
additional comments, but since you've already waited a month, I think
it's only fair to give you the go ahead to push. If there are comments
with your code, we can handle them then.

Thanks also adding the tests. You probably know this, but this is
always appreciated.

Thanks!
  

On 21 October 2015 at 15:47, Joel Brobecker <brobecker@adacore.com> wrote:
> The patch looks good to me, although I am not one of the main Python
> maintainers. Normally, I'd ask the author to wait an extra week for
> additional comments, but since you've already waited a month, I think
> it's only fair to give you the go ahead to push. If there are comments
> with your code, we can handle them then.
>
> Thanks also adding the tests. You probably know this, but this is
> always appreciated.
>
> Thanks!

Thanks, pushed!