Re: [PATCH 10/15] Class regcache_readonly

[Simon Marchi <simon dot marchi at ericsson dot com>]

On 2018-01-24 04:42 AM, Yao Qi wrote:
> On Wed, Jan 24, 2018 at 3:05 AM, Simon Marchi <simon.marchi@ericsson.com> wrote:
>> On 2017-12-01 05:48 AM, Yao Qi wrote:
>>> This patch adds a new class (type) for readonly regcache, which is
>>> created via regcache::save.  regcache_readonly inherts from
>>> regcache_read.
>>
>> Hi Yao,
>>
>> Just a note about the naming.  IIUC, the important thing about this
>> kind of regcache is that it's detached from any target.  Did you
>> think about naming it detached_regcache or something like that ?
>>
> 
> This kind of regcache is both detached and readonly.  As I said in
> https://sourceware.org/ml/gdb-patches/2017-07/msg00031.html,
> detached and readonly are orthogonal in design.  We have four
> different kinds of regcache,
> 
>  - readony detached regcache, this is what "class regcache_readonly"
>    about.  It has several uses,  1) record infcall state, 2) give a
>    regcache view to frame,
> 
>  - read-write detached regcache, this is what "class reg_buffer_rw"
>    about.  It is used jit.c and record-full.c, where GDB keeps a detached
>    regcache, but can read and write to it.
> 
>  - read-write attached regcache, that is what "class regcache" about.  It
>    is attached to target, read and write will go through target,
> 
>  - readonly attached regcache.  It can be used for target 'core', but this
>    piece is not included in this series,
> 
> so in this patch series, "readonly" implies "detached".
> 
> The major motivation of this patch series is to differentiate these kinds
> of regcache by different types, instead of by fields "m_readonly_p" or
> "m_detached_p" in "class regcache".

Just pitching some ideas, I don't think I understand the situation as well as
you do.

I assume we want to keep the "regcache" type to mean read/write and attached,
since that's the most common use case.  Keeping this will reduce the amount of
changes needed throughout the code base.  We can then qualify the other types
based on how they differ from "read/write" and "attached".  That would give us
(in the same order as your list above):

- readonly_detached_regcache
- detached_regcache
- regcache
- readonly_regcache

This would give a predictable naming, and makes it maybe easier to know what
to expect from each type.  The graph you used in message 0/15 would become:

                      reg_buffer
                         ^
                         |
                   ------+-----
                   ^
                   |
            readable_regcache (abstract)
                 ^
                 |
           ------+------
           ^           ^
           |           |
    detached_regcache readonly_detached_regcache
          ^
          |
      regcache

Simon