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

[Yao Qi <qiyaoltc@gmail.com>]

Pedro Alves <palves@redhat.com> writes:

> I think that that's too incomplete to evaluate.  :-)
>
> Why regcache on top of readonly_regcache and not the other
> way around?  Off hand, I'd think that 
>
>  struct regcache_base;
>  struct readonly_regcache : regcache_base {};
>  struct regcache : regcache_base {};
>
> would be the "obvious" first choice.

I have to agree, design like this affects other parts a lot though.

>
> What happens to all the "regcache->readonly_p" checks in
> spread around in multiple functions?  Do they disappear?

I wish they will.
  
> I think that if the design ends up with that flag still present 
> and functions exposing interfaces that work with
> a "struct regcache *" that can either be readonly
> or write-through, then it's likely that the design
> isn't complete.  [But replacing a single boolean
> checked in a few select places by virtual methods and a
> full blown vtable and a bunch of dispatching makes me
> cringe a bit too.  :-)  But OTOH, I suspect you want
> to add virtual methods for unit testing.]

I get your point and let me see how far can I go.  Yes, I need to make
some regcache methods virtual for unit tests, so far I only need to make
raw_write and ~regcache virtual.

>
> I'm totally not against this direction, to be clear,
> but it'd still suggest adding the tag dispatch ctor
> first (the simple version with no factory, just adds two
> lines of code compared to the copy ctor version), which
> allows getting rid of the heap allocation and the cleanups
> as you're doing in patch #6, and then consider changing
> the hierarchy in a follow up patch.

No problem, I'll replace my copy ctor with your tag dispatch ctor.

-- 
Yao (齐尧)