Re: [PATCH 10/15] Class regcache_readonly

Simon Marchi
2018-01-24T18:01:09Z

On 2018-01-24 12:37, Yao Qi wrote:
> On Wed, Jan 24, 2018 at 4:57 PM, Simon Marchi
> <simon.marchi@ericsson.com> wrote:
>>
>> 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
>
> Yes.
>
>> 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 should be readonly_attached_regcache.

The logic was that by "default" a regcache would be attached, unless
specified otherwise (because that's what the plain regcache is). So
that's why I suggested qualifying the detached regcache as such, while
the attached ones would be implicit.

>>
>> 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
>>
>
> This naming is fine to me except for readonly_detached_regcache
> as it is too long. As "readonly" implies "detached" in current
> context,
> can we name it readonly_regcache?

In this context readonly == detached, but aren't we going to want
readonly attached regcaches at some point? If so, there will be a
clash.

Simon