This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 3/8] infcall, c++: collect more pass-by-reference information
- From: Andrew Burgess <andrew dot burgess at embecosm dot com>
- To: "Aktemur, Tankut Baris" <tankut dot baris dot aktemur at intel dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Sun, 23 Jun 2019 20:23:49 +0100
- Subject: Re: [PATCH 3/8] infcall, c++: collect more pass-by-reference information
- References: <1556029914-21250-1-git-send-email-tankut.baris.aktemur@intel.com> <1556029914-21250-4-git-send-email-tankut.baris.aktemur@intel.com> <20190522203402.GL2568@embecosm.com> <B98F7326B8E238409968F562D326E1A90D023B2C@IRSMSX106.ger.corp.intel.com>
* Aktemur, Tankut Baris <tankut.baris.aktemur@intel.com> [2019-05-31 13:55:58 +0000]:
> > > 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) { ... }
> };
> */
That would be great, I find information like this in comments very
helpful so thank you.
>
> 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'.
>
OK, that sounds reasonable.
If nobody else reviews the remaining patches in this series then I
will get around to them soon I hope. I've been crazy busy the last
couple of weeks and am still trying to catch up on everything.
Thanks,
Andrew
> > > + 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
>