This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 5/6] Use copy ctor in regcache_dup
- From: Pedro Alves <palves at redhat dot com>
- To: Yao Qi <qiyaoltc at gmail dot com>, gdb-patches at sourceware dot org
- Date: Thu, 27 Apr 2017 18:37:24 +0100
- Subject: Re: [PATCH 5/6] Use copy ctor in regcache_dup
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=palves at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com D566D75726
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com D566D75726
- References: <1493152106-3246-1-git-send-email-yao.qi@linaro.org> <1493152106-3246-6-git-send-email-yao.qi@linaro.org>
On 04/25/2017 09:28 PM, Yao Qi wrote:
> gdb:
>
> 2017-04-25 Yao Qi <yao.qi@linaro.org>
>
> * regcache.c (regcache::regcache): New copy ctor.
> (do_cooked_read): Moved above.
> (regcache_dup): Use copy ctor.
> * regcache.h (struct regcache): Declare copy ctor and remove
> friend regcache_dup.
> ---
> index e6494a9..0d302a7 100644
> --- a/gdb/regcache.h
> +++ b/gdb/regcache.h
> @@ -240,6 +240,10 @@ public:
> : regcache (gdbarch, aspace_, true)
> {}
>
> + /* Copy constructor, create a readonly regcache from a non-readonly
> + regcache. */
> + regcache (const regcache &);
This one doesn't look right to me. This isn't a copy in the
normal C++ object copy sense. The new object isn't semantically the
same as the source. One can't use the new object the same way as the
source regcache, they're not interchangeable. This is bound to generate
confusion and problems.
Considering patch #6, it'd make more sense to me to
make that a separate constructor with tag dispatching, like:
struct regcache
{
struct readonly_t {};
static constexpr readonly_t readonly {};
regcache (readonly_t, const regcache &src); // old regcache_dup
};
Then used like:
regcache ro_copy (regcache::readonly, src);
or if you want, you could make that tag-based ctor private and
add a factory function:
struct regcache
{
private:
struct readonly_t {};
regcache(readonly_t, const regcache &src);
regcache(regcache &&src) { // implement this } // move ctor
public:
static regcache make_readonly_copy (const regcache &src)
{
return regcache (readonly_t{}, src);
}
};
Used like
regcache ro_copy = regcache::make_readonly_copy (src);
In any case, I think we should make sure to disable
the regular copy methods since the type doesn't really
support normal copy:
regcache(const regcache &) = delete;
void operator= (const regcache &) = delete;
Thanks,
Pedro Alves