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
Pedro Alves <palves@redhat.com> writes:
> 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.
I thought about this. The reason I still do this is that I can't think
of a case that we need to copy a read-write regcache to another
read-write regcache. So far, we only use copy(or transform) a
read-write regcache to a read-only regcache. However, I agree with you,
it is not a normal "copy ctor".
>
> 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);
I have a different design on this, that is, put readonly regcache and
readwrite regcache to two classes. readwrite regcache inherits readonly
regcache, and readonly regcache has a constructor whose argument is a
readwrite regcache.
class readonly_regcache
{
public:
explicit readonly_regcache (const regcache &);
}
class regcache : public readonly_regcache
{
}
What do you think?
>
> 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;
I agree. I'll add it.
--
Yao (齐尧)