At 11:51 AM 26/10/2005 +0100, Julian Foad wrote:
[snip]
>In this case, it's not worth splitting the patch into "implement the
macro" and
>"use the macro" because both parts are so simple (even though the latter
part
>is large), so just put it all in a single patch.
Okay.
[snip]
>I'm still thinking about whether we can do better than putting the
definition
>in svn_types.h. We really should try hard to avoid using that file as a
>dumping ground. I'm thinking svn_private_config.h would be better, since
this
>is an implementation detail that users of our libraries don't need to know
>about, and since the appropriate definition might depend on the compilation
>environment. For example, some compilers might complain more about
"(void)x"
>being a statement with no effect than they do about unused parameters.
I've figured out how to put it into svn_private_config.h.
I have a proposed change to the definition of the macro. Basically, what's
happening as I add it into all the places that need it is stuff like this:
svn_prop_kind_t kind = svn_property_kind (NULL, name);
SVN_UNUSED_PARAMETER (pool);
if (kind != svn_prop_regular_kind)
return svn_error_createf (...);
It has to go *after* the local variable declarations because it is a
statement, which often has the effect of splitting up code. What I was
thinking would be to allow:
SVN_UNUSED_PARAMETER (pool);
svn_prop_kind_t kind = svn_property_kind (NULL, name);
if (kind != svn_prop_regular_kind)
return svn_error_createf
..by turning the macro into something like:
#define SVN_UNUSED_PARAMETER(x) void *svn_unused_parameter__##x = &x
The compiler would then see a declaration. Now, the compiler might also be
emitting warnings for unused local variables. However, at least when GCC is
being used, there is an attribute which can be applied to it which will
stop it from producing that warning: __attribute__((__unused__)). In
addition, if the __deprecated__ attribute is applied to the parameter in
the function's signature, then the compiler will emit a warning whenever
code actually does use the parameter. (Deprecation isn't a perfect match to
the meaning we want, but it's the only attribute I saw which would result
in a warning.) In order to do this, another macro would also be needed,
perhaps:
#define SVN_UNUSED_PARAMETER_NAME(x) x __attribute__((__deprecated__))
it occurs to me that the __unused__ attribute could probably also be
applied directly here, but of course only GCC will support this, so
SVN_UNUSED_PARAMETER would still be needed.
Another thing that SVN_UNUSED_PARAMETER_NAME could do is simply leave the
name off for C++ code:
#ifdef __cplusplus
# define SVN_UNUSED_PARAMETER_NAME(x)
#elif defined __GCC__
# define SVN_UNUSED_PARAMETER_NAME(x) x __attribute__((__deprecated__))
#else
# define SVN_UNUSED_PARAMETER_NAME(x) x
#endif
It would mean even more expansion in the size of the code, but it would
also mean that we would be able to detect instances where parameters that
were marked as unused were in fact being used. I'm not sure how important
this is.
Usage would look like this:
static svn_error_t *
validate_prop (const char *name,
apr_pool_t * SVN_UNUSED_PARAMETER_NAME (pool))
{
SVN_UNUSED_PARAMETER (pool);
svn_prop_kind_t kind = svn_property_kind (NULL, name);
if (kind != svn_prop_regular_kind)
return svn_error_createf (...);
return SVN_NO_ERROR;
}
Any comments?
Jonathan Gilbert
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Oct 27 00:10:06 2005