Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Yao Qi <yao@codesourcery.com>
To: gdb-patches@sourceware.org
Subject: Re: [RFC] mips-tdep.c: Update mips_register_to_value(), et al...
Date: Fri, 10 Dec 2010 01:28:00 -0000	[thread overview]
Message-ID: <4D018246.6010508@codesourcery.com> (raw)
In-Reply-To: <20101209173448.47fe4f14@mesquite.lan>

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 (齐尧)


  reply	other threads:[~2010-12-10  1:28 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-08 23:47 Kevin Buettner
2010-12-09  9:01 ` Yao Qi
2010-12-10  0:34   ` Kevin Buettner
2010-12-10  1:28     ` Yao Qi [this message]
2010-12-09 11:06 ` Mark Kettenis
2010-12-11  0:04   ` Kevin Buettner
2010-12-16 22:52     ` Kevin Buettner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D018246.6010508@codesourcery.com \
    --to=yao@codesourcery.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox