On 10/04/2016 04:41 AM, Jonathan Wakely wrote:
On 4 October 2016 at 10:21, David Brown wrote:
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 like this convention at the call-site, otherwise "true" or "false"
doesn't tell you much and you have to look at the declaration.
FWIW, it occurs to me that another similar case is passing NULL
to a function that expects a pointer (or tree) argument. When
the function takes more than one such pointer (and perhaps even
handles null in some but not others), it can be hard to tell
which is which. For the null case (as opposed to for simply
determining which argument is an ordinary pointer and which one
is a tree) it's not enough to look at the declaration. One has
to look at the definition of the function or the comment above
it that lays out its contract to tell whether it handles a null
pointer or tree. But beyond the NULL_TREE attempt that (only
superficially) distinguishes ordinary pointers from trees there
is no convention in place where the argument would be similarly
annotated.
The same observation can be made about other constants (in the
GCC sources I've looked it, frequently argument numbers such as
in TREE_OPERAND or get_nth_callarg, or initial container sizes
such as auto_vec constructor, etc.)
(I realize that these examples could be viewed as arguments for
expanding the C++ FE convention beyond bool, or for abandoning
it as too incomplete to be generally useful.)
IMHO even better is to not use bool and define an enumeration type, so
the call site has something unambiguous like foo (1, 2, yes_bar) or
foo (1, 2, no_bar).
This strikes me as an indictment of hardcoding literals of any
kind, and a good argument in favor of using symbolic constants
in general, but less like a solution to a problem that's unique
to the two bool constants.
I assume the -1 default argument was a typo and that nobody is
proposing using -1 as a sensible bool value. I hope that's the case
anyway :-)
Yes, that was a typo, thanks for the correction!
In general I agree, although it could be useful in a more realistic
example. If the parameter with a default argument is a 'tree' and
could be null, then the comment serves as a reminder that callers
might not have passed a valid tree:
int frob_trees(tree lhs, tree rhs /* = NULL_TREE */)
{
// Handle the case where only one argument is passed:
if (rhs == NULL_TREE)
rhs = lhs; // or something more sensible.
// ...
}
Even here it's not hugely valuable, as the "Handle the case ..."
comment can convey the same information.
This is a great example, thank you! A "Handle NULL_TREE comment"
does convey the same information but it can get lost in the rest
of the comment text. In my own experience, the /* = NULL_TREE */
comment is much less likely to be missed. (YMMV of course.)
That said, putting attribute nonnull to use in declarations of
functions that do not accept null pointers would arguably be
an even better way of both documenting the contract for pointers
and helping detect its violations early.
Martin
