[PATCH 3/8] infcall, c++: collect more pass-by-reference information

[Aktemur, Tankut Baris]

> > Walk through a given type to collect information about whether the type
> > is copy constructible, destructible, trivially copyable, trivially copy
> > constructible, trivially destructible.  The previous algorithm returned
> > only a boolean result about whether the type is trivially copyable.
> > This patch computes more info.  Additionally, it utilizes DWARF attributes
> > that were previously not taken into account; namely,
> > DW_AT_deleted, DW_AT_defaulted, and DW_AT_calling_convention.
> 
> I'm basically happy with this, a few formatting issues and questions
> about comments below.
>

Thank you.

> >
> > +  /* FIXME taktemur/2019-04-23: What if there are multiple cctors?  */
> 
> Can such a situation arise?  If you know how it could but don't know
> how to handle it then can we expand the comment.  If you don't think
> such a situation could arise then lets delete this comment and add an
> assertion below.
> 

Such a situation can arise when there is a copy ctor that takes a
non-const &T and another that takes a const &T.  I'm planning to
add the example below to the comment:

  /* FIXME taktemur/2019-04-23: What if there are multiple copy ctors?
     E.g.:
       class C {
       public:
         C (C &c) { ... }
         C (const C &c) { ... }
       };
  */

The correct version shall be selected based on the type of the argument,
but I don't know how to express that in GDB.


> >    for (fieldnum = 0; fieldnum < TYPE_NFN_FIELDS (type); fieldnum++)
> >      for (fieldelem = 0; fieldelem < TYPE_FN_FIELDLIST_LENGTH (type, fieldnum);
> >  	 fieldelem++)
> > @@ -1282,49 +1415,53 @@ gnuv3_pass_by_reference (struct type *type)
> >  	const char *name = TYPE_FN_FIELDLIST_NAME (type, fieldnum);
> >  	struct type *fieldtype = TYPE_FN_FIELD_TYPE (fn, fieldelem);
> >
> > -	/* If this function is marked as artificial, it is compiler-generated,
> > -	   and we assume it is trivial.  */
> > -	if (TYPE_FN_FIELD_ARTIFICIAL (fn, fieldelem))
> > -	  continue;
> > -
> > -	/* If we've found a destructor, we must pass this by reference.  */
> >  	if (name[0] == '~')
> >  	  {
> > -	    info.trivially_copyable = false;
> > -	    return info;
> > +	    /* We've found a destructor.  */
> > +	    dtor_def = get_def_style (fn, fieldelem);
> > +	    info.dtor_name = TYPE_FN_FIELD_PHYSNAME (fn, fieldelem);
> 
> Maybe we should have an error or at least a warning if
> 'info.dtor_name' is not nullptr before this assignment - this would
> indicate multiple destructors, which seems weird, right?
> 

I believe the 'info.dtor_name' field can be nullptr even if a destructor
definition exists, if the destructor is inlined and hence its code does
not exist in the object file.  (Such a case also requires special treatment
and is handled at the client side, in gdb/infcall.c in part 7/8 of this patch.)
So, I thought I should gdb_assert on 'dtor_def == DOES_NOT_EXIST_IN_SOURCE'
instead.  Is this OK?

> > +	    if (is_copy_constructor_type (type, fieldtype))
> >  	      {
> > -		struct type *arg_target_type
> > -		  = check_typedef (TYPE_TARGET_TYPE (arg_type));
> > -		if (class_types_same_p (arg_target_type, type))
> > -		  {
> > -		    info.trivially_copyable = false;
> > -		    return info;
> > -		  }
> > +		cctor_def = get_def_style (fn, fieldelem);
> > +		info.cctor_name = TYPE_FN_FIELD_PHYSNAME (fn, fieldelem);
> 
> This would be where we assert that we only have one cctor I think...
> 

Similarly, I'm planning to assert 'cctor_def == DOES_NOT_EXIST_IN_SOURCE'.

> > +  bool cctor_implicitly_deleted
> > +    =  mctor_def != DOES_NOT_EXIST_IN_SOURCE
> > +    && cctor_def == DOES_NOT_EXIST_IN_SOURCE;
> 
> I think this should be parenthesised like this:
> 
>   bool cctor_implicitly_deleted
>     =  (mctor_def != DOES_NOT_EXIST_IN_SOURCE
>         && cctor_def == DOES_NOT_EXIST_IN_SOURCE);
> 
> My reference is:
>   https://www.gnu.org/prep/standards/standards.html#Formatting
> 

Thanks for the pointer.  I'll address these formatting issues in the next update.

> 
> Thanks,
> Andrew

Regards,
-Baris

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928