This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: style convention: /*foo_p=*/ to annotate bool arguments


On 04/10/16 01:48, Martin Sebor wrote:
> In a recent review Jason and I discussed the style convention
> commonly followed in the C++ front end to annotate arguments
> in calls to functions taking bool parameters with a comment
> along the lines of
> 
>   foo (1, 2, /*bar_p=*/true);
> 
> I pointed out to Jason that in another code review, Jeff asked
> to remove the same comment from the middle-end [1].  In the
> interest of consistency Jason and I thought I should bring this
> up for discussion so we can all be clear on whether or not this
> is something worth standardizing and documenting.
> 
> As a separate question, in the same discussion I mention to Jason
> a convention that I myself have found useful where the value of
> a default argument is indicated in a comment in a function
> definition that is declared elsewhere to take one, e.g., like so:
> 
>   // In some header:
>   void foo (int, int, bool = -1);
> 
>   // In some .c file:
>   void foo (int x, int y, bool bar_p /* = false */)
>   {
>     ...
>   }
> 
> Jason thought this would be a good convention.  Is there any
> interest in/support for adopting it?
> 

IMHO, a commenting convention that makes it easy to write incorrect and
misleading comments is far worse than a convention that omits the
comments entirely.

I am not involved in gcc development (I'm just an interested observer on
this list), so I don't know much about the coding style in gcc at the
moment.  But /I/ would prefer a style like this:

// In some header:
void foo(int x, int y, bool bar_p = true);

// In some .c file:
void foo(int x, int y, bool bar_p)
{
...
}

There are several differences, which gcc folks may or may not agree
with, but which might be worth considering:

1. "bool" should be either "true" or "false" - not an integer.

2. I think parameter names are important in declarations - they help
tell you what the function is, and how to use it.

(As an aside, I would /really/ like a gcc warning option that would
check for consistency in parameter names.  This would be particularly
useful if C++ ever gets a named parameter feature.)

3. In the implementation, there is /no/ comment about the default value
of "bar_p".  The implementation has to work correctly regardless of the
value of "bar_p", so knowing the default value is of little benefit.  If
it is relevant, perhaps because you want to emphasize a particular path
for speed, then a comment might be useful - but it is better to use code
(like __builtin_expect) than comments.  Comments make it easy to be
inconsistent, and are unnecessary when the same information is easily
available by looking at the header (modern IDE's make that quick).

mvh.,

David


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]