Re: [RFC] mips-tdep.c: Update mips_register_to_value(), et al...

Yao Qi
2010-12-10T01:28:54Z

On 12/10/2010 08:34 AM, Kevin Buettner wrote:
>>>
>>> * mips-tdep.c (big_endian_4_byte_fp_reg_with_double_type_p)
>>> (eight_byte_gp_reg_with_shorter_type_p): New functions.
>>
>> Names of these two new functions are a little bit long. :-)
>
> I agree that they are long, but are they too long?
>
> Here, taken from the patch, are the various uses of these functions:
>
> mips_convert_register_p:
> + return big_endian_4_byte_fp_reg_with_double_type_p (gdbarch, regnum, type)
> + || eight_byte_gp_reg_with_shorter_type_p (gdbarch, regnum, type);
>
> mips_register_to_value and mips_value_to_register:
> + if (big_endian_4_byte_fp_reg_with_double_type_p (gdbarch, regnum, type))
> + else if (eight_byte_gp_reg_with_shorter_type_p (gdbarch, regnum, type))
>
> Note that the names are just (barely) short enough that the function
> parameters do not need to be placed on a separate line.
>

Yes, from this point of view, they are not too long.

>> Sometimes, it is hard to describe function's behavior only by its name.
>> Can we name them as mips_convert_register_p_{1,2}, and give comments in
>> details to each of them?
>
> That could be done. I would prefer to use names that are a bit more
> descriptive though. I think that the names that I have chosen, as bad
> as they are, still make the code easier to understand than if generic
> predicate names were used. I was hoping that someone would suggest some
> names that are shorter, but still reasonably descriptive. But, as you
> say, that might not be possible.
>
> Hmm...
>
> Maybe it'd work out better if I put less detail in the name. How
> about these names?
>
> mips_convert_register_float_case_p
> mips_convert_register_gpreg_case_p
>
> (Or something along those lines...)

Looks fine.

>
> You'd still have to read the code, or possibly a comment, to find
> out the precise meaning of the predicate, but the names contain
> just enough information so that one won't unwittingly be confused
> with the other.

It is perfect to use a function according to the description of its
name, without reading comments and precise meaning of it. Usually, it
might be impossible. Even if we choose
"big_endian_4_byte_fp_reg_with_double_type_p" and
"eight_byte_gp_reg_with_shorter_type_p" for function names, some people
will still have to read the comments, at least, when they want to use
them. Personally, I'd like to name function in a reasonably descriptive
way, and leave comments in details for that function.

--
Yao (é½å°§)