RE: [PATCH 1/2][PR fortran/26373][PR fortran/22497] gdb/fortran: add support for accessing fields of extended types

["Kempke, Nils-Christian" <nils-christian.kempke@intel.com>]

Hi Tom,

> -----Original Message-----
> From: Tom Tromey <tom@tromey.com>
> Sent: Friday, January 14, 2022 9:06 PM
> To: Nils-Christian Kempke via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Kempke, Nils-Christian <nils-christian.kempke@intel.com>; Bernhard
> Heckel <bernhard.heckel@intel.com>
> Subject: Re: [PATCH 1/2][PR fortran/26373][PR fortran/22497] gdb/fortran:
> add support for accessing fields of extended types
> 
> >>>>> ">" == Nils-Christian Kempke via Gdb-patches <gdb-
> patches@sourceware.org> writes:
> 
> >> From: Bernhard Heckel <bernhard.heckel@intel.com>
> >> Fortran 2003 supports type extension.  This patch allows access
> >> to inherited members by using their fully qualified name as described
> >> in the Fortran standard.
> 
> Thanks for the patch.
> 
> >> In doing so the patch also fixes a bug in GDB when trying to access the
> >> members of a base class in a derived class via the derived class' base
> >> class member.
> 
> >> @@ -2374,6 +2374,12 @@ value_struct_elt (struct value **argp,
> >>        if (v)
> >>  	return v;
> 
> >> +      /* Fortran: If it is not a field it is the type name of an inherited
> >> +	 structure.  */
> >> +      v = search_struct_field (name, *argp, t, 1);
> >> +      if (v)
> >> +	return v;
> 
> Putting this here makes me wonder if it's possible to construct test
> cases in other languages that would work without this patch and then
> fail with it.

Well, since this patch so far did not care about the language actually used,
it enables one to do things like

  (gdb) p derived_obj
  $1 = {<base> = {...}, <No data fields>}
  (gdb) p derived_obj.base
  $2 = {...}

in all languages, which probably is unexpected.  The above example would be
C++ and some class "derived" inheriting from "base".  The syntax
derived_obj.base is no standard C++..
I am not sure whether one would actually break things since the only leftover
check in the method is a (according to the comment) C++ specific one for
member variables and I do not think that one can break C++ with the change.

> 
> I'm not sure what the options are though.  Doing the work elsewhere, say
> in a custom Fortran expression operation, might make the Python API
> harder to work with.  Maybe having value_struct_elt check
> current_language would work... that's also quite ugly but other value
> operations are doing that.

Mh, I am not too sure about the Fortran expression - would that not require
a Fortran specific version of value_struct_elt?  The methods
search_struct_method and search_struct_field are local to the valops.c file so
this would be a bigger redesign? I know that Ada has its own value_struct_elt
but it also implements the two helper methods.

Checking the language seems ok to me, considering the style of the rest of valops.

On more option that is also cleaner than directly checking the type might be to
generalize the concept of this "member access by base name" a bit and add a
method - say base_access_by_base_type_name - to language.h allowing or
disallowing an access to the base type by its type name.  Then one would have to
loop over all base types in valops.c as well.  I am not sure whether there exist other
languages that allow this kind of access though.. If not it might be a bit much?
 
Cheers,

Nils

> 
> Or maybe my fears are overblown.
> 
> Any thoughts on this?
> 
> Tom
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