Re: [PATCH 5/6] Use copy ctor in regcache_dup

[Pedro Alves <palves at redhat dot com>]

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